Closed trusktr closed 1 year ago
Side note, this problem is not apparent in any of the examples because they all use an infinite render loop, so because the scene is always re-rendering constantly, then eventually when the texture is loaded the re-draw shows it.
Yet another option is to remove the isTextureLoaded
uniform altogether, then no other change is needed because the user can render the scene in TextureLoader.load
's callback and it will work. In this case, if the user prefers not to render black color (for not-yet-loaded texture) they can avoid setting mat.texture
until the texture is loaded, which is the same pattern as with all other material textures built into Three.js.
In other words, this is the already existing pattern people use in Three.js:
// Example 4
const mat = new MeshPhysicalMaterial()
const tex = new TextureLoader.load('foo.png', () => {
mat.map = tex // set the texture in here instead, to avoid black color.
mat.needsUpdate = true
renderer.render(scene, camera)
})
// mat.map = tex // Don't set it yet if we don't want to render black color.
And if we apply the same pattern to ProjectedMaterial (assuming we delete the isTextureLoaded
, it looks like this:
// Example 5
const mat = new ProjectedMaterial()
const tex = new TextureLoader.load('foo.png', () => {
mat.texture = tex // set the texture in here instead, to avoid black color.
mat.needsUpdate = true
renderer.render(scene, camera)
})
// mat.texture = tex // Don't set it yet if we don't want to render black color.
and now that aligns with existing Three.js patterns which means API consistency.
Which method do you like most? The method from Example 1, 3, or 5?
EDIT: The method in example 5 still needs to handle texture set before they've loaded, in order to call saveDimensions
, so Example 5 isn't so good either then due to this unique requirement, unless we override Material.needsUpdate
so that it will call saveDimensions
. With this in mind, setting the texture early would be like the following (still with the isTextureLoaded
uniform deleted):
// Example 6
const mat = new ProjectedMaterial()
const tex = new TextureLoader.load('foo.png', () => {
mat.needsUpdate = true // signal update, so saveDimensions can be called
renderer.render(scene, camera)
})
mat.texture = tex // set texture early, it may render black color
Unfortunately using needsUpdate
means the renderer has to re-compile the material program. As an alternative, then the user would have to do this which is also less ideal:
// Example 7
const mat = new ProjectedMaterial()
const tex = new TextureLoader.load('foo.png', () => {
mat.updateFromTexture() // make a public API, does not require re-compiling the material
renderer.render(scene, camera)
})
mat.texture = tex // set texture early, it may render black color
Example 1 is still the simplest option here then, while https://github.com/mrdoob/three.js/pull/24145 would be even better still.
Side note, I am making these changes in parallel in my TypeScript version, to eventually compile it to WebAssembly along with Three.js.
Oops, I forgot, this issue only happens if we set the material early before it is loaded, because that queues up the addLoadListener
.
However, if the user does exactly like in Example 5, there is no issue because at the moment that texture
property is set, it will see the texture is already loaded and immediately it will set uniforms.isTextureLoaded
to true, and any subsequent animation frame will render correctly.
…so people know when to do things (f.e. to re-render a scene).
As an alternative to https://github.com/mrdoob/three.js/pull/24145 which was rejected, this is a less ideal way to signal to users when texture load (in the perspective of ProjectedMaterial) has happened.
Also see the Discussion. Note that using a
TextureLoader.load
callback does not help with the issue described there.This change allows someone to do the following and for the texture to appear on screen as expected:
Note that the following does not work:
As an alternative to this PR, we could add a new
isTextureLoaded
getter/setter. If we do this instead, then the following will be a working alternative:Which one do you like better? Example 1, or Example 3?
I like the method in Example 1 better because it does not present the user with an opportunity to do things wrong, whereas with the method in Example 3 the user can forget to set
isTextureLoaded
or they can set it at the wrong time before the texture is actually loaded.To be honest, the cleanest solution would be if https://github.com/mrdoob/three.js/pull/24145 was approved, in which case the example 2 above would work fine.