Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(composer): hdr url is sometimes wrong #352

Merged
merged 1 commit into from
Jan 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/scene-composer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@
"global": {
"lines": 77.35,
"statements": 76.47,
"functions": 76.8,
"functions": 76.7,
"branches": 62.98,
"branchesTrue": 100
}
Expand Down
6 changes: 6 additions & 0 deletions packages/scene-composer/src/common/loadingManagers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { LoadingManager } from 'three';

export const GLTFLoadingManager = new LoadingManager();
export const EnvironmentLoadingManager = new LoadingManager();

export const tmLoadingManagers = [GLTFLoadingManager, EnvironmentLoadingManager];
9 changes: 7 additions & 2 deletions packages/scene-composer/src/components/WebGLCanvasManager.tsx
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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';
Expand All @@ -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');

Expand Down Expand Up @@ -79,7 +84,7 @@ export const WebGLCanvasManager: React.FC = () => {
return (
<React.Fragment>
<EditorMainCamera />
{environmentPreset in presets && <Environment preset={environmentPreset} />}
{environmentPreset in presets && <Environment preset={environmentPreset} extensions={envLoaderExtension} />}
<group name={ROOT_OBJECT_3D_NAME} dispose={null}>
{rootNodeRefs &&
rootNodeRefs.map((rootNodeRef) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
findComponentByType,
findNearestViableParentAncestorNodeRef,
} from '../../../utils/nodeUtils';
import { GLTFLoadingManager } from '../../../common/loadingManagers';

import { useGLTF } from './GLTFLoader';

Expand Down Expand Up @@ -78,6 +79,8 @@ export const GLTFModelComponent: React.FC<GLTFModelProps> = ({
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(() => {
Expand All @@ -103,7 +106,7 @@ export const GLTFModelComponent: React.FC<GLTFModelProps> = ({
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import create from 'zustand';
import { DefaultLoadingManager } from 'three';

import { tmLoadingManagers } from '../../../common/loadingManagers';

type Data = {
errors: string[];
active: boolean;
Expand All @@ -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: [],
Expand Down