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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix surface colormap discontinuity for circular colorscales #959

Merged
merged 2 commits into from
Sep 22, 2016

Conversation

etpinard
Copy link
Contributor

fixes #940

This PR implements @dfcreative's gl-vis/gl-surface3d#8 patch in plotly.js. Big ups! 馃嵒

This PR adds logic for circular colorscale in surface traces as discussed in gl-vis/gl-surface3d#8 (comment)

I chose to not add a new colorscale attribute for the moment as the behavior for circular colorscale is on par with 2D heatmap and contour traces.

- bump gl-surface3d dep to 1.3.0
- set 'vertexColor' to true when colormap is circular
  e.g. in complex plane surface
@rreusser
Copy link
Contributor

rreusser commented Sep 22, 2016

Only potential blocker from my point of view: how many of the existing colormaps are circular? It seems preferable that the evaluation mode not suddenly change for most colormaps (IMO to a sometimes-desirable but usually-slightly-inferior method). It would be nice if it were sorta opt-in by choosing a colormap that's intentionally circular (or at least that it's not most of them that are affected).

@etpinard
Copy link
Contributor Author

how many of the existing colormaps are circular?

Of the built-in plotly colorscales you mean? In that case: none.

It seems preferable that the evaluation mode not suddenly change for most colormaps

I disagree. To me, this PR is simply a bug fix.

Can you think of a use case where a user that sets a circular colorscale would want to display a gap in the resulting colormap?

@rreusser
Copy link
Contributor

rreusser commented Sep 22, 2016

Ah okay. I'm 100% content then. If that's the case, then I agree that this is a perfectly good way to encode the intent, making this a nice little feature/bug fix. 馃憤馃徏

@etpinard etpinard merged commit b2b68cd into master Sep 22, 2016
@etpinard etpinard deleted the surface-colormap-discontinuity branch September 22, 2016 15:13
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.

Color mapping discontinuity in surface surfacecolor
2 participants