chore: harden color management tests

pmndrs · Apr 18, 2023 · ccc52af · ccc52af
1 parent adbea92
commit ccc52af
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 19 deletions.
diff --git a/packages/fiber/src/core/utils.ts b/packages/fiber/src/core/utils.ts
@@ -339,19 +339,6 @@ export function applyProps(instance: Instance, data: InstanceProps | DiffSet) {
         key = 'outputColorSpace'
         value = value === sRGBEncoding ? SRGBColorSpace : LinearSRGBColorSpace
       }
-
-      // Auto-convert sRGB textures, for now ...
-      // https://github.com/pmndrs/react-three-fiber/issues/344
-    } else if (
-      !rootState.linear &&
-      currentInstance[key] instanceof THREE.Texture &&
-      // sRGB textures must be RGBA8 since r137 https://github.com/mrdoob/three.js/pull/23129
-      currentInstance[key].format === THREE.RGBAFormat &&
-      currentInstance[key].type === THREE.UnsignedByteType
-    ) {
-      const texture = currentInstance[key] as THREE.Texture
-      if (hasColorSpace(texture) && hasColorSpace(rootState.gl)) texture.colorSpace = rootState.gl.outputColorSpace
-      else texture.encoding = rootState.gl.outputEncoding
     }
 
     // Deal with pointer events ...
@@ -394,6 +381,19 @@ export function applyProps(instance: Instance, data: InstanceProps | DiffSet) {
       // Else, just overwrite the value
     } else {
       currentInstance[key] = value
+
+      // Auto-convert sRGB textures, for now ...
+      // https://github.com/pmndrs/react-three-fiber/issues/344
+      if (
+        currentInstance[key] instanceof THREE.Texture &&
+        // sRGB textures must be RGBA8 since r137 https://github.com/mrdoob/three.js/pull/23129
+        currentInstance[key].format === THREE.RGBAFormat &&
+        currentInstance[key].type === THREE.UnsignedByteType
+      ) {
+        const texture = currentInstance[key] as THREE.Texture
+        if (hasColorSpace(texture) && hasColorSpace(rootState.gl)) texture.colorSpace = rootState.gl.outputColorSpace
+        else texture.encoding = rootState.gl.outputEncoding
+      }
     }
 
     invalidateInstance(instance)

diff --git a/packages/fiber/tests/core/renderer.test.tsx b/packages/fiber/tests/core/renderer.test.tsx
@@ -740,31 +740,40 @@ describe('renderer', () => {
   })
 
   it('should respect color management preferences via gl', async () => {
+    const texture = new THREE.Texture() as THREE.Texture & { colorSpace?: string }
+    let key = 0
+    function Test() {
+      return <meshBasicMaterial key={key++} map={texture} />
+    }
+
     const LinearEncoding = 3000
     const sRGBEncoding = 3001
 
     let gl: THREE.WebGLRenderer & { outputColorSpace?: string } = null!
-    await act(async () => (gl = root.render(<group />).getState().gl))
+    await act(async () => (gl = root.render(<Test />).getState().gl))
     expect(gl.outputEncoding).toBe(sRGBEncoding)
     expect(gl.toneMapping).toBe(THREE.ACESFilmicToneMapping)
+    expect(texture.encoding).toBe(sRGBEncoding)
 
-    await act(async () =>
-      root.configure({ gl: { outputEncoding: LinearEncoding, toneMapping: THREE.NoToneMapping } }).render(<group />),
-    )
+    await act(async () => root.configure({ linear: true, flat: true }).render(<Test />))
     expect(gl.outputEncoding).toBe(LinearEncoding)
     expect(gl.toneMapping).toBe(THREE.NoToneMapping)
+    expect(texture.encoding).toBe(LinearEncoding)
 
     // Sets outputColorSpace since r152
     const SRGBColorSpace = 'srgb'
     const LinearSRGBColorSpace = 'srgb-linear'
 
     gl.outputColorSpace ??= ''
+    texture.colorSpace ??= ''
 
-    await act(async () => root.configure({ linear: true }).render(<group />))
+    await act(async () => root.configure({ linear: true }).render(<Test />))
     expect(gl.outputColorSpace).toBe(LinearSRGBColorSpace)
+    expect(texture.colorSpace).toBe(LinearSRGBColorSpace)
 
-    await act(async () => root.configure({ linear: false }).render(<group />))
+    await act(async () => root.configure({ linear: false }).render(<Test />))
     expect(gl.outputColorSpace).toBe(SRGBColorSpace)
+    expect(texture.colorSpace).toBe(SRGBColorSpace)
   })
 
   it('should respect legacy prop', async () => {