Skip to content

Add Vanilla storybook for 3D tiles renderer#62

Closed
jeantimex wants to merge 3 commits intotakram-design-engineering:mainfrom
jeantimex:main
Closed

Add Vanilla storybook for 3D tiles renderer#62
jeantimex wants to merge 3 commits intotakram-design-engineering:mainfrom
jeantimex:main

Conversation

@jeantimex
Copy link
Copy Markdown

3d-tiles-renderer-tsx.mov

This is the Vanilla implementation of the Tokyo 3D tiles renderer demo.

The codes related to atmosphere setup are mostly based on Atmosphere-Vanilla.tsx. The codes that related to 3D tiles rendering are referenced from googleMapsExample.js.

@shotamatsuda
Copy link
Copy Markdown
Member

Hello! While I’m glad to see you’ve successfully integrated 3D Tiles Renderer in vanilla JS, I’m a bit hesitant to include it in our Storybook. Aside from the use of 3D Tiles Renderer (Globe in your code), the setup is almost identical to the clouds story for vanilla JS, which already demonstrates the “deferred” lighting with the atmosphere.

How about pulling the Globe into the helpers directory and adding comments in the atmosphere and cloud stories to show how it can be used?

A few additional notes:

  • I’m inclined to avoid unnecessary states (scene and camera, etc.) as instance variables in the Globe. These could be passed as arguments to update(). Separating GlobeControls from Globe might make it more natural to treat Globe as a function that returns TilesRenderer.

  • Shadow map isn’t needed here, so it can be omitted.

  • You can find the default parameters for AerialPerspectiveEffect here, and redundant settings can be omitted.

@jeantimex
Copy link
Copy Markdown
Author

Thank you so much for the feedback! Yes, I can pull the Globe into the helpers directory and add comments to the atmosphere and cloud stories.

I've been busy on work, so I will find some time to take care of it. :)

@shotamatsuda
Copy link
Copy Markdown
Member

Closing in favor of mrdoob/three.js#33292

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