I cannot reproduce this on either of those URLs, and neither of them cause brush_blend to be compiled for me. However, if I enable gfx.webrender.precache-shaders then I can indeed reproduce. We seem to hit this problem in `brush_blend_ALPHA_PASS`, `cs_svg_filter`, and `brush_yuv_image_ALPHA_PASS_TEXTURE_2D_YUV`. The driver advertises itself as having 16 varying vectors available. It appers to be up to drivers precisely how to pack varyings in to those vectors but the [spec](https://www.khronos.org/registry/OpenGL/specs/es/3.0/GLSL_ES_Specification_3.00.pdf) does require a minimum required algorithm. Angle has an [implementation](https://searchfox.org/mozilla-central/rev/2b99ea2e97eef00a8a1c7e24e5fe51ab5304bc42/gfx/angle/checkout/src/compiler/translator/VariablePacker.cpp#395) of this algorithm to test whether packing should succeed, and it says that the shaders in question should indeed fit in to 16 vecs. So it appears the driver is not spec compliant. Working around the driver bug is trivial: we can manually pack a few varyings in to vecs. But the part I'm struggling with is how to test this and ensure we don't regress it in the future, given that this driver is not tested on CI. I need to try to reverse engineer what's going on in the driver, and then probably add some rust bindings to angle and test this in angle_shader_validation. My best guess at what's happening in the driver is that it doesn't do any packing whatsoever. If we sum up the number of rows for each varying without any packing, and also remove unused varyings from the count then the numbers *nearly* look correct: For cs_svg_image after I've manually packed some varyings this logic gives me 17 vecs in the vertex shader and 16 in the fragment shader. The vertex includes `gl_Position` but the fragment does not reference `gl_FragCoord`. The spec states that: > If referenced in the fragment shader (after preprocessing), the built-in special variables (gl_FragCoord, gl_FrontFacing and gl_PointCoord) are included when calculating the storage requirements of fragment inputs. It doesn't say anything about `gl_Position` in the vertex shader, so maybe it doesn't count. So I think we want to sum up the number of rows used by each varying without any packing, only counting used varyings, and discounting gl_Position. Using that logic the numbers match whether or not this driver will successfully compile the shaders.
Bug 1695912 Comment 1 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
I cannot reproduce this on either of those URLs, and neither of them cause brush_blend to be compiled for me. However, if I enable gfx.webrender.precache-shaders then I can indeed reproduce. We seem to hit this problem in `brush_blend_ALPHA_PASS`, `cs_svg_filter`, and `brush_yuv_image_ALPHA_PASS_TEXTURE_2D_YUV`. The driver advertises itself as having 16 varying vectors available. It appers to be up to drivers precisely how to pack varyings in to those vectors but the [spec](https://www.khronos.org/registry/OpenGL/specs/es/3.0/GLSL_ES_Specification_3.00.pdf) does require a minimum required algorithm. Angle has an [implementation](https://searchfox.org/mozilla-central/rev/2b99ea2e97eef00a8a1c7e24e5fe51ab5304bc42/gfx/angle/checkout/src/compiler/translator/VariablePacker.cpp#395) of this algorithm to test whether packing should succeed, and it says that the shaders in question should indeed fit in to 16 vecs. So it appears the driver is not spec compliant. Working around the driver bug is trivial: we can manually pack a few varyings in to vecs. But the part I'm struggling with is how to test this and ensure we don't regress it in the future, given that this driver is not tested on CI. I need to try to reverse engineer what's going on in the driver, and then probably add some rust bindings to angle and test this in angle_shader_validation. My best guess at what's happening in the driver is that it doesn't do any packing whatsoever. If we sum up the number of rows for each varying without any packing, and also remove unused varyings from the count then the numbers *nearly* look correct: For cs_svg_image after I've manually packed some varyings so that it successfully compiles, this logic gives me 17 vecs in the vertex shader and 16 in the fragment shader. The vertex shader includes `gl_Position` but the fragment shader does not reference `gl_FragCoord`. The spec states that: > If referenced in the fragment shader (after preprocessing), the built-in special variables (gl_FragCoord, gl_FrontFacing and gl_PointCoord) are included when calculating the storage requirements of fragment inputs. It doesn't say anything about `gl_Position` in the vertex shader, so maybe it doesn't count. So I think we want to sum up the number of rows used by each varying without any packing, only counting used varyings, and discounting gl_Position. Using that logic the numbers match whether or not this driver will successfully compile the shaders.
I cannot reproduce this on either of those URLs, and neither of them cause brush_blend to be compiled for me. However, if I enable gfx.webrender.precache-shaders then I can indeed reproduce. We seem to hit this problem in `brush_blend_ALPHA_PASS`, `cs_svg_filter`, and `brush_yuv_image_ALPHA_PASS_TEXTURE_2D_YUV`. The driver advertises itself as having 16 varying vectors available. It appers to be up to drivers precisely how to pack varyings in to those vectors but the [spec](https://www.khronos.org/registry/OpenGL/specs/es/3.0/GLSL_ES_Specification_3.00.pdf) does require a minimum required algorithm. Angle has an [implementation](https://searchfox.org/mozilla-central/rev/2b99ea2e97eef00a8a1c7e24e5fe51ab5304bc42/gfx/angle/checkout/src/compiler/translator/VariablePacker.cpp#395) of this algorithm to test whether packing should succeed, and it says that the shaders in question should indeed fit in to 16 vecs. So it appears the driver is not spec compliant. Working around the driver bug is trivial: we can manually pack a few varyings in to vecs. But the part I'm struggling with is how to test this and ensure we don't regress it in the future, given that this driver is not tested on CI. I need to try to reverse engineer what's going on in the driver, and then probably add some rust bindings to angle and test this in angle_shader_validation. My best guess at what's happening in the driver is that it doesn't do any packing whatsoever. If we sum up the number of rows for each varying without any packing, and also remove unused varyings from the count then the numbers *nearly* look correct: For cs_svg_image after I've manually packed some varyings so that it successfully compiles, this logic gives me 17 vecs in the vertex shader and 16 in the fragment shader. The vertex shader includes `gl_Position` but the fragment shader does not reference `gl_FragCoord`. The spec states that: > If referenced in the fragment shader (after preprocessing), the built-in special variables (gl_FragCoord, gl_FrontFacing and gl_PointCoord) are included when calculating the storage requirements of fragment inputs. It doesn't say anything about `gl_Position` in the vertex shader, so maybe it doesn't count. So I think we want to sum up the number of rows used by each varying without any packing, only counting used varyings, and discounting gl_Position. Using that logic the numbers match whether or not this driver will successfully compile the shaders, for these shaders at least.