Repository navigation
GLEmulation: divide texture coordinates by q when it can differ from 1 - #28063
TheTomBritton wants to merge 1 commit into
Conversation
GL divides the s and t texture coordinates by q before the lookup. The generated fixed-function shaders sampled with texture2D(coord.xy) and ignored q, which is wrong for four-component texture coordinates (for example (s*q, t*q, 0, q) for perspective-correct texturing) and for texture matrices whose last row is not (0, 0, 0, 1), even with two- or three-component coordinates. Decide per draw which texture units need the divide: those whose texture coordinate array has four components, or whose texture matrix (when one is in use) has a projective last row. Those units sample with texture2DProj; everything else keeps the cheaper texture2D. The mask is part of the renderer key, and getRenderer re-checks it on every draw, since a texture matrix can become projective without any of the state changes that make it look the renderer up again. User shader programs are unaffected. Cube maps are left as they were. Adds a browser test with three bands: an affine draw, the same draw after only the texture matrix changed to double q, and four-component texture coordinates whose q runs from 1 to 3 across the screen. The last two fail before this change and all three pass after it. Fixes emscripten-core#27947 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UR3bCLyLBQiGtevGJx2ARt
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the fixed-function fragment shader generation in LEGACY_GL_EMULATION to correctly handle projective texture lookups by dividing texture coordinates by q when necessary. This is achieved by tracking projective texture units and updating the shader generation logic to use texture2DProj instead of texture2D when q can differ from 1. A regression test was added to verify the fix. The review feedback identified a potential runtime error and suggested more robust handling of matrix access and floating-point comparisons.
| var m = GLImmediate.matrix[2/*t*/ + i]; | ||
| if ((attr && attr.size == 4) || | ||
| (GLImmediate.useTextureMatrix && (m[3] || m[7] || m[11] || m[15] != 1))) { |
There was a problem hiding this comment.
Directly accessing properties of m can cause a runtime TypeError if GLImmediate.matrix is not fully initialized or if m is undefined for texture unit i. Additionally, the expression (m[3] || m[7] || m[11] || m[15] != 1) relies on implicit truthy/falsy coercion of floating-point values, which is less clear and can be fragile.\n\nWe should safely retrieve m and perform explicit comparisons against 0 and 1.
var m = GLImmediate.matrix && GLImmediate.matrix[2/*t*/ + i];\n if ((attr && attr.size == 4) ||\n (GLImmediate.useTextureMatrix && m && (m[3] !== 0 || m[7] !== 0 || m[11] !== 0 || m[15] !== 1))) {
Fixes #27947.
Fixed-function GL samples at
s/q, t/q; the generated samplers ignoredq, so homogeneous texture coordinates (glTexCoordPointer(4, ...)withq = 1/w, the GL 1.x technique for perspective-correct texturing) drew wrong.Runtime detection turned out to be modest, with one catch: the component count alone isn't enough, because a texture matrix whose last row isn't (0,0,0,1) also makes
q != 1with 2-component coordinates. Each draw now works out which units needq- a 4-component texcoord array, or a projective texture-matrix last row - and that goes into the renderer key and is re-checked per draw, since a matrix change alone doesn't trigger a renderer lookup. Only those units usetexture2DProj; everything else keepstexture2Dand its cost.Adds
test_gl_texcoord_q, covering 4-component perspective coordinates and a matrix-only change between two draws. Both cases fail before the change and pass after it on Chrome/ANGLE (Metal and SwiftShader); the other LEGACY_GL_EMULATION browser tests pass locally. If you would rather have the simpler unconditionaltexture2DProjwith your performance note, it is a one-line change and I'm happy to switch.Side finding, not fixed here: fixed-function
textureCubegets a.xycoordinate, which isn't valid GLSL.🤖 Generated with Claude Code
https://claude.ai/code/session_01AFhyBALrPm8CSkyHzQizDH