AfterImageNode: Fix usage of multiple nodes. #30834
Conversation
| @@ -1,9 +1,6 @@ | |||
| import { RenderTarget, Vector2, QuadMesh, NodeMaterial, RendererUtils, TempNode, NodeUpdateType } from 'three/webgpu'; | |||
| import { nodeObject, Fn, float, uv, texture, passTexture, uniform, sign, max, convertToTexture } from 'three/tsl'; | |||
|
|
|||
| const _size = /*@__PURE__*/ new Vector2(); | |||
| const _quadMeshComp = /*@__PURE__*/ new QuadMesh(); | |||
There was a problem hiding this comment.
Using a quad mesh in module scope is a pattern frequently used in FX classes.
Does the error mentioned in #30832 happen with other classes as well? E.g. AnamorphicNode?
There was a problem hiding this comment.
I just did a quick test and it seems to works fine on anamorphic node
There was a problem hiding this comment.
@Mugen87 the main difference between anamorphic and afterImage is one set the material in the nodeBuilder ( afterImage ) , the other one in the updateBefore
There was a problem hiding this comment.
one set the material in the nodeBuilder ( afterImage )
Do you mind explaining in more detail. I'm afraid I don't understand yet.
If possible, I would prefer a solution that avoids handling quad meshes differently.
There was a problem hiding this comment.
afterImage : _quadMeshComp.material = materialComposed is in setup( builder )
So the material is override by the last afterImage node built and it create the bug with multiple afterImage / PostProcessing.
anamorphic :_quadMesh.material = this._material is set in updateBefore( frame ) so before the rendering, therefore it allow one _quadMesh to be share between all anamorphic instance with no problem.
The second approach (anamorphic) is better and functionnal with multiple node /PostProcessing, but then I'm wondering why there is more than one _quadMesh for all fx node and why not only one quadMesh share between all of them?
There was a problem hiding this comment.
interesting, maybe its time as threejs orient itself to support webgpu ? also looking at caniuse es2020 & es2020 Feature support list i think it will be safe on new version of threejs to go for it. @mrdoob
There was a problem hiding this comment.
@Mugen87 i change the fix , using updateBefore( frame ) , tested on my project and it works correctly now with multiple postprocessing / afterImage node
There was a problem hiding this comment.
Regarding ES2020: The big blocker were build tools like Webpack 4 at that time. Since we don't transpile, we must make sure tools like Webpack 4 are compatible with the raw code base, see #25168.
There was a problem hiding this comment.
Understand, just checked and webpack4 it isnt support anymore ( last commit 6years ago ) so I guess its safe to assume users using latest threejs also evolve but i dont have the full scope / policy of threejs and let you guys decide! thanks for validate the PR.
There was a problem hiding this comment.
Another note: I did a quick analyze using ai on other node and afterImageNode looks like an isolated case ;)
Mugen87
changed the title
Fixed #30832
Description
Fix afterImage when multiple
PostProcessing&afterImageare used