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

A few surface contours fixes #2712

Merged
merged 3 commits into from
Jun 11, 2018
Merged

A few surface contours fixes #2712

merged 3 commits into from
Jun 11, 2018

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Jun 8, 2018

Fixes two surface contour line bugs uncovered in @alexcjohnson 's https://codepen.io/alexcjohnson/pen/gKLBgR?editors=0010:

  • bug 1: contour directions (x,y or z) that have a 0-length first level (these are computed in gl-surface3d) did not pass the correct contour color to the shaders. See Fix contours when first level has length zero gl-vis/gl-surface3d#11
  • bug 2: contour.(x|y|z).highlight wasn't working properly for the same mock (possibly due to that same 0-length first level condition)

Note that both these bugs aren't noticeably on our https://rreusser.github.io/plotly-mock-viewer/#gl3d_contour-lines

etpinard added 2 commits June 8, 2018 14:17

Unverified

This user has not yet uploaded their public signing key.
@etpinard etpinard added bug something broken status: reviewable labels Jun 8, 2018
@alexcjohnson
Copy link
Collaborator

Awesome, thanks for taking a look at this!

There's a 3rd bug in that codepen (and your new mock here), that contour lines I expected to be straight are not rendered straight. It looks to me as though it's actually the surface that's not getting drawn in quite the right place - perhaps it's trying to smooth the sharp edges a bit or something - and then the contours are at the correct coordinates but faithfully following the incorrect surface. See what happens to hover when I zoom in on the corner of the bottom:
screen shot 2018-06-08 at 2 47 27 pm

Anyway, this is still 2 bugs down so lets get it in! 💃
Do you want me to make a new issue for the 3rd one?

@etpinard
Copy link
Contributor Author

etpinard commented Jun 8, 2018

Do you want me to make a new issue for the 3rd one?

That would be great!

@etpinard etpinard merged commit 69a1004 into master Jun 11, 2018
@etpinard etpinard deleted the surface-contours-fixes branch June 11, 2018 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants