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 Labels with backgrounds #4939

Merged
merged 2 commits into from
Feb 1, 2017
Merged

Fix Labels with backgrounds #4939

merged 2 commits into from
Feb 1, 2017

Conversation

bagnell
Copy link
Contributor

@bagnell bagnell commented Jan 31, 2017

Revert to old billboard rendering method when labels have a background. Fixes #4927.

@emackey
Copy link
Contributor

emackey commented Jan 31, 2017

Take a look at the Everest Terrain demo in Sandcastle, click Sample Everest Terrain (on the 3rd row) to see lots of labels. If you try this in Cesium 1.29, it will look a lot better than what's on this branch or in master. The old mode doesn't exactly correspond to typical OPAQUE or TRANSPARENT, as it had blending turned on but was also writing depth for non-zero alpha pixels, meaning it was expecting the labels to be "mostly" opaque, like a cutout mode. Of course this strategy punched holes in things at the alpha fringes, but when label backgrounds are on there's no fringe. Is it worth trying to get this mode back? I think so...

labelbackgroundfix

@bagnell
Copy link
Contributor Author

bagnell commented Jan 31, 2017

Thanks @emackey. Updated.

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 1, 2017

Code, tests, and related Sandcastle examples look good.

Merging since this is needed for the release tomorrow morning.

@bagnell thanks for the fix.

@emackey if you have more feedback, please submit an issue.

@pjcozzi pjcozzi merged commit dde2a18 into master Feb 1, 2017
@pjcozzi pjcozzi deleted the billboard-outline-fix branch February 1, 2017 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants