small refactor (#4623)

google · Jan 9, 2024 · 2acefd8 · 2acefd8
1 parent acfb155
commit 2acefd8
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 11 deletions.
diff --git a/packages/model-viewer/src/features/environment.ts b/packages/model-viewer/src/features/environment.ts
@@ -14,7 +14,7 @@
  */
 
 import {property} from 'lit/decorators.js';
-import {Texture} from 'three';
+import {ACESFilmicToneMapping, AgXToneMapping, CustomToneMapping, Texture} from 'three';
 
 import ModelViewerElementBase, {$needsRender, $progressTracker, $renderer, $scene, $shouldAttemptPreload} from '../model-viewer-base.js';
 import {clamp, Constructor, deserializeUrl} from '../utilities.js';
@@ -88,7 +88,10 @@ export const EnvironmentMixin = <T extends Constructor<ModelViewerElementBase>>(
       }
 
       if (changedProperties.has('toneMapping')) {
-        this[$scene].toneMapping = this.toneMapping;
+        this[$scene].toneMapping = this.toneMapping === 'commerce' ?
+            CustomToneMapping :
+            this.toneMapping === 'agx' ? AgXToneMapping :
+                                         ACESFilmicToneMapping;
         this[$needsRender]();
       }
 

diff --git a/packages/model-viewer/src/three-components/ModelScene.ts b/packages/model-viewer/src/three-components/ModelScene.ts
@@ -13,11 +13,10 @@
  * limitations under the License.
  */
 
-import {AnimationAction, AnimationActionLoopStyles, AnimationClip, AnimationMixer, Box3, Camera, Euler, Event as ThreeEvent, LoopPingPong, LoopRepeat, Material, Matrix3, Mesh, Object3D, PerspectiveCamera, Raycaster, Scene, Sphere, Texture, Triangle, Vector2, Vector3, WebGLRenderer} from 'three';
+import {ACESFilmicToneMapping, AnimationAction, AnimationActionLoopStyles, AnimationClip, AnimationMixer, Box3, Camera, Euler, Event as ThreeEvent, LoopPingPong, LoopRepeat, Material, Matrix3, Mesh, Object3D, PerspectiveCamera, Raycaster, Scene, Sphere, Texture, ToneMapping, Triangle, Vector2, Vector3, WebGLRenderer} from 'three';
 import {CSS2DRenderer} from 'three/examples/jsm/renderers/CSS2DRenderer.js';
 import {reduceVertices} from 'three/examples/jsm/utils/SceneUtils.js';
 
-import {ToneMappingValue} from '../features/environment.js';
 import {$currentGLTF, $model, $originalGltfJson} from '../features/scene-graph.js';
 import {$nodeFromIndex, $nodeFromPoint} from '../features/scene-graph/model.js';
 import ModelViewerElementBase, {$renderer, EffectComposerInterface, RendererInterface} from '../model-viewer-base.js';
@@ -99,7 +98,7 @@ export class ModelScene extends Scene {
   public bakedShadows = new Set<Mesh>();
 
   public exposure = 1;
-  public toneMapping: ToneMappingValue = 'auto';
+  public toneMapping: ToneMapping = ACESFilmicToneMapping;
   public canScale = true;
 
   private isDirty = false;

diff --git a/packages/model-viewer/src/three-components/Renderer.ts b/packages/model-viewer/src/three-components/Renderer.ts
@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import {ACESFilmicToneMapping, AgXToneMapping, CustomToneMapping, Event, EventDispatcher, ShaderChunk, Vector2, WebGLRenderer} from 'three';
+import {ACESFilmicToneMapping, CustomToneMapping, Event, EventDispatcher, ShaderChunk, Vector2, WebGLRenderer} from 'three';
 
 import {$updateEnvironment} from '../features/environment.js';
 import {ModelViewerGlobalConfig} from '../features/loading.js';
@@ -438,7 +438,7 @@ export class Renderer extends
     const exposureIsNumber =
         typeof exposure === 'number' && !Number.isNaN(exposure);
     const env = element.environmentImage;
-    const compensateExposure = toneMapping === 'commerce' &&
+    const compensateExposure = toneMapping === CustomToneMapping &&
         (env == null || env === 'neutral' || env === 'legacy');
     this.threeRenderer.toneMappingExposure =
         (exposureIsNumber ? exposure : 1.0) *
@@ -521,10 +521,7 @@ export class Renderer extends
       } else {
         this.threeRenderer.autoClear =
             true;  // this might get reset by the effectRenderer
-        this.threeRenderer.toneMapping = scene.toneMapping === 'commerce' ?
-            CustomToneMapping :
-            scene.toneMapping === 'agx' ? AgXToneMapping :
-                                          ACESFilmicToneMapping;
+        this.threeRenderer.toneMapping = scene.toneMapping;
         this.threeRenderer.render(scene, scene.camera);
       }
       if (this.multipleScenesVisible ||