-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
AfterImageNode: Refactor code and improve demo. #30433
Conversation
|
||
// material and TSL | ||
|
||
const material = new THREE.SpriteNodeMaterial( { blending: THREE.AdditiveBlending, depthWrite: false, transparent: true } ); |
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.
@sunag I had to set transparent
to true
since the default value of SpriteNodeMaterial
is false
.
However, SpriteMaterial.transparent
is true
by default. Is the new default value of SpriteNodeMaterial
as expected?
If not, it seems the copy of default values in NodeMaterial.setDefaultValues()
does not work in all cases.
three.js/src/materials/nodes/NodeMaterial.js
Lines 1028 to 1060 in 88062ed
setDefaultValues( material ) { | |
// This approach is to reuse the native refreshUniforms* | |
// and turn available the use of features like transmission and environment in core | |
for ( const property in material ) { | |
const value = material[ property ]; | |
if ( this[ property ] === undefined ) { | |
this[ property ] = value; | |
if ( value && value.clone ) this[ property ] = value.clone(); | |
} | |
} | |
const descriptors = Object.getOwnPropertyDescriptors( material.constructor.prototype ); | |
for ( const key in descriptors ) { | |
if ( Object.getOwnPropertyDescriptor( this.constructor.prototype, key ) === undefined && | |
descriptors[ key ].get !== undefined ) { | |
Object.defineProperty( this.constructor.prototype, key, descriptors[ key ] ); | |
} | |
} | |
} |
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.
Yes, sprites should be transparent by default.
If not, it seems the copy of default values in NodeMaterial.setDefaultValues() does not work in all cases.
It would just define values that do not exist in the class in order to save some computation when cloning the classes, of course this could be solved by testing, but I think this approach is a hack and should be discontinued at some point and replaced by something like #28328
Related issue: -
Description
The PR slightly refactors the code of
AfterImageNode
and improves documentation and also makeswebgpu_postprocessing_afterimage
more interesting. The demo shows how you can useAfterImageNode
for implementing a basic "trails" effect for particles.https://rawcdn.githack.com/mrdoob/three.js/0f4c50f7e0f1008470bed836b398229161d794cf/examples/webgpu_postprocessing_afterimage.html