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

Implement scatter cliponaxis: false #1824

Closed
wants to merge 10 commits into from
Closed

Conversation

etpinard
Copy link
Contributor

Resolves #1385

In brief, this PR adds a new layer to each cartesian and ternary (and eventually geo) subplot where the clipping <rect> isn't applied. Instead, points (i.e. markers, text nodes and error bars) are removed if their (x,y) or (a,b,c) coordinates are outside the subplot's current axis range box as discussed in #1385 (comment).

Note that, line and fills are never plotted in the noclip layer. This fact led to some non-trivial changes in scatter/plot.js. I would enjoy a thorough review from @alexcjohnson and @rreusser there. That said, as the scatterternary module reuses scatter/plot.js, implement the cliponaxis: false behavior was pretty DRY 馃尨.

TODO:

  • agree on attribute name (cc @cldougl )
  • test animations (cc @rreusser might need your help here)
  • test overlaying axes

Maybes:

  • implement cliponaxis: false for scattergeo (might leave this for another PR)

cc @geocosmite

- N.B. dflt and current behavior has `cliponaxis: true`.
- with nested <g.scatterlayer> to implement `cliponaxis: false`
- apply on ax._offset values to this new layer, but
  don't attach clip path!
- factor out markers/text nodes renderer out plotOne
- add plotOneNoClip which draw markers, text nodes and errorbars
  in the plotnoclip layer
- always draw lines and fills in regular (clipped) plot layer!
- use in Drawing.translatePoint to clear
  out-of-range markers/text nodes when `cliponaxis: false`
- mock plotinfo.plotnoclip before calling scatter/plot.js
- override isPtWithinRange methods for ternary
@alexcjohnson
Copy link
Collaborator

Right now the layering depends on cliponaxis - I think we need to preserve the previous behavior, which probably means keeping the layer structure as it was but pushing the clipping down to the level of point groups and line groups. One simple example where this is not desired:

Plotly.newPlot(gd,[
    {y: [1,1,1,1,1], mode:'markers+lines', marker: {size: 20}},
    {x:[0, 1.5, 2.5, 4], y: [0, 2, 0, 2], mode: 'markers+lines', marker: {size: 20}}
],
{width: 400, height: 400, xaxis: {range: [0, 4]}, yaxis: {range: [0, 2]}})

screen shot 2017-06-27 at 6 24 09 pm

Plotly.restyle(gd,'cliponaxis',false)

screen shot 2017-06-27 at 6 24 35 pm

The blue points are no longer behind the orange lines.

I guess if you have two marker traces there would be an even more dramatic effect if you made the first one cliponaxis: false and the second true: the markers of the first would jump on top of the second. And if you had a dense field of markers from one trace, with a sparse 'lines+markers' in front of it, both cliponaxis: false, the top trace would lose its lines and have disconnected markers visible...

Also: we should recalculate visible markers during zoom/pan (as well as during animation, which you already mentioned), so we don't get situations like these (dragging the top right corner out and then in):
screen shot 2017-06-27 at 6 36 33 pm
screen shot 2017-06-27 at 6 36 43 pm

@alexcjohnson
Copy link
Collaborator

re: attribute name: Since this applies only to markers I'd put it in the marker container, but I'm not sure what to call it. Just to throw some things out there:
marker.edgeeffect: 'clip'/'hide' ('hide': never clip, only hide out of range markers)
marker.overflow: true/false (true: allow markers to overflow the subplot bounds)

@etpinard
Copy link
Contributor Author

Right now the layering depends on cliponaxis - I think we need to preserve the previous behavior, which probably means keeping the layer structure as it was but pushing the clipping down to the level of point groups and line groups.

Yeah, I thought about that, but didn't think it would be a big deal. Guess I was wrong.

Since this applies only to markers I'd put it in the marker container,

I guess you didn't see the part about text nodes and error bars.

@etpinard
Copy link
Contributor Author

Also: we should recalculate visible markers during zoom/pan (as well as during animation, which you already mentioned), so we don't get situations like these (dragging the top right corner out and then in):

That too, I didn't think this would be a big deal. I suspect 99% of the use cases for cliponaxis: false would be for image export.

@geocosmite
Copy link

I'm glad that you guys are doing such a thorough job in considering the edge cases.

By the way, for scientific plots my intuition would be the opposite of yours: 99% of the use cases will not want the symbols to be truncated by the axis boundaries irrespective of the zoom. A survey of our users (all scientists admittedly) found that not a single one of them could come up with a use case where symbol truncation was a desirable feature. Based on this response our current plan is hardwire cliponaxis: false.

@alexcjohnson
Copy link
Collaborator

Since this applies only to markers I'd put it in the marker container,

I guess you didn't see the part about text nodes and error bars.

Ah right, sorry. Text should work this way for sure, so the attribute doesn't belong in marker.

I'm not sure about error bars though (apologies for not noticing the comments on this last week) - my gut reaction is to not include them outside the clip area, since (unlike markers and text) their ends correspond to actual axis values, so if the axis value at the end of an error bar is not displayed, then the bar should not be displayed either. I suppose you could argue that the caps should not be clipped, in the direction perpendicular to the errors, but that's adding a whole other layer of complexity for what seems like little benefit.

@etpinard
Copy link
Contributor Author

99% of the use cases will not want the symbols to be truncated by the axis boundaries irrespective of the zoom.

That's not what I meant. I was referring to interacting by panning or via the zoom box on a cliponaxis: false graph isn't much of a gain compared to the current behavior.

@etpinard
Copy link
Contributor Author

Closing this thing. 馃槨

@etpinard etpinard closed this Jun 28, 2017
@etpinard etpinard deleted the scatter-cliponaxis-false branch June 28, 2017 01:49
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.

None yet

3 participants