From 4af6d837ae1421a0fac91e3a019c11c080bab1ce Mon Sep 17 00:00:00 2001 From: Xinyi Date: Fri, 6 Jan 2023 17:19:57 -0800 Subject: [PATCH] fix(composer): hdr url is sometimes wrong --- packages/scene-composer/package.json | 2 +- .../src/common/loadingManagers.ts | 6 ++ .../src/components/WebGLCanvasManager.tsx | 9 +- .../ModelRefComponent/GLTFModelComponent.tsx | 5 +- .../__tests__/GLTFModelComponent.spec.tsx | 3 +- .../__snapshots__/useProgress.spec.tsx.snap | 0 .../three-fiber/hooks/useProgress.spec.tsx | 3 +- .../three-fiber/hooks/useProgress.tsx | 86 ++++++++++--------- 8 files changed, 67 insertions(+), 47 deletions(-) create mode 100644 packages/scene-composer/src/common/loadingManagers.ts rename packages/scene-composer/{tests => src}/components/three-fiber/hooks/__snapshots__/useProgress.spec.tsx.snap (100%) rename packages/scene-composer/{tests => src}/components/three-fiber/hooks/useProgress.spec.tsx (96%) diff --git a/packages/scene-composer/package.json b/packages/scene-composer/package.json index 49987be16..7d87dc396 100644 --- a/packages/scene-composer/package.json +++ b/packages/scene-composer/package.json @@ -161,7 +161,7 @@ "global": { "lines": 77.35, "statements": 76.47, - "functions": 76.8, + "functions": 76.7, "branches": 62.98, "branchesTrue": 100 } diff --git a/packages/scene-composer/src/common/loadingManagers.ts b/packages/scene-composer/src/common/loadingManagers.ts new file mode 100644 index 000000000..61c0d9320 --- /dev/null +++ b/packages/scene-composer/src/common/loadingManagers.ts @@ -0,0 +1,6 @@ +import { LoadingManager } from 'three'; + +export const GLTFLoadingManager = new LoadingManager(); +export const EnvironmentLoadingManager = new LoadingManager(); + +export const tmLoadingManagers = [GLTFLoadingManager, EnvironmentLoadingManager]; diff --git a/packages/scene-composer/src/components/WebGLCanvasManager.tsx b/packages/scene-composer/src/components/WebGLCanvasManager.tsx index ef3efb45f..8a0a5dadb 100644 --- a/packages/scene-composer/src/components/WebGLCanvasManager.tsx +++ b/packages/scene-composer/src/components/WebGLCanvasManager.tsx @@ -1,6 +1,6 @@ import * as THREE from 'three'; import * as awsui from '@awsui/design-tokens'; -import React, { useCallback, useContext, useEffect, useRef, useState } from 'react'; +import React, { useContext, useEffect, useRef, useState } from 'react'; import { GizmoHelper, GizmoViewport } from '@react-three/drei'; import { ThreeEvent, useThree } from '@react-three/fiber'; @@ -14,6 +14,7 @@ import { getGlobalSettings } from '../common/GlobalSettings'; import { ViewCursorWidget } from '../augmentations/components/three-fiber/viewpoint/ViewCursorWidget'; import { getIntersectionTransform } from '../utils/raycastUtils'; import { createNodeWithPositionAndNormal } from '../utils/nodeUtils'; +import { EnvironmentLoadingManager } from '../common/loadingManagers'; import Environment, { presets } from './three-fiber/Environment'; import { StatsWindow } from './three-fiber/StatsWindow'; @@ -25,6 +26,10 @@ import IntlProvider from './IntlProvider'; const GIZMO_MARGIN: [number, number] = [72, 72]; +const envLoaderExtension = (loader: THREE.Loader) => { + loader.manager = EnvironmentLoadingManager; +}; + export const WebGLCanvasManager: React.FC = () => { const log = useLifecycleLogging('WebGLCanvasManager'); @@ -79,7 +84,7 @@ export const WebGLCanvasManager: React.FC = () => { return ( - {environmentPreset in presets && } + {environmentPreset in presets && } {rootNodeRefs && rootNodeRefs.map((rootNodeRef) => { diff --git a/packages/scene-composer/src/components/three-fiber/ModelRefComponent/GLTFModelComponent.tsx b/packages/scene-composer/src/components/three-fiber/ModelRefComponent/GLTFModelComponent.tsx index 05c436b53..0948fc4ea 100644 --- a/packages/scene-composer/src/components/three-fiber/ModelRefComponent/GLTFModelComponent.tsx +++ b/packages/scene-composer/src/components/three-fiber/ModelRefComponent/GLTFModelComponent.tsx @@ -22,6 +22,7 @@ import { findComponentByType, findNearestViableParentAncestorNodeRef, } from '../../../utils/nodeUtils'; +import { GLTFLoadingManager } from '../../../common/loadingManagers'; import { useGLTF } from './GLTFLoader'; @@ -78,6 +79,8 @@ export const GLTFModelComponent: React.FC = ({ component.uri, uriModifier, (loader) => { + loader.manager = GLTFLoadingManager; + loader.manager.onStart = appendFunction(loader.manager.onStart, () => { // Use setTimeout to avoid mutating the state during rendering process setTimeout(() => { @@ -103,7 +106,7 @@ export const GLTFModelComponent: React.FC = ({ contentLength = progressEvent.total; } // @ts-ignore - __onDownloadProgress is injected in the LoadingProgress component - const onDownloadingProgress = THREE.DefaultLoadingManager.__onDownloadProgress; + const onDownloadingProgress = GLTFLoadingManager.__onDownloadProgress; if (onDownloadingProgress) { const target = progressEvent.target as XMLHttpRequest; diff --git a/packages/scene-composer/src/components/three-fiber/ModelRefComponent/__tests__/GLTFModelComponent.spec.tsx b/packages/scene-composer/src/components/three-fiber/ModelRefComponent/__tests__/GLTFModelComponent.spec.tsx index c43108f0b..c0753440b 100644 --- a/packages/scene-composer/src/components/three-fiber/ModelRefComponent/__tests__/GLTFModelComponent.spec.tsx +++ b/packages/scene-composer/src/components/three-fiber/ModelRefComponent/__tests__/GLTFModelComponent.spec.tsx @@ -51,6 +51,7 @@ import { IModelRefComponentInternal, useStore } from '../../../../store'; import { GLTFModelComponent } from '../GLTFModelComponent'; import { KnownComponentType } from '../../../..'; import { getScaleFactor } from '../../../../utils/mathUtils'; +import { GLTFLoadingManager } from '../../../../common/loadingManagers'; // @ts-ignore jest.mock('scheduler', () => require('scheduler/unstable_mock')); @@ -181,7 +182,7 @@ describe('GLTFLoader', () => { loaded: 11, }; const onDownloadProgressSpy = jest - .spyOn(THREE.DefaultLoadingManager as any, '__onDownloadProgress') + .spyOn(GLTFLoadingManager as any, '__onDownloadProgress') .mockImplementation(() => null); await ReactThreeTestRenderer.create( diff --git a/packages/scene-composer/tests/components/three-fiber/hooks/__snapshots__/useProgress.spec.tsx.snap b/packages/scene-composer/src/components/three-fiber/hooks/__snapshots__/useProgress.spec.tsx.snap similarity index 100% rename from packages/scene-composer/tests/components/three-fiber/hooks/__snapshots__/useProgress.spec.tsx.snap rename to packages/scene-composer/src/components/three-fiber/hooks/__snapshots__/useProgress.spec.tsx.snap diff --git a/packages/scene-composer/tests/components/three-fiber/hooks/useProgress.spec.tsx b/packages/scene-composer/src/components/three-fiber/hooks/useProgress.spec.tsx similarity index 96% rename from packages/scene-composer/tests/components/three-fiber/hooks/useProgress.spec.tsx rename to packages/scene-composer/src/components/three-fiber/hooks/useProgress.spec.tsx index 23fb4f743..43c289a6d 100644 --- a/packages/scene-composer/tests/components/three-fiber/hooks/useProgress.spec.tsx +++ b/packages/scene-composer/src/components/three-fiber/hooks/useProgress.spec.tsx @@ -1,9 +1,10 @@ import { DefaultLoadingManager } from 'three'; -import { useProgressImpl } from '../../../../src/components/three-fiber/hooks/useProgress'; +import { useProgressImpl } from './useProgress'; jest.mock('three', () => ({ DefaultLoadingManager: {}, + LoadingManager: class {}, })); describe('useProgress', () => { diff --git a/packages/scene-composer/src/components/three-fiber/hooks/useProgress.tsx b/packages/scene-composer/src/components/three-fiber/hooks/useProgress.tsx index 51aff107a..6b7f3400f 100644 --- a/packages/scene-composer/src/components/three-fiber/hooks/useProgress.tsx +++ b/packages/scene-composer/src/components/three-fiber/hooks/useProgress.tsx @@ -1,6 +1,8 @@ import create from 'zustand'; import { DefaultLoadingManager } from 'three'; +import { tmLoadingManagers } from '../../../common/loadingManagers'; + type Data = { errors: string[]; active: boolean; @@ -16,48 +18,50 @@ type Data = { let saveLastTotalLoaded = 0; export const useProgressImpl = (set) => { - DefaultLoadingManager.onStart = (item, loaded, total) => { - set({ - active: true, - item, - loaded, - total, - progress: ((loaded - saveLastTotalLoaded) / (total - saveLastTotalLoaded)) * 100, - }); - }; - DefaultLoadingManager.onLoad = () => { - set({ active: false }); - }; - DefaultLoadingManager.onError = (item) => set((state) => ({ errors: [...state.errors, item] })); - DefaultLoadingManager.onProgress = (item, loaded, total) => { - if (loaded === total) { - saveLastTotalLoaded = total; - } - set({ - active: true, - item, - loaded, - total, - progress: ((loaded - saveLastTotalLoaded) / (total - saveLastTotalLoaded)) * 100 || 100, - }); - }; + [DefaultLoadingManager, ...tmLoadingManagers].forEach((manager) => { + manager.onStart = (item, loaded, total) => { + set({ + active: true, + item, + loaded, + total, + progress: ((loaded - saveLastTotalLoaded) / (total - saveLastTotalLoaded)) * 100, + }); + }; + manager.onLoad = () => { + set({ active: false }); + }; + manager.onError = (item) => set((state) => ({ errors: [...state.errors, item] })); + manager.onProgress = (item, loaded, total) => { + if (loaded === total) { + saveLastTotalLoaded = total; + } + set({ + active: true, + item, + loaded, + total, + progress: ((loaded - saveLastTotalLoaded) / (total - saveLastTotalLoaded)) * 100 || 100, + }); + }; - // Add an additional function to DefaultLoadingManager to track downloading progress. - // Note: since we add an additional method to the default loading manager, and three does not use interface - // when declaring the type of the loading manager, there is no easy way to extend a class in typescript. - // This is a bit hacky, but the alternative is to recreate all the loader related classes by ourselves. - // We can revisit if we have the need to do a lot of customization when loading assets in the future. - // The risk of conflicting with the threejs future change is small since we uses an underscored name here. - // @ts-ignore - DefaultLoadingManager.__onDownloadProgress = (url: string, loaded: number, total: number) => { - set({ - active: true, - downloadItem: url, - downloaded: loaded, - downloadTotal: total, - downloadProgress: total ? (loaded / total) * 100 : /* istanbul ignore next */ 0, - }); - }; + // Add an additional function to loading managers to track downloading progress. + // Note: since we add an additional method to the default loading manager, and three does not use interface + // when declaring the type of the loading manager, there is no easy way to extend a class in typescript. + // This is a bit hacky, but the alternative is to recreate all the loader related classes by ourselves. + // We can revisit if we have the need to do a lot of customization when loading assets in the future. + // The risk of conflicting with the threejs future change is small since we uses an underscored name here. + // @ts-ignore + manager.__onDownloadProgress = (url: string, loaded: number, total: number) => { + set({ + active: true, + downloadItem: url, + downloaded: loaded, + downloadTotal: total, + downloadProgress: total ? (loaded / total) * 100 : /* istanbul ignore next */ 0, + }); + }; + }); return { errors: [],