[solved] Fragment shader not working anymore in r114

Questions
1

Upgrading from r113 to r114 suddenly a fragment shader is not working anymore and is causing the full scene to break. It all comes down to a single line inside the fragment shader code (see comment after line below).

static getFragmentShader() {
        return [
            'uniform vec3 diffuse;',

            'struct DirectionalLight {',
                'vec3 direction;',
                'vec3 color;',
                'int shadow;',
                'float shadowBias;',
                'float shadowRadius;',
                'vec2 shadowMapSize;',
            '};',
            'uniform DirectionalLight directionalLights[NUM_DIR_LIGHTS];',

            'void main() { ',
                'vec4 addedLights = vec4(0.0, 0.0, 0.0, 1.0);',

                'for (int i = 0; i < NUM_DIR_LIGHTS; i++) {',
                    'addedLights.rgb += directionalLights[i].color;', // LOOKS LIKE THIS LINE IS THE ISSUE
                '}',

                'gl_FragColor = vec4(diffuse, 1.0) * addedLights * 1.5;',
            '}',
        ].join('\n');
}

I can’t find anything in the migration part about fragment shaders that have changed. Neither about DirectionalLights, but I might be overlooking something?

I think this is pretty difficult to debug as we don’t have any console log inside shader code or whatever. But it’s clearly no problem in WebGL being different now, as the old code using threejs r113 still works perfectly fine on the same machine and with the exact same shader code. So something must have changed in threejs from 113 to 114 causing this.

So the problem seems to be inside the line 'addedLights.rgb += directionalLights[i].color;'. When I replace directionalLights[i].color by just a vec3(1.0, 1.0, 1.0) there is no problem. So addedLights.rgb looks fine and is a vec3 (as we might expect). So the problem seem to be in the directionalLights[i].color.
The directionalLight exists, otherwise it wouldn’t be inside the loop. So there seem to be something off with the .color thing here.

I tried changing the color property inside the DirectionalLight struct into a vec4 type, as I could imagine the color could has been changed into having opacity as well. But that doesn’t seem to be the case as it doesn’t solve the issue.

I am quite lost at this point as I don’t know any way to debug this. Like how can we know what values are actually inside the directionalLights[i].color? It really looks like the issue is inside that value, or the DirectionalLight struct have been changed in r114. But where can I find the right way of doing this in r114?

Anybody could help me with this issue? Give me some clue on how to debug these values? Maybe somebody knows something that has been changed in threejs r114 causing this?

A little help would be appreciated a lot here! :slight_smile:

3

There’s an undocumented change in all light structs.
Light uniforms no longer include the shadow information in the same struct.

Use the following code, and everything should go back to normal:

struct DirectionalLight {
	vec3 direction;	
	vec3 color;
}

FYI, these changes happened on PR #18627

And the main reason why it wasn’t documented is because this an internal change and doesn’t directly reflect on the public API for the user.

2 Likes
4

Thanks a lot @sciecode ! That totally worked, so as you wrote I only needed to remove the three lines of shadow-code inside the DirectionalLight struct to make it work!

Nice to know now!

BTW, I don’t really understand why this would not be documented in the changelog as the ShaderMaterial still is part of the public API. If you weren’t here with this answer it probably would have taken a long time for me to figure out what have been changed. But hey, at least now I know! Thanks again! Another project upgraded to r114 now (and this was a major project), so this made my day! :slight_smile:

5

I kinda agree that maybe it should be, but at the same time I understand why it is not.

The thing is that three.js is really good at having different built-in solutions for many common problems, but when it comes down to adapting or modifying these solutions it is not. One could argue that, although ShaderMaterial can utilize the built-in uniforms, it is not fully supported or documented by Three.js.

If you wish to modify or adapt materials, it is expected that you understand how each ShaderChunk behaves and how they are integrated on the ShaderLib Materials. It expects a certain level of familiarity with the internal shaders and material creation.

relevant - Issue #17088

1 Like
6

@sciecode I understand that this is a lower level of coding that needs a better understanding. But I had that understanding when writing these shaders and I still believe it would help us if these breaking changes were in the changelog. Because when they are not and something is broken by just upgrading from r113 to r114, we are completely blind. The first thing I do than is read the changelog again, but if nothing is in there it’s could really be everything from that point as I think we should be able to trust the changelog in the first place. I would logically think that if something is not in the changelog, it isn’t changed. What’s the use of having a changelog then? It’s not fun to walk through all threejs code ourselves when just upgrading, just to debug what have been changed causing issues. But maybe I just missed something in that changelog that should point me in that direction still? I don’t know.

7

Oh by the way, I’m not implying you don’t have the understanding. I’m just explaining that it is expected that someone has that understanding in order to adapt the shaders.

The problem lies in what you are considering to be a breaking change and what Three.js considers to be a breaking change. A custom shader is by definition custom made, there’s no way of knowing which internal changes would break someone’s custom shader. Therefore, it is not treated as breaking change.

Three.js only documents changes on the public API, the shader internals are not considered public API. In other words, if you decide to write a shader that utilizes the internal uniforms three.js exposes, it is on you to make sure these were updated or not.

Not to mention that there are loads of shader changes happening on a regular basis, mentioning these in the changelog would only serve to clutter and make the important changes less visible.

Edit: With that being said, if we had a ShaderChunk documentation. We could use those pages to reflect the changes done to these internal shaders and make it a lot easier for users to determine if changes occured or not. But creating these is a lot of work, I can’t deny that.

Edit2: Also relevant, the Release page contains all the PRs that were merged during the last release cycle, although not direct, it may help narrowing down on breaking changes.

1 Like
8

Oh by the way, I’m not implying you don’t have the understanding. I’m just explaining that it is expected that someone has that understanding in order to adapt the shaders.

@sciecode I didn’t take it that way, but thanks for clearing that up. Appreciated.

About what you wrote about the changelog; I understand your point.

1 Like