Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Option to turn off normal shading #7399

Merged
merged 12 commits into from
Mar 14, 2019

Conversation

Vineg
Copy link
Contributor

@Vineg Vineg commented Dec 10, 2018

Small fixes.
#5152

@cesium-concierge
Copy link

Thanks for the pull request @Vineg!

  • ✔️ Signed CLA found.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.

Reviewers, don't forget to make sure that:

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

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

@mramato
Copy link
Contributor

mramato commented Dec 10, 2018

Thanks for the PR @Vineg! @lilleyse can you take a look a this?

@mramato mramato requested a review from lilleyse December 10, 2018 13:59
@lilleyse
Copy link
Contributor

Thanks @Vineg. Would you mind opening a separate PR for the point cloud changes so that we can focus on the geometric error fix exclusively?

@Vineg
Copy link
Contributor Author

Vineg commented Dec 11, 2018

Ok, tomorrow

@Vineg Vineg force-pushed the point-cloud-scale-fixes branch from ca38e70 to a563bda Compare December 13, 2018 16:36
@Vineg Vineg changed the title Scale 3dTiles geometric error with model Option to turn off normal shading Dec 13, 2018
@hpinkos
Copy link
Contributor

hpinkos commented Dec 27, 2018

@Vineg is this ready for review now? Next time, make a comment when it's ready so we get notified. We don't get notified when you push new commits. Thanks!

@Vineg
Copy link
Contributor Author

Vineg commented Dec 28, 2018

@lilleyse @hpinkos this one is ready

@cesium-concierge
Copy link

Thanks again for your contribution @Vineg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@hpinkos
Copy link
Contributor

hpinkos commented Jan 28, 2019

@lilleyse can you review this?

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.

Thanks @Vineg, the approach looks good. Just some minor comments.

@hpinkos
Copy link
Contributor

hpinkos commented Feb 21, 2019

@lilleyse I think this is ready.

@Vineg @geoscan-builder next time you push commits after a review, post a comment when the PR is ready for another look. We don't get notifications when commits are pushed up so we missed that you were working on this. Thanks!

@lilleyse
Copy link
Contributor

Thanks for the updates @Vineg. I just tweaked a few things and updated documentation and CHANGES.md.

@lilleyse lilleyse merged commit 5cbb5e5 into CesiumGS:master Mar 14, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants