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 for dynamic polylines with polyline dash material #5681

Merged
merged 4 commits into from Jul 27, 2017

Conversation

jasonbeverage
Copy link
Contributor

This fixes an issue where dynamic polylines (rendered with the PolylineCollection) weren't properly defining the POLYLINE_DASH define in the vertex shader and causing rendering issues b/c the v_angle wasn't being computed properly.

You can see an example of the issue here: http://cesiumjs.org/Cesium/Apps/Sandcastle/?src=Hello%20World.html&label=Showcases&gist=1b4c2cf989ab0304020fdb3f94374e93

This fix looks for v_angle in the material shader and then adds the POLYLINE_DASH define if it finds it.

@mramato
Copy link
Member

mramato commented Jul 26, 2017

Thanks @jasonbeverage. This probably fixes #5450, since using a CallbackProperty triggers PolylineCollection.

@emackey can you take a look at this since you reported #5450 originally. Thanks!

@jasonbeverage
Copy link
Contributor Author

Yeah that should fix it. I didn't know that issue existed :)

@emackey
Copy link
Contributor

emackey commented Jul 26, 2017

Confirmed, this does fix #5450.

@bagnell Are you OK with keying off such a simple name here? Should v_angle be renamed to something more unique?

@pjcozzi
Copy link
Contributor

pjcozzi commented Jul 26, 2017

Thanks @jasonbeverage! Can you please update CHANGES.md?

As @emackey suggested, would you also mind renaming v_angle to something like v_polylineAngle? Probably don't use the czm_ prefix.

@jasonbeverage
Copy link
Contributor Author

Updated CHANGES.md and also changed v_angle to v_polylineAngle.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jul 27, 2017

Thanks again!

@pjcozzi pjcozzi merged commit 9a1da37 into CesiumGS:master Jul 27, 2017
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.

None yet

4 participants