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 ground polyline texcoords bug for first segment on some platforms #8024

Merged
merged 1 commit into from
Jul 22, 2019

Conversation

likangning93
Copy link
Contributor

Fixes #8017.

Lengthwise texcoords for the first segment of ground polylines weren't getting computed correctly on some platforms, which manifested as buggy arrow materials on the first segment of polylines on terrain.

When packing vertex attributes to polyline volumes, we attach to each vertex a pair of floats that are used to compute each fragment's distance along the polyline as a whole from its texcoords within the segment - there's a scale term and a translation term. We also use the sign and/or magnitude of these terms to indicate in the shader whether a vertex is on the top, bottom, right, or left side of the shadow volume.

The translation term should always be in the range [0, 1) since it's a translation in texcoord space, so we say that if the value packed is negative or if it has a magnitude greater than 1.0, then the vertex is on the "bottom" of the volume. The value then needs to be remapped to [0, 1).

The "magnitude greater than 1.0" part is there because the first volume has a translation term 0.0. We were always setting it to Number.POSITIVE_INFINITY in this case as a value clearly outside the [0, 1) range, but it looks like on some platforms that can have reliability problems when remapping, so this PR brings it down to a very reasonable 9.0 instead.

Sandcastle for comparison:

@likangning93 likangning93 requested a review from lilleyse July 22, 2019 11:36
@cesium-concierge
Copy link

Thanks for the pull request @likangning93!

  • ✔️ Signed CLA found.
  • ❔ Unit tests were not updated.
    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

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

@lilleyse
Copy link
Contributor

Thanks @likangning93, and good timing too.

@lilleyse lilleyse merged commit 18e0d0c into master Jul 22, 2019
@lilleyse lilleyse deleted the fixGroundPolylineTexcoords branch July 22, 2019 23:11
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.

Clamped to ground arrow polyline renders a regular line between the first 2 positions
3 participants