Skip to content

Conversation

@Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Sep 23, 2025

Fixed #31750.

Description

The PR makes sure TextureNode manages the y-flip as a uniform which allows to reuse a texture node with textures with different flip-y requirements.

WebGPU is not affected by this change. The new uniform value is only created when the builder requests it which is only true for WebGL.

@github-actions
Copy link

github-actions bot commented Sep 23, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 338.24
79.14
338.24
79.14
+0 B
+0 B
WebGPU 586.79
161.97
587.02
162.04
+238 B
+64 B
WebGPU Nodes 585.39
161.73
585.63
161.79
+238 B
+63 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 469.7
113.74
469.7
113.74
+0 B
+0 B
WebGPU 656.24
177.47
656.48
177.54
+238 B
+70 B
WebGPU Nodes 610.23
166.64
610.47
166.71
+238 B
+65 B

@Mugen87 Mugen87 marked this pull request as draft September 23, 2025 09:14
if ( this.sampler ) {

uvNode = uvNode.flipY();
uvNode = this._flipYUniform.equal( true ).select( uvNode.flipY(), uvNode );
Copy link
Collaborator Author

@Mugen87 Mugen87 Sep 23, 2025

Choose a reason for hiding this comment

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

Some examples fail e.g. webgpu_materials_matcap and it seems it has to do with the new select() statement. For some reasons, the compiled GLSL ends up wrong.

From SpectorJS:

nodeVar1 = bool( f_nodeUniform3 );
if ( ( nodeVar1 == true ) ) {
    positionViewDirection = normalize( v_positionViewDirection );
    nodeVar2 = normalize( vec3( positionViewDirection.z, 0.0, ( - positionViewDirection.x ) ) );
    normalViewGeometry = normalize( v_normalViewGeometry );
    NORMAL_normalView = normalViewGeometry;
    normalView = NORMAL_normalView;
    matcapUV = ( ( vec2( dot( nodeVar2, normalView ), dot( cross( positionViewDirection, nodeVar2 ), normalView ) ) * vec2( 0.495 ) ) + vec2( 0.5 ) );
    nodeVar3 = ( f_nodeUniform4 * vec3( matcapUV, 1.0 ) ).xy;
    nodeVar0 = vec2( nodeVar3.x, 1.0 - nodeVar3.y );
}
else {
    matcapUV = ( ( vec2( dot( nodeVar2, normalView ), dot( cross( positionViewDirection, nodeVar2 ), normalView ) ) * vec2( 0.495 ) ) + vec2( 0.5 ) );
    nodeVar0 = ( f_nodeUniform4 * vec3( matcapUV, 1.0 ) ).xy;
}

Um, it seems the if/else statement from the select holds way too much code.

Copy link
Contributor

@cmhhelgeson cmhhelgeson Sep 23, 2025

Choose a reason for hiding this comment

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

I think in the short-term this could potentially be resolved by using the uniformFlow context in your highlighted line of code, but the default behavior is certainly not correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

I did a short test and got this as output in the fragment shader. There's probably some optimizations that can be done to only evaluate the f_nodeUniform4 * matcapUV operation once...

nodeVar17 = texture( nodeUniform2, ( nodeVar1 == true ) ? vec2( nodeVar16.x, 1.0 - nodeVar16.y ) : ( f_nodeUniform4 * vec3( matcapUV, 1.0 ) ).xy );

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How did you update the TSL source to get this output?

Copy link
Contributor

Choose a reason for hiding this comment

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

this._flipYUniform.equal( true ).select( uvNode.flipY(), uvNode ).uniformFlow();

Copy link
Collaborator Author

@Mugen87 Mugen87 Sep 24, 2025

Choose a reason for hiding this comment

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

Thanks! I like the fact that the if/else blocks are replaced with a ternary operation.

Unfortunately, none of the suggested versions work right now. Both the uniformFlow() code as well as the uvNode.toVar() definitions break environment maps in different ways.

E.g. add uniformFlow() and then test webgpu_loader_gltf_transmission or webgpu_loader_gltf_sheen. The environment maps end up way too bright.

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Mugen87 I'm checking this bug, the caching and optimization system is failing at some point in the conditionals.

@sunag sunag marked this pull request as ready for review September 25, 2025 17:03
@sunag sunag added this to the r181 milestone Sep 25, 2025
@sunag sunag merged commit c91c019 into mrdoob:dev Sep 26, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WebGPU: UV flips Y on WebGL fallback when a texture node is reused

3 participants