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

Examples: Simpler atmosphere approach in webgpu_compute_birds #29366

Merged
merged 4 commits into from
Sep 10, 2024
Merged

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Sep 9, 2024

Related issue: #29334

Description

Instead of MeshStandardMaterial and HemisphereLight we can do the sky atmosphere with a simpler colors gradient with TSL.

@sunag My first real TSL experience 😇

Not sure how I feel about these chainable methods...

This ends up reading better to me:

add( 1.5, positionLocal.y )

Compared to this:

positionLocal.y.add( 1.5 )

Specially with sub()...

positionLocal.y.sub( 1.0 )

If I write that, but I decide I want to do 1.0 - y I feel limited...

This seems simpler and more flexible:

sub( 1.0, positionLocal.y )

Edit:

Hmm... But then by reading the code in #29354 ...

const pulse = pcurve(
	sin( hash( instanceIndex ).mul( PI2 ).add( timerGlobal( 0.5 ).mul( PI2 ) ) ).mul( 0.5 ).add( 0.5 ),
	0.25,
	0.25
).mul( 10.0 ).add( 1.0 );

I can see it ends up more compact.
But we end up with a language that has two reading directions...🤔

@mrdoob mrdoob added this to the r169 milestone Sep 9, 2024
@sunag
Copy link
Collaborator

sunag commented Sep 9, 2024

I think some redundancies are positive. In fact, we have redundancy in practically every type of language, such as in JS when creating conditionals, variables, functions, etc.

In my experience, chainable tends to be more readable for large lines of code and more efficient for drafting since it doesn't need to import new methods. I think both have their qualities and can be used by different user profiles. For example, I would use positionLocal.y.oneMinus().

This will certainly get better when we have Operator Overloading

@sunag sunag merged commit 5a83287 into dev Sep 10, 2024
11 checks passed
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.

2 participants