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: Add TSL VFX Tornado #29020

Merged
merged 3 commits into from
Jul 30, 2024
Merged

Conversation

brunosimon
Copy link
Contributor

Description

Adds WebGPU/TSL VFX Tornado example.
I included the post processing bloom.

localhost_8083_examples_webgpu_tsl_vfx_tornado html

I added the screenshot to the exceptions since I can't generate one that works with the tests.

@Mugen87 Mugen87 added this to the r168 milestone Jul 30, 2024
@WestLangley
Copy link
Collaborator

Just thinking out loud...

Each three.js example should demonstrate a three.js feature. They are not simply showcases.

I wonder if this example should be renamed webgpu_postprocessing_bloom_tornado.html?

... or any one of many other reasonable options...

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 30, 2024

The idea behind @brunosimon's webgpu_tsl_* examples is to showcase how you can use TSL to implement various effects. IMO, the naming is fine as it is.

I also think we should not take the existing example policy too strict in this case. We need visual striking examples for promoting the new material system and renderer.

@WestLangley
Copy link
Collaborator

I also think we should not take the existing example policy too strict in this case

Well, at least you admit we have an existing policy. By choosing not to adhere to it, the policy becomes meaningless for the rest of us.

webgpu_postprocessing_bloom_emissive.html uses tsl.

@sunag
Copy link
Collaborator

sunag commented Jul 30, 2024

I think the example is intended to show the tornado, the bloom would just be a graphical enhancement but not mandatory.

@sunag sunag merged commit ddd1bc6 into mrdoob:dev Jul 30, 2024
11 checks passed
@WestLangley
Copy link
Collaborator

I think the example is intended to show the tornado

"tornado" is not a feature of three.js.

@sunag
Copy link
Collaborator

sunag commented Jul 30, 2024

"tornado" is not a feature of three.js.

Earth, smoke, galaxy are not either. The examples are based on showing how to do this using TSL.

@cmhhelgeson
Copy link
Contributor

cmhhelgeson commented Jul 30, 2024

"tornado" is not a feature of three.js.

Earth, smoke, galaxy are not either. The examples are based on showing how to do this using TSL.

I hope I'm not speaking out of turn here, but I think @WestLangley would find issue with those names as well, which also do not follow the previous policy. I don't believe he is discounting the need for visually arresting samples that demonstrate the new node system, nor that there's no benefit to a wide range of samples, just that the naming of the samples should more directly address or specify the sample's predominantly utilized feature set. I think this issue could be easily addressed by appending additional keywords to the existing names. For instance, we have multiple compute particle samples that technically address the same features, but each example is still prefixed with "webgpu_compute_particles" to directly indicate what feature set the example primarily addresses.

@sunag
Copy link
Collaborator

sunag commented Jul 30, 2024

just that the naming of the samples should more directly address or specify the predominant feature set used within an example.

I think we are not straying from this logic, we have 150~ tornado lines vs 1 bloom line. If bloom is essential to the functioning of the example as it is in compute it would make sense, but it's not the case #29020 (comment).

I agree with the tags I think is a good idea, regardless of the names.

@cmhhelgeson
Copy link
Contributor

cmhhelgeson commented Jul 30, 2024

If bloom is essential to the functioning of the example as it is in compute it would make sense, but it's not the case #29020 (comment).

I think we're still getting hung up on a specific suggestion rather than the broad principle behind the suggestion. Perhaps "bloom" is not the proper identifier for the tornado sample. It could be geometry, points, perlin, textures, luminance, anything that is truly relevant to the example. But naming the sample tornado, while accurate to the sample's intended functionality, nullifies the sample's effectiveness to the website's initial viewer as a reference for anything other than itself. It may be impossible to peg a given sample's utility down to a single feature set, but it's at least worth having the discussion on more effective names before that determination is made.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 30, 2024

It could be geometry, points, perlin, textures, luminance, anything that is truly relevant to the example.

If you want to add additional information to an example, you can use the tag mechanism. In this way, e.g if you search for "bloom", all examples pop up that have "bloom" as a tag.

Tags are maintained here: https://github.com/mrdoob/three.js/blob/dev/examples/tags.json

@mrdoob
Copy link
Owner

mrdoob commented Jul 31, 2024

The idea behind @brunosimon's webgpu_tsl_* examples is to showcase how you can use TSL to implement various effects. IMO, the naming is fine as it is.

I also think we should not take the existing example policy too strict in this case. We need visual striking examples for promoting the new material system and renderer.

Agreed. I think having some complex examples showing how to use TSL is good.

As a reminder, in the old days we used to have complex examples like this too but they were hard to maintain because they used a lot of custom GLSL. Thanks to TSL we shouldn't have maintenance issues this time 🤞

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.

6 participants