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

[wgpu.image] Workaround WGPU OpenGL heuristics #2259

Merged

Conversation

PolyMeilex
Copy link
Contributor

@PolyMeilex PolyMeilex commented Feb 18, 2024

wgpu OpenGL image rendering has been broken for years (every image is a black rectangle), but finally I accumulated enough willpower to debug and fix this.

So, it turns out that wgpu has to use heuristics to guess that the texture we are creating will be used as TEXTURE_2D_ARRAY rather than TEXTURE_2D, and OpenGL needs to know that ahead of time, otherwise it will refuse to bind it because of type mismatch.

The real fix should be on wgpu side, but that seems to be known and not obvious problem.
So in the meantime, I just increased default layers allocation in the atlas to 2. That way wgpu will see layer depth 2 and correctly assume that we want to use this texture as TEXTURE_2D_ARRAY

EDIT: Found an issue that mentions the array layer limitations: gfx-rs/wgpu#1574

Fixes #1774.
Fixes #2180.

@hecrj hecrj added this to the 0.12 milestone Feb 18, 2024
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! We finally figured out the mystery 🥳

Looks like #2009 was quite close!

@PolyMeilex
Copy link
Contributor Author

Great, when the amount of layers is a multiple of 6 we will lose images again, as wgpu hard-codes 6 to a cube map 🤦
I belive this is still better then nothing tho.

@hecrj
Copy link
Member

hecrj commented Feb 19, 2024

@PolyMeilex Looking at the code, if we are hitting the first branch of the match it means the texture isn't cube compatible; so we should be safe and never hit that problem.

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.

Thank you! Long-standing issues gone 🥳

Comment on lines +26 to +33
pub fn new(device: &wgpu::Device, backend: wgpu::Backend) -> Self {
let layers = match backend {
// On the GL backend we start with 2 layers, to help wgpu figure
// out that this texture is `GL_TEXTURE_2D_ARRAY` rather than `GL_TEXTURE_2D`
// https://github.com/gfx-rs/wgpu/blob/004e3efe84a320d9331371ed31fa50baa2414911/wgpu-hal/src/gles/mod.rs#L371
wgpu::Backend::Gl => vec![Layer::Empty, Layer::Empty],
_ => vec![Layer::Empty],
};
Copy link
Member

Choose a reason for hiding this comment

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

Made it so that the additional memory is only allocated when using OpenGL.

@hecrj hecrj enabled auto-merge February 19, 2024 07:29
@hecrj hecrj modified the milestones: 0.12, 0.12.1 Feb 19, 2024
@hecrj hecrj merged commit 121d220 into iced-rs:master Feb 19, 2024
13 checks passed
@PolyMeilex PolyMeilex deleted the wgpu-image-workaround-wgpu-gl-heuristics branch February 19, 2024 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants