-
-
Notifications
You must be signed in to change notification settings - Fork 382
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
fix: avoid norm16 for textures with linear filtering #3194
base: master
Are you sure you want to change the base?
Conversation
Can we render a simple offscreen canvas (16x16) using the sample pattern (in your examples) to check if it is black entirely? This will help us decide whether to use the norm16 usage with linear filtering. I don't see any other viable options. The funny thing is WebKit in iOS also doesn't have support for float linear, which makes me wonder if this approach (rendering something as a template and then deciding on the right path—norm16 or not, float or not, filterable or not) is the maintainable way forward. |
806ad25
to
dd7e000
Compare
I added the runtime check on a 4x4 canvas. It should be enough to check one pixel, if it's not black then linear filtering works. It's probably better for compatibility to do these checks at runtime, but there might be some performance impact. A 4x4 canvas is small so it's fine, but doing the same test on a 512x512 canvas gave me a warning from chrome, about a GPU stall when using readPixels. OpenGL and its adjacent APIs like WebGL have always had quirks, on top of multiple versions and extensions... I suppose we've been spoiled by how uniformly supported most web/JS stuff has become after IE died. |
This only happens on certain devices due to a driver bug, see KhronosGroup/WebGL#3706
dd7e000
to
7f7cbef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code-wise this looks good.
LGTM. On the question of runtime testing of extension functionality - the OpenGL/webGL specs are clear that if the extension is present, it is implemented. Occasionally, we come across such issues where the driver implementers left something out. IMO, testing all the required functionality can be an added step but should not be part of the render step. I can imagine a testing suite runs through and spits out all the supported functionality providing the flags that need to be set for a working vtk-js environment on the current system. These flags can then form a configuration file loaded in each time for future vtk-js app runs. |
Context
On certain hardware, combining linear texture filtering with the normalized fixed point types from EXT_texture_norm16 results in black pixels and a WebGL warning from the browser.
According to this proposed webgpu extension, there are a lot of mobile devices that don't support linear filtering for norm16 (eg Adreno 5XX, 7XX devices). This includes recent flagships like the Samsung Galaxy 24 Ultra, where I found this issue.
Unlike floats which have the OES_texture_float_linear extension, it doesn't seem possible to check using webgl if linear filtering is present for norm16 types. It would have been much better to limit the change only when it really is a problem. Not sure if I'm missing anything here, but this seems like a gap in the standard... I might open an issue there to askI asked: KhronosGroup/WebGL#3706
Also, here's two recent bugs from cornerstone that I think are a manifestation of this problem. Same error message, same black canvas (except turned gray due to window levels)
The phones mentioned there are Huawei P50, Huawei P60, Huawei Mate 9. These have Adreno/Mali GPUs, matching what the webgpu proposal said.
Results
Texture now acts as if EXT_texture_norm16 is unsupported when LINEAR filtering is chosen. It should fall back to some float type.
Changes
by defaultwhen a runtime check determines it won't workadded override API if someone wants to force itenableLinearNorm16Ext(useLinearNorm16: boolean): void
PR and Code Checklist
npm run reformat
to have correctly formatted codeTesting
Unfortunately you need a phone that has a problematic GPU. Like the previous texture filtering issue, I tested this with a modified vtk.js directly in OHIF.
If you do have a phone, I found a useful jsfiddle that reproduces the bug. It directly renders a texture with webgl: https://jsfiddle.net/sedghi/hxLegk5y/
This is how it should look, on a desktop GPU where everything is fine:
This is how it looks on a phone that has the problem:
And identical code, but replacing LINEAR with NEAREST fixes the black image: