-
Notifications
You must be signed in to change notification settings - Fork 816
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
EmissiveStrength and Clearcoat extensions #4319
Changes from 2 commits
7beee02
c4eda51
706b6e3
cc7a6b6
e14c82a
e672cf5
10eadec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,18 +16,18 @@ | |
import {Mesh, MeshBasicMaterial, OrthographicCamera, PlaneGeometry, Scene, Texture as ThreeTexture, WebGLRenderTarget} from 'three'; | ||
|
||
import {blobCanvas} from '../../model-viewer-base.js'; | ||
import {Image as GLTFImage} from '../../three-components/gltf-instance/gltf-2.0.js'; | ||
import {Renderer} from '../../three-components/Renderer.js'; | ||
|
||
import {Image as ImageInterface} from './api.js'; | ||
import {$correlatedObjects, $onUpdate, $sourceObject, ThreeDOMElement} from './three-dom-element.js'; | ||
import {$correlatedObjects, $onUpdate, ThreeDOMElement} from './three-dom-element.js'; | ||
|
||
|
||
const quadMaterial = new MeshBasicMaterial(); | ||
const quad = new PlaneGeometry(2, 2); | ||
let adhocNum = 0; | ||
|
||
export const $threeTexture = Symbol('threeTexture'); | ||
export const $threeTextures = Symbol('threeTextures'); | ||
export const $applyTexture = Symbol('applyTexture'); | ||
|
||
/** | ||
|
@@ -41,30 +41,28 @@ export class Image extends ThreeDOMElement implements ImageInterface { | |
return this[$correlatedObjects]?.values().next().value as ThreeTexture; | ||
} | ||
|
||
constructor( | ||
onUpdate: () => void, texture: ThreeTexture|null, | ||
gltfImage: GLTFImage|null) { | ||
gltfImage = gltfImage ?? { | ||
name: (texture && texture.image && texture.image.src) ? | ||
texture.image.src.split('/').pop() : | ||
'adhoc_image', | ||
uri: (texture && texture.image && texture.image.src) ? | ||
texture.image.src : | ||
'adhoc_image' + adhocNum++ | ||
}; | ||
super(onUpdate, gltfImage, new Set<ThreeTexture>(texture ? [texture] : [])); | ||
get[$threeTextures](): Set<ThreeTexture> { | ||
return this[$correlatedObjects] as Set<ThreeTexture>; | ||
} | ||
|
||
constructor(onUpdate: () => void, texture: ThreeTexture|null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this predates you, but do we know when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, or at least it should work the same way as https://github.com/google/model-viewer/blob/8546f41450af57ee37fe7c00e3ef210138f6e3a0/packages/model-viewer/src/features/scene-graph/sampler.ts#L80C2-L100C56 Maybe another refactor where we remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think that would be ideal. |
||
super(onUpdate, new Set<ThreeTexture>(texture ? [texture] : [])); | ||
|
||
if (!this[$threeTexture].image.src) { | ||
this[$threeTexture].image.src = 'adhoc_image' + adhocNum++; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
} | ||
} | ||
|
||
get name(): string { | ||
return (this[$sourceObject] as GLTFImage).name || ''; | ||
return this[$threeTexture].image.name || ''; | ||
} | ||
|
||
get uri(): string|undefined { | ||
return (this[$sourceObject] as GLTFImage).uri; | ||
return this[$threeTexture].image.src; | ||
} | ||
|
||
get bufferView(): number|undefined { | ||
return (this[$sourceObject] as GLTFImage).bufferView; | ||
return this[$threeTexture].image.bufferView; | ||
} | ||
|
||
get element(): HTMLVideoElement|HTMLCanvasElement|undefined { | ||
|
@@ -88,7 +86,9 @@ export class Image extends ThreeDOMElement implements ImageInterface { | |
} | ||
|
||
set name(name: string) { | ||
(this[$sourceObject] as GLTFImage).name = name; | ||
for (const texture of this[$threeTextures]) { | ||
texture.image.name = name; | ||
} | ||
} | ||
|
||
update() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were you still planning to rename
$correlatedObjects
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the "correlated" name comes from the default correlation through the gltfMap from ThreeJS Loader:
model-viewer/packages/model-viewer/src/features/scene-graph/model.ts
Line 96 in 8546f41
I'm not sure if the name should change.