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

Add procedurally-generated billboard clouds #9737

Merged
merged 17 commits into from
Aug 28, 2021
Merged

Add procedurally-generated billboard clouds #9737

merged 17 commits into from
Aug 28, 2021

Conversation

j9liu
Copy link
Contributor

@j9liu j9liu commented Aug 19, 2021

CumulusCloud

Clouds

This PR implements the cloud enhancement as outlined in #9691, including the following features.

This is still a work in progress. Remaining tasks:

  • Add images to CumulusCloud and CloudCollection documentation.
  • Create a Sandcastle showcasing multiple clouds in a non-empty scene.

@cesium-concierge
Copy link

Thanks for the pull request @j9liu!

  • ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

@j9liu j9liu changed the base branch from clouds to main August 23, 2021 15:31
Copy link
Contributor

@lilleyse lilleyse left a comment

Choose a reason for hiding this comment

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

@j9liu great job on this! The approach you went with strikes the right balance of ease of use and flexibility for advanced users. Excellent doc and unit tests like usual. Mostly minor feedback from me on the code side.

While it would be great to have fully procedural, volumetric clouds in the future this is a great start. I'm looking forward to your roadmap issue.

Apps/Sandcastle/gallery/Cloud Parameters.html Outdated Show resolved Hide resolved
Apps/Sandcastle/gallery/Cloud Parameters.html Outdated Show resolved Hide resolved
Source/Scene/CloudType.js Outdated Show resolved Hide resolved
Source/Scene/CloudCollection.js Show resolved Hide resolved
Source/Scene/CloudCollection.js Outdated Show resolved Hide resolved
Source/Shaders/CloudCollectionFS.glsl Outdated Show resolved Hide resolved
Source/Shaders/CloudCollectionFS.glsl Show resolved Hide resolved
Source/Shaders/CloudCollectionFS.glsl Outdated Show resolved Hide resolved
Source/Shaders/CloudCollectionFS.glsl Outdated Show resolved Hide resolved
Source/Scene/CloudCollection.js Outdated Show resolved Hide resolved
@lilleyse
Copy link
Contributor

Some more meta feedback - now that I've reviewed the code I think CloudCollection could possibly contain a BillboardCollection under the hood, but there would have to be some changes to get it to work:

  • BillboardCollection deals with both the position/sizing/orientation of billboards and texture atlasing. These two aspects would need to be decoupled since CloudCollection only needs the first as it does its own procedural texturing.
  • BillboardCollection would need to allow for custom attributes - like cloud brightness and the various noise parameters
  • BillboardCollection would need to support a shader hook in point in the frag shader, so that CloudCollection can insert its own procedural shading

Given the extent of these changes the current approach still makes sense, but if we want to consolidate in the future I think we can.

@j9liu
Copy link
Contributor Author

j9liu commented Aug 27, 2021

@lilleyse - just made some changes, let me know what you think!

@lilleyse
Copy link
Contributor

I pushed some small tweaks in 41db4a8 and updated CHANGES.md.

Thanks again @j9liu

@lilleyse lilleyse merged commit a8926a3 into main Aug 28, 2021
@lilleyse lilleyse deleted the clouds-texture branch August 28, 2021 01:14
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.

4 participants