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

Fix specular flickering in processModelMaterialsCommon #5773

Merged
merged 2 commits into from
Aug 24, 2017
Merged

Conversation

lilleyse
Copy link
Contributor

If a model uses the KHR_materials_common extension, sets its technique to "PHONG", but doesn't provide specular or shininess values in its material, default values are 0 are used. This causes something to go wrong in the shader math resulting in a flickery effect. This fix is just smarter about checking if the model really has specular.

This issue has come up on the forum a couple times now:
https://groups.google.com/forum/#!topic/cesium-dev/kR7u5R5wq5k
https://groups.google.com/forum/#!topic/cesium-dev/JtTnEncijn0

@cesium-concierge
Copy link

@lilleyse, thanks for the pull request!

I noticed that CHANGES.md has not been updated. If this change updates the public API in any way, fixes a bug, or makes any non-trivial update, please add a bullet point to CHANGES.md and comment on this pull request so we know it was updated. For more info, see the Pull Request Guidelines.

I noticed that a file in one of our ThirdParty folders (ThirdParty/, Source/ThirdParty/) has been added or modified. Please verify that it has a section in LICENSE.md and that its license information is up to date with this new version. Once you do, please confirm by commenting on this pull request.

I am a bot who helps you make Cesium awesome! Thanks again.

@pjcozzi
Copy link
Contributor

pjcozzi commented Aug 23, 2017

Did you also update gltf-pipeline?

Please update CHANGES.md and merge.

@lilleyse lilleyse merged commit bb2413b into master Aug 24, 2017
@cesium-concierge
Copy link

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/forum/#!topic/cesium-dev/kR7u5R5wq5k
https://groups.google.com/forum/#!topic/cesium-dev/JtTnEncijn0

If this issue affects any of these threads, please post a comment like the following:

The issue at #5773 has just been closed and may resolve your issue. Look for the change in thenext stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.

I am a bot who helps you make Cesium awesome! Thanks again.

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.

3 participants