Make Intel Mesa Bug Workaround More Specific #1653
Conversation
This helps make sure that we don't workaround the fastclear bug on cards that don't actually have the bug, by filtering on the renderer string.
| let gl = &device.shared.context; | ||
|
|
||
| // Check renderer version to see if we need to workaround intel mesa bug | ||
| if self.enable_intel_mesa_fastclear_workaround.is_none() { |
There was a problem hiding this comment.
this needs to happen in the expose function, where the renderer string is analyzed. And the result flag should go into PrivateCapabilities ideally
There was a problem hiding this comment.
this needs to happen in the expose function,
That's what I wanted to do initially, but a surface can be created without having enumerated adapters, so I didn't know the best way to go about getting to the capability information exposed by the adapter inside the surface, when the adapter could have been exposed at any time after the surface was created.
Any ideas for how to do that?
There was a problem hiding this comment.
Would this still hold when #1656 is merged?
| self.enable_intel_mesa_fastclear_workaround = Some( | ||
| renderer.contains("Mesa") | ||
| && renderer.contains("Intel") | ||
| && renderer.contains("CML GT2"), |
There was a problem hiding this comment.
This is going to be way to restrictive. This limits it to Comet Lake and only the GT2 model of that (which is the mid-range gpu in the lineup). https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4972/diffs?diff_id=75888#22f5d1004713c9bbf857988c7efb81631ab88f99_323_327 comment seems to suggest the issue applies to all skylake based machines which matches with the issue not showing up on my haswell machine.
Here are the PCI ids for all intel cards: https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/include/pci_ids/i965_pci_ids.h.
What we need to do is match on all idents which are "**L". You could write something like:
renderer.split().any(|substr| substr.len() == 3 && substr[2] == 'L') as the model check. You should also point back to this documentation to show why we're making these decisions.
There was a problem hiding this comment.
OK, great, I was wondering what exactly those model numbers meant. :)
|
|
||
| // Check renderer version to see if we need to workaround intel mesa bug | ||
| if self.enable_intel_mesa_fastclear_workaround.is_none() { | ||
| let renderer = gl.get_parameter_string(glow::RENDERER); |
There was a problem hiding this comment.
For string comparisons, we should always make them lower case first.
| let renderbuffer = gl.create_renderbuffer().unwrap(); | ||
| gl.bind_renderbuffer(glow::RENDERBUFFER, Some(renderbuffer)); | ||
| gl.renderbuffer_storage( | ||
| glow::RENDERBUFFER, | ||
| format_desc.internal, | ||
| if self.enable_intel_mesa_fastclear_workaround.unwrap() { | ||
| glow::RGBA8 |
There was a problem hiding this comment.
What end result does this have? Why can we just use a different format?
There was a problem hiding this comment.
Does this actually start making the tests pass, considering this is just for the surface?
There was a problem hiding this comment.
What end result does this have? Why can we just use a different format?
It fixes he pixelated edges on my machine. I don't know why, though. It may be related to how in the mesa issue the original poster said that if he commented out glfwWindowHint(GLFW_SRGB_CAPABLE, 1); it would render fine. It might be an issue specifically with the sRGB formats.
Beats me. 🤷♂️
Does this actually start making the tests pass, considering this is just for the surface?
That's a good point. It doesn't fix the tests.
I don't know how it's fixing it for the window display, but if it isn't fixing it for the reftests then this probably isn't going to work.
|
I'm closing this for now, being that the color format fix doesn't fix the reftests. I think we're going to need another strategy. |
This helps make sure that we don't workaround the fastclear bug on cards
that don't actually have the bug, by filtering on the renderer string.
Connections
Another attempt at #1627
Description
This makes our fastclear bug workaround more specific to the card card that isn't working. I'm not sure if we need to instead check the driver version or some other factor, though. I just applied the workaround for a few significant substrings in the renderer string.
Testing
Tested with the GL backend on Mesa Intel(R) UHD Graphics (CML GT2).