Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove colons from shader labels #1779

Merged
merged 1 commit into from
Apr 7, 2023

Conversation

Davidster
Copy link
Contributor

when using the DXC compiler feature from wgpu, iced's shaders fail to compile due to a weird bug I discovered that causes something to go wrong if the shader's label contains any colons. So this PR removed the colons (they can alternatively be escaped if you prefer that, like "iced\\:\\:image". I checked it and it works).

Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this should be automatically escaped by wgpu. Has this issue been reported there?

@Davidster
Copy link
Contributor Author

I mentioned it in Element, but from what I saw it's a limitation of their dxc dependency hassle, or potentially even the directX compiler itself. I'll take a closer look to try to pin down the right spot where the change should be made, just thought I'd open the PR since I'm now depending on an iced fork with this change.

@hecrj hecrj added bug Something isn't working improvement An internal improvement wgpu labels Apr 7, 2023
@hecrj
Copy link
Member

hecrj commented Apr 7, 2023

Sure! We can merge this. Just pointing out that wgpu should probably be escaping these if they can cause issues.

Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🥳

@hecrj hecrj enabled auto-merge April 7, 2023 04:16
@hecrj hecrj merged commit 931b30d into iced-rs:master Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working improvement An internal improvement wgpu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants