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

3D Tiles - Ternary Functions (fixed) #4761

Merged
merged 4 commits into from
Dec 19, 2016

Conversation

Dylan-Brown
Copy link
Contributor

Previously, I had incorrectly merged into my branch from master in #4709, for issue #4694. This PR is even with the 3d-tiles branch, except the changes I had made for the ternary functions, and so the files changed should reflect only the 4 I changed. Implemented in this PR are the mix and clamp functions for the 3d-tiles styling language. @lilleyse

@Dylan-Brown Dylan-Brown mentioned this pull request Dec 16, 2016
4 tasks
addStyle('Clamp and Mix', {
color : "clamp(color(), 0.0, 100.0)",
pointSize : "mix(0.0, 2.0, 0.5) * 5"
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Use some of the point cloud properties in here so that the point cloud is more dynamic looking. Also right now we technically don't support using clamp with anything but numbers (this will change soon).

Copy link
Contributor

Choose a reason for hiding this comment

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

It does translate to valid glsl code though by accident.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, what kind of properties should I be using? And should I remove clamp then?

Copy link
Contributor

Choose a reason for hiding this comment

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

Clamp is fine, I just mean to use the per-point properties temperature or id.

Copy link
Contributor

Choose a reason for hiding this comment

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

like clamp(${temperature}, 0.1, 0.2)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lilleyse Done!

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah my mistake here, the shader doesn't compile because color: clamp(${temperature}, 0.1, 0.2) doesn't result in a color, but "color() * clamp(${temperature}, 0.1, 0.2)" should do the trick.

Can you also include temperature in the pointSize calculation.

Copy link
Contributor Author

@Dylan-Brown Dylan-Brown Dec 19, 2016

Choose a reason for hiding this comment

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

@lilleyse Also done! Also, I figured out that I can't run the Sandcastle app locally because in the Sandcastle folder, index.html refers to a few scripts that simply don't exist for me; it's easy to see using the javascript console in my chrome browser. So hopefully these adjustments work well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I'm not sure why that's happening but the changes do look good.

@lilleyse
Copy link
Contributor

Thanks @Dylan-Brown

@lilleyse lilleyse merged commit 31cf664 into CesiumGS:3d-tiles Dec 19, 2016
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