MeshNormalNodeMaterial: Convert packed normal to working color space

[WestLangley]

Fixes #30575.

Also #28831 (comment).

By convention, the packed normal, when interpreted as a color, is assumed to be in sRGB color space. It must be converted to linear working color space.

[github-actions]

📦 Bundle size

Full ESM build, minified and gzipped.

	Before	After	Diff
WebGL	336.65 78.39	336.65 78.39	+0 B +0 B
WebGPU	522.37 144.98	522.33 145	-34 B +19 B
WebGPU Nodes	521.83 144.88	521.8 144.9	-34 B +19 B

🌳 Bundle size after tree-shaking

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

	Before	After	Diff
WebGL	465.7 112.26	465.7 112.26	+0 B +0 B
WebGPU	592.8 160.61	592.8 160.62	+7 B +7 B
WebGPU Nodes	547.93 150.06	547.93 150.06	+0 B +0 B

[Mugen87]

Do you mind removing the SRGBColorSpace from that line. toWorkingColorSpace() has just one parameter.

[WestLangley]

Oops... but can you explain how it converts to working color space without knowing what color space to convert from?

[Mugen87]

The function assumes the given color node is in output color space.

There is an alternative TSL function that is more flexible in that regard:

three.js/src/nodes/display/ColorSpaceNode.js

Lines 160 to 169 in 3bb6e70

	/**
	* TSL function for converting a given color node from the given color space to the current working color space.
	*
	* @tsl
	* @function
	* @param {Node} node - Represents the node to convert.
	* @param {string} colorSpace - The source color space.
	* @returns {ColorSpaceNode}
	*/
	export const colorSpaceToWorking = ( node, colorSpace ) => nodeObject( new ColorSpaceNode( nodeObject( node ), colorSpace, WORKING_COLOR_SPACE ) );

[WestLangley]

The function (toWorkingColorSpace) assumes the given color node is in output color space.

Then it should be called toWorkingColorSpaceFromOutputColorSpace.

Is "output color space" the renderer's output color space?... I guess that is true...

Then the inverse method should be toOutputColorSpaceFromWorkingColorSpace.

[WestLangley]

These are fine, since the source and destination are clear -- although I prefer using the more specific terms: targetColorSpace and sourceColorSpace.

three.js/src/nodes/display/ColorSpaceNode.js

Line 158 in 972f726

export const workingToColorSpace = ( node, colorSpace ) => nodeObject( new ColorSpaceNode( nodeObject( node ), WORKING_COLOR_SPACE, colorSpace ) );

three.js/src/nodes/display/ColorSpaceNode.js

Line 169 in 972f726

export const colorSpaceToWorking = ( node, colorSpace ) => nodeObject( new ColorSpaceNode( nodeObject( node ), colorSpace, WORKING_COLOR_SPACE ) );

//

These are also fine, for the same reason -- although I would rename them. and adhere to the same nomenclature as above: workingToColorSpace( color, targetColorSpace ) and colorSpaceToWorking( color, sourceColorSpace ).

three.js/src/math/ColorManagement.js

Lines 79 to 89 in 972f726

	fromWorkingColorSpace: function ( color, targetColorSpace ) {
	return this.convert( color, this.workingColorSpace, targetColorSpace );
	},
	toWorkingColorSpace: function ( color, sourceColorSpace ) {
	return this.convert( color, sourceColorSpace, this.workingColorSpace );
	},

//

But I think these should be removed, as is it not clear from the name what they do, and it is not clear to me why they are needed.

three.js/src/nodes/display/ColorSpaceNode.js

Line 137 in 972f726

export const toOutputColorSpace = ( node ) => nodeObject( new ColorSpaceNode( nodeObject( node ), WORKING_COLOR_SPACE, OUTPUT_COLOR_SPACE ) );

three.js/src/nodes/display/ColorSpaceNode.js

Line 147 in 972f726

export const toWorkingColorSpace = ( node ) => nodeObject( new ColorSpaceNode( nodeObject( node ), OUTPUT_COLOR_SPACE, WORKING_COLOR_SPACE ) );

/ping @gkjohnson

[gkjohnson]

But I think these should be removed, as is it not clear from the name what they do, and it is not clear to me why they are needed.

I haven't used nodes much yet but I tend to agree that more explicit functions are better and using functions that assume information about color spaces can make code unclear.

[WestLangley]

@Mugen87 When you get a chance, can you think about this a bit? 🙏

All we need are these two methods: workingToColorSpace() and colorSpaceToWorking() -- in both ColorSpaceNode.js and ColorManagement.js. This avoids having different nomenclatures for the same thing.

[Mugen87]

I'll try to revisit this issue when the JSDoc migration has been finished.

[Mugen87]

It seems a few examples using WebGPURenderer and a normal material require updated E2E screenshots.

[WestLangley]

It seems a few examples using WebGPURenderer and a normal material require updated E2E screenshots.

I have been unable to successfully update screenshots for some time now, unfortunately.

[Mugen87]

There is a trick you can use: If you open the failed CI job, you can actually download the actual and expected screenshots but only from the failed examples.

The artifact download link is at the bottom of the section "Upload Output Screenshots". If you now use the _actual screenshots from the downloaded zips and replace the contents in examples/screenshots, the CI should work again.