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

Fixed "Scene/GroundPolylinePrimitive adds width instance attribute" #8197

Merged
merged 1 commit into from
Sep 20, 2019

Conversation

IanLilleyT
Copy link
Contributor

This test failed on mac when rendering a 0 width ground polyline, likely due to a small gpu/driver discrepancy that left a tiny sliver on the screen. So now this test uses an offset look position that a width 1 polyline is inside but a width 0 one is not.

@cesium-concierge
Copy link

cesium-concierge commented Sep 19, 2019

Thanks for the pull request @IanLilleyT!

  • ✔️ 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.

@likangning93
Copy link
Contributor

Works (or fails gracefully) in IE11.

Unable to check because specs are broken in IE11 (#8083 (comment)), but it shouldn't be a problem because these specs aren't supported in IE11 and should just be returning without testing anyway.

@likangning93
Copy link
Contributor

Thanks @IanLilleyT, looks to be working as advertised. I'll verify on another mac shortly.

@likangning93
Copy link
Contributor

Confirmed working in Safari on a 2015 Macbook Pro (GT 750 model) running in Intel mode, although for some reason the og test also passes in this configuration. It might be an AMD specific thing. Anyway, thanks @lilleyse!

@likangning93 likangning93 merged commit c8eec8c into master Sep 20, 2019
@likangning93 likangning93 deleted the groundPolylineTestFix branch September 20, 2019 12:41
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