Shader Refactoring

[WestLangley]

Follow-on to #24752.

This post is intended as a basis for further discussion.

EDIT: It looks like we have agreement here. Yay!

[WestLangley]

@Mugen87 Some users may be assigning a PMREM as a background. It is fair to say that support for that is no longer needed now that blurry backgrounds are supported?

[Mugen87]

Yes, that makes sense. Users can now assign the "raw" or "unprocessed" environment map to Scene.background and let the renderer handle the background blurriness.

[WestLangley]

@Mugen87 I am reasonably-happy with this refactoring. Please let me know what you think.

[Mugen87]

Instead of using the term backgroundCube, what do you think about skybox instead?

[WestLangley]

OK, we can change it. How do you feel about blurredCube shader? That is the most descriptive name I can think of...

[Mugen87]

True, but I have the feeling skybox is what most 3D users understand of what the shader essentially represents. It's a common term in 3D engines.

However, the name is not something that blocks the PR for me. If we can't quickly agree on a name, we can merge the refactoring first and then discuss alternatives. backgroundCube isn't wrong after all.

[WestLangley]

We already have a skybox shader called 'cube' shader.

• 'cube': a skybox shader, used by post-processing, and other apps

• 'background': used by the renderer for scene.background (flat only)

• 'backgroundCube': used by the renderer for blurring scene.background (cube only)

The nomenclature is not a deal breaker for me, but let's revisit in a later PR. 😇

[WestLangley]

Some users may be assigning a PMREM as a background. Is it fair to say that support for that is no longer needed now that blurry backgrounds are supported?

Yes, that makes sense.

@Mugen87 FYI, with this PR, assigning a PMREM directly to scene.background is still supported. Furthermore, simultaneously setting scene.backgroundBlurriness will blur it.

I am open leaving that as-is, or removing it in a follow-on PR.

[WestLangley]

/ping @sunag just in case...

[Mugen87]

Let's merge this so we have it in r146.

[WestLangley]

Some users may be assigning a PMREM as a background. Is it fair to say that support for that is no longer needed now that blurry backgrounds are supported?

I have changed my mind on this. Some users may have only a PMREM. For example,

scene.environment = pmremGenerator.fromScene( new RoomEnvironment() ).texture;

scene.background = scene.environment; // assign PMREM to background

scene.backgroundBlurriness = 0.2;

So, as long as PMREM is used for blurring scene.background, I think we should continue to support assigning a PMREM as a scene background texture.

[Mugen87]

I've added a note in the migration guide for r146 about this change. There are some users directly using THREE.ShaderLib['cube'] and this PR broke their code, see:

https://stackoverflow.com/questions/75149020/property-envmap-removed-from-shader-cube-in-three-js-r146