From a6e1e3b41898e9d93c0fa9552544fa3ab95398d0 Mon Sep 17 00:00:00 2001 From: Noeri Huisman <8823461+mrxz@users.noreply.github.com> Date: Mon, 8 May 2023 18:46:55 +0200 Subject: [PATCH] Set THREE.ColorManagement.legacyMode to false when using color management (#5210) Co-authored-by: Noeri Huisman --- .../generic-tracked-controller-controls.js | 1 - src/components/hp-mixed-reality-controls.js | 1 - src/components/light.js | 6 ------ src/components/line.js | 3 --- src/components/magicleap-controls.js | 1 - src/components/obj-model.js | 2 -- src/components/oculus-go-controls.js | 2 -- src/components/oculus-touch-controls.js | 2 -- src/components/scene/screenshot.js | 4 ++-- src/components/valve-index-controls.js | 1 - src/components/vive-controls.js | 5 ----- src/shaders/flat.js | 3 --- src/shaders/phong.js | 3 --- src/shaders/shadow.js | 1 - src/shaders/standard.js | 5 ----- src/systems/renderer.js | 21 +++++++------------ tests/components/scene/screenshot.test.js | 10 ++++----- tests/systems/renderer.test.js | 9 ++++---- 18 files changed, 19 insertions(+), 61 deletions(-) diff --git a/src/components/generic-tracked-controller-controls.js b/src/components/generic-tracked-controller-controls.js index 11562e51b63..f9c50f7aa2b 100644 --- a/src/components/generic-tracked-controller-controls.js +++ b/src/components/generic-tracked-controller-controls.js @@ -67,7 +67,6 @@ module.exports.Component = registerComponent('generic-tracked-controller-control this.controllerPresent = false; this.wasControllerConnected = false; this.lastControllerCheck = 0; - this.rendererSystem = this.el.sceneEl.systems.renderer; this.bindMethods(); // generic-tracked-controller-controls has the lowest precedence. diff --git a/src/components/hp-mixed-reality-controls.js b/src/components/hp-mixed-reality-controls.js index ffbde34b627..5848745a3ae 100644 --- a/src/components/hp-mixed-reality-controls.js +++ b/src/components/hp-mixed-reality-controls.js @@ -61,7 +61,6 @@ module.exports.Component = registerComponent('hp-mixed-reality-controls', { this.onButtonTouchEnd = function (evt) { onButtonEvent(evt.detail.id, 'touchend', self, self.data.hand); }; this.onButtonTouchStart = function (evt) { onButtonEvent(evt.detail.id, 'touchstart', self, self.data.hand); }; this.previousButtonValues = {}; - this.rendererSystem = this.el.sceneEl.systems.renderer; this.bindMethods(); }, diff --git a/src/components/light.js b/src/components/light.js index 6c6286b1c91..0f387ed3f86 100644 --- a/src/components/light.js +++ b/src/components/light.js @@ -56,7 +56,6 @@ module.exports.Component = registerComponent('light', { var el = this.el; this.light = null; this.defaultTarget = null; - this.rendererSystem = this.el.sceneEl.systems.renderer; this.system.registerLight(el); }, @@ -67,7 +66,6 @@ module.exports.Component = registerComponent('light', { var data = this.data; var diffData = diff(data, oldData); var light = this.light; - var rendererSystem = this.rendererSystem; var self = this; // Existing light. @@ -80,13 +78,11 @@ module.exports.Component = registerComponent('light', { switch (key) { case 'color': { light.color.set(value); - rendererSystem.applyColorCorrection(light.color); break; } case 'groundColor': { light.groundColor.set(value); - rendererSystem.applyColorCorrection(light.groundColor); break; } @@ -281,12 +277,10 @@ module.exports.Component = registerComponent('light', { getLight: function (data) { var angle = data.angle; var color = new THREE.Color(data.color); - this.rendererSystem.applyColorCorrection(color); color = color.getHex(); var decay = data.decay; var distance = data.distance; var groundColor = new THREE.Color(data.groundColor); - this.rendererSystem.applyColorCorrection(groundColor); groundColor = groundColor.getHex(); var intensity = data.intensity; var type = data.type; diff --git a/src/components/line.js b/src/components/line.js index 452af93a090..3696e7e54d1 100644 --- a/src/components/line.js +++ b/src/components/line.js @@ -16,7 +16,6 @@ module.exports.Component = registerComponent('line', { var data = this.data; var geometry; var material; - this.rendererSystem = this.el.sceneEl.systems.renderer; material = this.material = new THREE.LineBasicMaterial({ color: data.color, opacity: data.opacity, @@ -26,7 +25,6 @@ module.exports.Component = registerComponent('line', { geometry = this.geometry = new THREE.BufferGeometry(); geometry.setAttribute('position', new THREE.BufferAttribute(new Float32Array(2 * 3), 3)); - this.rendererSystem.applyColorCorrection(material.color); this.line = new THREE.Line(geometry, material); this.el.setObject3D(this.attrName, this.line); }, @@ -59,7 +57,6 @@ module.exports.Component = registerComponent('line', { } material.color.setStyle(data.color); - this.rendererSystem.applyColorCorrection(material.color); material.opacity = data.opacity; material.transparent = data.opacity < 1; material.visible = data.visible; diff --git a/src/components/magicleap-controls.js b/src/components/magicleap-controls.js index ee926f58cfe..6e0b687ac58 100644 --- a/src/components/magicleap-controls.js +++ b/src/components/magicleap-controls.js @@ -56,7 +56,6 @@ module.exports.Component = registerComponent('magicleap-controls', { this.onButtonTouchEnd = function (evt) { onButtonEvent(evt.detail.id, 'touchend', self); }; this.onButtonTouchStart = function (evt) { onButtonEvent(evt.detail.id, 'touchstart', self); }; this.previousButtonValues = {}; - this.rendererSystem = this.el.sceneEl.systems.renderer; this.bindMethods(); }, diff --git a/src/components/obj-model.js b/src/components/obj-model.js index dcf4642c9a4..55ae9f6facc 100644 --- a/src/components/obj-model.js +++ b/src/components/obj-model.js @@ -64,9 +64,7 @@ module.exports.Component = registerComponent('obj-model', { self.model.traverse(function (object) { if (object.isMesh) { var material = object.material; - if (material.color) rendererSystem.applyColorCorrection(material.color); if (material.map) rendererSystem.applyColorCorrection(material.map); - if (material.emissive) rendererSystem.applyColorCorrection(material.emissive); if (material.emissiveMap) rendererSystem.applyColorCorrection(material.emissiveMap); } }); diff --git a/src/components/oculus-go-controls.js b/src/components/oculus-go-controls.js index 2798253e273..5f5ee9b62ff 100644 --- a/src/components/oculus-go-controls.js +++ b/src/components/oculus-go-controls.js @@ -83,7 +83,6 @@ module.exports.Component = registerComponent('oculus-go-controls', { this.onButtonTouchEnd = function (evt) { onButtonEvent(evt.detail.id, 'touchend', self); }; this.controllerPresent = false; this.lastControllerCheck = 0; - this.rendererSystem = this.el.sceneEl.systems.renderer; this.bindMethods(); }, @@ -197,6 +196,5 @@ module.exports.Component = registerComponent('oculus-go-controls', { } button = buttonMeshes[buttonName]; button.material.color.set(color); - this.rendererSystem.applyColorCorrection(button.material.color); } }); diff --git a/src/components/oculus-touch-controls.js b/src/components/oculus-touch-controls.js index f92baec874e..e0b8e1114bf 100644 --- a/src/components/oculus-touch-controls.js +++ b/src/components/oculus-touch-controls.js @@ -197,7 +197,6 @@ module.exports.Component = registerComponent('oculus-touch-controls', { this.controllerPresent = false; this.lastControllerCheck = 0; this.previousButtonValues = {}; - this.rendererSystem = this.el.sceneEl.systems.renderer; this.bindMethods(); this.triggerEuler = new THREE.Euler(); }, @@ -501,7 +500,6 @@ module.exports.Component = registerComponent('oculus-touch-controls', { color = (state === 'up' || state === 'touchend') ? buttonMeshes[buttonName].originalColor || this.data.buttonColor : state === 'touchstart' ? this.data.buttonTouchColor : this.data.buttonHighlightColor; button = buttonMeshes[buttonName]; button.material.color.set(color); - this.rendererSystem.applyColorCorrection(button.material.color); } } }); diff --git a/src/components/scene/screenshot.js b/src/components/scene/screenshot.js index 72ad39a64f8..f221c133b97 100644 --- a/src/components/scene/screenshot.js +++ b/src/components/scene/screenshot.js @@ -85,7 +85,7 @@ module.exports.Component = registerComponent('screenshot', { getRenderTarget: function (width, height) { return new THREE.WebGLRenderTarget(width, height, { - encoding: this.el.sceneEl.renderer.outputEncoding, + colorSpace: this.el.sceneEl.renderer.outputColorSpace, minFilter: THREE.LinearFilter, magFilter: THREE.LinearFilter, wrapS: THREE.ClampToEdgeWrapping, @@ -153,7 +153,7 @@ module.exports.Component = registerComponent('screenshot', { format: THREE.RGBFormat, generateMipmaps: true, minFilter: THREE.LinearMipmapLinearFilter, - encoding: THREE.sRGBEncoding + colorSpace: THREE.SRGBColorSpace }); // Create cube camera and copy position from scene camera. cubeCamera = new THREE.CubeCamera(el.camera.near, el.camera.far, cubeRenderTarget); diff --git a/src/components/valve-index-controls.js b/src/components/valve-index-controls.js index d1bbbc82e4f..dab3b9ef016 100644 --- a/src/components/valve-index-controls.js +++ b/src/components/valve-index-controls.js @@ -73,7 +73,6 @@ module.exports.Component = registerComponent('valve-index-controls', { this.onButtonTouchEnd = function (evt) { onButtonEvent(evt.detail.id, 'touchend', self); }; this.onButtonTouchStart = function (evt) { onButtonEvent(evt.detail.id, 'touchstart', self); }; this.previousButtonValues = {}; - this.rendererSystem = this.el.sceneEl.systems.renderer; this.bindMethods(); }, diff --git a/src/components/vive-controls.js b/src/components/vive-controls.js index dd3dbc2854d..72bd9789174 100644 --- a/src/components/vive-controls.js +++ b/src/components/vive-controls.js @@ -77,7 +77,6 @@ module.exports.Component = registerComponent('vive-controls', { this.onButtonTouchEnd = function (evt) { onButtonEvent(evt.detail.id, 'touchend', self); }; this.onButtonTouchStart = function (evt) { onButtonEvent(evt.detail.id, 'touchstart', self); }; this.previousButtonValues = {}; - this.rendererSystem = this.el.sceneEl.systems.renderer; this.bindMethods(); }, @@ -241,7 +240,6 @@ module.exports.Component = registerComponent('vive-controls', { setButtonColor: function (buttonName, color) { var buttonMeshes = this.buttonMeshes; - var rendererSystem = this.rendererSystem; if (!buttonMeshes) { return; } @@ -249,11 +247,8 @@ module.exports.Component = registerComponent('vive-controls', { if (buttonName === 'grip') { buttonMeshes.grip.left.material.color.set(color); buttonMeshes.grip.right.material.color.set(color); - rendererSystem.applyColorCorrection(buttonMeshes.grip.left.material.color); - rendererSystem.applyColorCorrection(buttonMeshes.grip.right.material.color); return; } buttonMeshes[buttonName].material.color.set(color); - rendererSystem.applyColorCorrection(buttonMeshes[buttonName].material.color); } }); diff --git a/src/shaders/flat.js b/src/shaders/flat.js index 72cd9887cbc..73df91aeaac 100755 --- a/src/shaders/flat.js +++ b/src/shaders/flat.js @@ -24,11 +24,9 @@ module.exports.Shader = registerShader('flat', { * Adds a reference from the scene to this entity as the camera. */ init: function (data) { - this.rendererSystem = this.el.sceneEl.systems.renderer; this.materialData = {color: new THREE.Color()}; this.textureSrc = null; getMaterialData(data, this.materialData); - this.rendererSystem.applyColorCorrection(this.materialData.color); this.material = new THREE.MeshBasicMaterial(this.materialData); }, @@ -45,7 +43,6 @@ module.exports.Shader = registerShader('flat', { updateMaterial: function (data) { var key; getMaterialData(data, this.materialData); - this.rendererSystem.applyColorCorrection(this.materialData.color); for (key in this.materialData) { this.material[key] = this.materialData[key]; } diff --git a/src/shaders/phong.js b/src/shaders/phong.js index 924f6fc767e..14054c0b95c 100644 --- a/src/shaders/phong.js +++ b/src/shaders/phong.js @@ -52,11 +52,9 @@ module.exports.Shader = registerShader('phong', { * Adds a reference from the scene to this entity as the camera. */ init: function (data) { - this.rendererSystem = this.el.sceneEl.systems.renderer; this.materialData = { color: new THREE.Color(), specular: new THREE.Color(), emissive: new THREE.Color() }; this.textureSrc = null; getMaterialData(data, this.materialData); - this.rendererSystem.applyColorCorrection(this.materialData.color); this.material = new THREE.MeshPhongMaterial(this.materialData); utils.material.updateMap(this, data); }, @@ -79,7 +77,6 @@ module.exports.Shader = registerShader('phong', { updateMaterial: function (data) { var key; getMaterialData(data, this.materialData); - this.rendererSystem.applyColorCorrection(this.materialData.color); for (key in this.materialData) { this.material[key] = this.materialData[key]; } diff --git a/src/shaders/shadow.js b/src/shaders/shadow.js index 09ffcc883a5..d6ff730a45e 100644 --- a/src/shaders/shadow.js +++ b/src/shaders/shadow.js @@ -16,7 +16,6 @@ module.exports.Shader = registerShader('shadow', { * Adds a reference from the scene to this entity as the camera. */ init: function (data) { - this.rendererSystem = this.el.sceneEl.systems.renderer; this.material = new THREE.ShadowMaterial(); }, diff --git a/src/shaders/standard.js b/src/shaders/standard.js index 9a08dd827ff..1b4cb61d278 100755 --- a/src/shaders/standard.js +++ b/src/shaders/standard.js @@ -61,11 +61,8 @@ module.exports.Shader = registerShader('standard', { * Adds a reference from the scene to this entity as the camera. */ init: function (data) { - this.rendererSystem = this.el.sceneEl.systems.renderer; this.materialData = {color: new THREE.Color(), emissive: new THREE.Color()}; getMaterialData(data, this.materialData); - this.rendererSystem.applyColorCorrection(this.materialData.color); - this.rendererSystem.applyColorCorrection(this.materialData.emissive); this.material = new THREE.MeshStandardMaterial(this.materialData); }, @@ -90,8 +87,6 @@ module.exports.Shader = registerShader('standard', { var key; var material = this.material; getMaterialData(data, this.materialData); - this.rendererSystem.applyColorCorrection(this.materialData.color); - this.rendererSystem.applyColorCorrection(this.materialData.emissive); for (key in this.materialData) { material[key] = this.materialData[key]; } diff --git a/src/systems/renderer.js b/src/systems/renderer.js index 3765db5a7db..3b36851e831 100644 --- a/src/systems/renderer.js +++ b/src/systems/renderer.js @@ -20,8 +20,7 @@ module.exports.System = registerSystem('renderer', { toneMapping: {default: 'no', oneOf: ['no', 'ACESFilmic', 'linear', 'reinhard', 'cineon']}, precision: {default: 'high', oneOf: ['high', 'medium', 'low']}, sortObjects: {default: false}, - colorManagement: {default: false}, - gammaOutput: {default: false}, + colorManagement: {default: true}, alpha: {default: true}, foveationLevel: {default: 1} }, @@ -36,12 +35,8 @@ module.exports.System = registerSystem('renderer', { renderer.physicallyCorrectLights = data.physicallyCorrectLights; renderer.toneMapping = THREE[toneMappingName + 'ToneMapping']; - if (data.colorManagement || data.gammaOutput) { - renderer.outputEncoding = THREE.sRGBEncoding; - if (data.gammaOutput) { - warn('Property `gammaOutput` is deprecated. Use `renderer="colorManagement: true;"` instead.'); - } - } + THREE.ColorManagement.enabled = data.colorManagement; + renderer.outputColorSpace = data.colorManagement ? THREE.SRGBColorSpace : THREE.LinearSRGBColorSpace; if (sceneEl.hasAttribute('antialias')) { warn('Component `antialias` is deprecated. Use `renderer="antialias: true"` instead.'); @@ -62,13 +57,11 @@ module.exports.System = registerSystem('renderer', { renderer.xr.setFoveation(data.foveationLevel); }, - applyColorCorrection: function (colorOrTexture) { - if (!this.data.colorManagement || !colorOrTexture) { + applyColorCorrection: function (texture) { + if (!this.data.colorManagement || !texture) { return; - } else if (colorOrTexture.isColor) { - colorOrTexture.convertSRGBToLinear(); - } else if (colorOrTexture.isTexture) { - colorOrTexture.encoding = THREE.sRGBEncoding; + } else if (texture.isTexture) { + texture.colorSpace = THREE.SRGBColorSpace; } }, diff --git a/tests/components/scene/screenshot.test.js b/tests/components/scene/screenshot.test.js index 3241e9fd6ef..6cb0548a343 100644 --- a/tests/components/scene/screenshot.test.js +++ b/tests/components/scene/screenshot.test.js @@ -3,9 +3,9 @@ suite('screenshot', function () { var component; var sceneEl; - function checkRenderTarget (renderTarget, encoding) { + function checkRenderTarget (renderTarget, colorSpace) { const texture = renderTarget.texture; - assert.equal(texture.encoding, encoding); + assert.equal(texture.colorSpace, colorSpace); assert.equal(texture.minFilter, THREE.LinearFilter); assert.equal(texture.magFilter, THREE.LinearFilter); assert.equal(texture.wrapS, THREE.ClampToEdgeWrapping); @@ -38,20 +38,20 @@ suite('screenshot', function () { }); test('capture renders screenshot correctly (w/o Color Management)', function () { + sceneEl.setAttribute('renderer', 'colorManagement: false'); sceneEl.addEventListener('loaded', () => { component = sceneEl.components.screenshot; const renderTarget = component.getRenderTarget(); - checkRenderTarget(renderTarget, THREE.LinearEncoding); + checkRenderTarget(renderTarget, THREE.LinearSRGBColorSpace); }); document.body.appendChild(sceneEl); }); test('capture renders screenshot correctly (w/ Color Management)', function () { - sceneEl.setAttribute('renderer', 'colorManagement: true'); sceneEl.addEventListener('loaded', () => { component = sceneEl.components.screenshot; const renderTarget = component.getRenderTarget(); - checkRenderTarget(renderTarget, THREE.sRGBEncoding); + checkRenderTarget(renderTarget, THREE.SRGBColorSpace); }); document.body.appendChild(sceneEl); }); diff --git a/tests/systems/renderer.test.js b/tests/systems/renderer.test.js index 84c7aeb1a68..21a9e4bd6a4 100644 --- a/tests/systems/renderer.test.js +++ b/tests/systems/renderer.test.js @@ -16,10 +16,11 @@ suite('renderer', function () { assert.strictEqual(rendererSystem.highRefreshRate, false); assert.strictEqual(rendererSystem.physicallyCorrectLights, false); assert.strictEqual(rendererSystem.sortObjects, false); + assert.strictEqual(rendererSystem.colorManagement, true); // Verify properties that are transferred from the renderer system to the rendering engine. var renderingEngine = sceneEl.renderer; - assert.notOk(renderingEngine.outputEncoding); + assert.strictEqual(renderingEngine.outputColorSpace, THREE.SRGBColorSpace); assert.notOk(renderingEngine.sortObjects); assert.strictEqual(renderingEngine.physicallyCorrectLights, false); done(); @@ -29,10 +30,10 @@ suite('renderer', function () { test('change renderer colorManagement', function (done) { var sceneEl = createScene(); - sceneEl.setAttribute('renderer', 'colorManagement: true;'); + sceneEl.setAttribute('renderer', 'colorManagement: false;'); sceneEl.addEventListener('loaded', function () { - assert.ok(sceneEl.renderer.outputEncoding); - assert.equal(sceneEl.renderer.outputEncoding, THREE.sRGBEncoding); + assert.ok(sceneEl.renderer.outputColorSpace); + assert.equal(sceneEl.renderer.outputColorSpace, THREE.LinearSRGBColorSpace); done(); }); document.body.appendChild(sceneEl);