-
-
Notifications
You must be signed in to change notification settings - Fork 50
Adding missing enums to FeatureName and CanvasAlphaMode. #719
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
base: main
Are you sure you want to change the base?
Conversation
hey, I had a look and I believe the issue might actually be related to this line wgpu-py/wgpu/backends/wgpu_native/_api.py Line 790 in d25edb1
CanvasAlphaModes in _classes .
Can you check what capabilities your canvas/adapter return? import wgpu
from wgpu.gui.auto import WgpuCanvas
canvas = WgpuCanvas()
adapter = wgpu.gpu.request_adapter_sync()
context = canvas.get_context("wgpu")
capabilities = context._get_capabilities(adapter)
print(capabilities) as for the changes. .idl is an external file - so it can't be edited. Differences are marked in
|
That's odd 🤔 So it looks like "unpremultiplied" is referenced in webgpu.h, but not in the webgpu spec. I'm not sure if this is a deliberate difference, or that the spec simply has not caught up yet. I don't think there's a super-elegant way to fix this, but we should make sure that things can work on machine such as yours. I think @Vipitis's idea makes sense. In - if alpha_mode not in enums.CanvasAlphaMode:
+ if not (alpha_mode in enums.CanvasAlphaMode or alpha_mode in ["unpremultiplied"]}:
I'm thinking that if "opaque" is used, but it's not in `capabilities["alpha_modes"]`, maybe we should fallback to an alpha mode that is supported, and print a warning. What do you think @Vipitis ? |
I am more wondering how you get a difference between Alpha blending is one of these cases where you get undefined behavior by some GPU vendors and even spec violations. So being a little more lenient might be the way to go. |
Thanks for such a quick reply!
Yes, this is the query that made me think there is a missing field in CanvasAlphaModes, here is the output:
For the record, I'm on a 2022 M2 Mac Air running Ventura 13.4.
Oh, thanks! I wasn't able to run into this casually digging through the code, do you think there is some way to make it more discoverable?
This works on my machine too---happy to change the diff to this if y'all say it's the way to go. |
I think a good explanation is given in the codegen readme: https://github.com/pygfx/wgpu-py/blob/main/codegen/README.md After spending maybe 100 hours in this codebase doing the updates I feel like I understand most of it by now. It helps to see the separation between the webgpu front such as wgpu-py/codegen/wgpu_native_patcher.py Line 123 in d25edb1
I ran into this myself the other day, perhaps we can find a way to include all native only flags and enums without needing to manually whitelist them there. But some of the names are nearly the same and could be confusing. I hope this helps a little, maybe if you can share which direction your code digging went - maybe that can help to improve the documentation. |
Oh, glad to see 'opqaue' is in there 😅 I initially misread that you meant 'unpremultiplied' was the only one. To provide some context on API 'discoverability'; the idea is that we stick to the WebGPU spec for the main API, and users can include native-specific features from
Yeah, also these cases are rare, and tend to be edge-cases that are each special in their own way, so I guess just local whitelisting is the easiest way 🤷
Thanks for testing. That would be great! |
Hi friends,
Please let me know if I am missing something in this PR! I tried going through the how-to-contribute docs but it's highly likely I missed something.
I ran into an issue where
unpremultiplied
was the only canvas alpha mode available on my Mac, but the check on line 350 of_classes.py
inside ofGPUCanvasConfigure
would fail. Adding this to the enum fixes it and I am able to use alpha blending.I also had to use
texture-adapter-specific-format-features
to enable multisampling on f32 textures. Using the string works, but I figured I'd add it too since I am changing things.Looking forward to feedback!
Best,
Ana