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

SpriteNodeMaterial: Add sizeAttenuation #29394

Open
wants to merge 19 commits into
base: dev
Choose a base branch
from

Conversation

RenaudRohlinger
Copy link
Collaborator

@RenaudRohlinger RenaudRohlinger commented Sep 12, 2024

Related issue: #29269 #29374

Description

Following #29374, I will spread the features and fixes among multiple PRs instead. This fixes #29269.

This contribution is funded by Segments.AI & Utsubo

Copy link

github-actions bot commented Sep 12, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 686.07
169.79
686.07
169.79
+0 B
+0 B
WebGPU 832.49
223.05
832.81
223.14
+313 B
+90 B
WebGPU Nodes 832
222.92
832.32
223.01
+313 B
+89 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 462.42
111.53
462.42
111.53
+0 B
+0 B
WebGPU 529.96
142.77
530.28
142.87
+313 B
+98 B
WebGPU Nodes 486.62
132.63
486.67
132.65
+45 B
+16 B

@RenaudRohlinger RenaudRohlinger changed the title SpriteNodeMaterial: Add sizeAttenuation SpriteNodeMaterial: Add sizeAttenuation and fix perspective transformation Sep 12, 2024
@RenaudRohlinger
Copy link
Collaborator Author

Could someone test the example webgpu_tsl_galaxy with this PR on a windows? I have no clue what could be wrong there I'm out of ideas, tests runs fine on my macos.

@RenaudRohlinger RenaudRohlinger changed the title SpriteNodeMaterial: Add sizeAttenuation and fix perspective transformation SpriteNodeMaterial: Add sizeAttenuation Sep 12, 2024
@WestLangley
Copy link
Collaborator

WestLangley commented Sep 12, 2024

Can you please test with a single sprite and confirm disabling size attenuation works as you expect it to?

I tried to help, but the PR is not working for me in its current form.

@RenaudRohlinger
Copy link
Collaborator Author

The issue seems to be fixed and was related to using const scale with assign instead of let scale.

webgpu_sprites.html example with sprite.scale.set( .001 * imageWidth, .001 * imageHeight, 1.0 ); and sizeAttenuation: false:
Screenshot 2024-09-14 at 18 36 35

@@ -317,6 +317,12 @@ export default class RenderObject {

}

if ( object.center ) {

cacheKey += object.center.x + ',' + object.center.y + ',';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uniform values ​​should not generate cache key.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately center is not a uniform value.

In that case we probably want to deprecate that property and move it to the SpriteNodeMaterial as SpriteNodeMaterial.centerNode?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be uniform in both renderers, but I think you may have a problem related to the uniform reference. Maybe if you replace uniform() with reference( 'center', 'vec2' ) can it work in your tests?

alignedPosition = alignedPosition.sub( uniform( object.center ).sub( 0.5 ) );

setupPosition( { object, context } ) {
setupPosition( { object, camera, context } ) {

const useSizeAttenuation = this.sizeAttenuation;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const sizeAttenuation = this.sizeAttenuation;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add sizeAttenuation to SpriteNodeMaterial
4 participants