Skip to content

Commit

Permalink
Fix for hover behavior on scatter fills tonextx/y.
Browse files Browse the repository at this point in the history 
Previous code would not correctly construct the polygons
used for detection whether the cursor is within the fill
region of a trace.

Some of the previously created tests fail with this as the
hover points in question cannot currently be correctly
detected using the detection polygons.
  • Loading branch information
@lumip
lumip committed Jan 23, 2024
1 parent 4628741 commit be658dd
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/traces/scatter/hover.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
}

// even if hoveron is 'fills', only use it if we have polygons too
if(hoveron.indexOf('fills') !== -1 && trace._polygons) {
if(hoveron.indexOf('fills') !== -1 && trace._polygons && trace._polygons.length > 0) {
var polygons = trace._polygons;
var polygonsIn = [];
var inside = false;
Expand Down
132 changes: 114 additions & 18 deletions src/traces/scatter/plot.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,17 +143,28 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
var ownFillDir = trace.fill.charAt(trace.fill.length - 1);
if(ownFillDir !== 'x' && ownFillDir !== 'y') ownFillDir = '';

var fillAxisIndex, fillAxisZero;
if(ownFillDir === 'y') {
fillAxisIndex = 1;
fillAxisZero = ya.c2p(0, true);
} else if(ownFillDir === 'x') {
fillAxisIndex = 0;
fillAxisZero = xa.c2p(0, true);
}

// store node for tweaking by selectPoints
cdscatter[0][plotinfo.isRangePlot ? 'nodeRangePlot3' : 'node3'] = tr;

var prevRevpath = '';
var prevPolygons = [];
var prevtrace = trace._prevtrace;
var prevFillsegments = null;

if(prevtrace) {
prevRevpath = prevtrace._prevRevpath || '';
tonext = prevtrace._nextFill;
prevPolygons = prevtrace._polygons;
prevPolygons = prevtrace._ownPolygons;
prevFillsegments = prevtrace._fillsegments;
}

var thispath;
Expand All @@ -166,7 +177,15 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
// functions for converting a point array to a path
var pathfn, revpathbase, revpathfn;
// variables used before and after the data join
var pt0, lastSegment, pt1, thisPolygons;
var pt0, lastSegment, pt1;

// thisPolygons always contains only the polygons of this trace only
// whereas trace._polygons may be extended to include those of the previous
// trace as well for exclusion during hover detection
var thisPolygons = [];
trace._polygons = [];

var fillsegments = [];

// initialize line join data / method
var segments = [];
Expand Down Expand Up @@ -220,35 +239,51 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
});

// since we already have the pixel segments here, use them to make
// polygons for hover on fill
// polygons for hover on fill; we first merge segments where the fill
// is connected into "fillsegments"; the actual polygon construction
// is deferred to later to distinguish between self and tonext/tozero fills.
// TODO: can we skip this if hoveron!=fills? That would mean we
// need to redraw when you change hoveron...
thisPolygons = trace._polygons = new Array(segments.length);
fillsegments = new Array(segments.length);
var fillsegmentCount = 0;
for(i = 0; i < segments.length; i++) {
trace._polygons[i] = polygonTester(segments[i]);
var curpoints;
var pts = segments[i];
if(!curpoints || !ownFillDir) {
curpoints = pts.slice();
fillsegments[fillsegmentCount] = curpoints;
fillsegmentCount++;
} else {
curpoints.push.apply(curpoints, pts);
}
}
trace._fillsegments = fillsegments.slice(0, fillsegmentCount);
fillsegments = trace._fillsegments;

if(segments.length) {
pt0 = segments[0][0];
pt0 = segments[0][0].slice();
lastSegment = segments[segments.length - 1];
pt1 = lastSegment[lastSegment.length - 1];
pt1 = lastSegment[lastSegment.length - 1].slice();
}

makeUpdate = function(isEnter) {
return function(pts) {
thispath = pathfn(pts);
thisrevpath = revpathfn(pts);
thisrevpath = revpathfn(pts); // side-effect: reverses input
// calculate SVG path over all segments for fills
if(!fullpath) {
fullpath = thispath;
revpath = thisrevpath;
} else if(ownFillDir) {
// for fills with fill direction: ignore gaps
fullpath += 'L' + thispath.substr(1);
revpath = thisrevpath + ('L' + revpath.substr(1));
} else {
fullpath += 'Z' + thispath;
revpath = thisrevpath + 'Z' + revpath;
}

// actual lines get drawn here, with gaps between segments if requested
if(subTypes.hasLines(trace)) {
var el = d3.select(this);

Expand Down Expand Up @@ -290,16 +325,58 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
transition(selection).attr('d', 'M0,0Z');
}

// helper functions to create polygons for hoveron fill detection
var makeSelfPolygons = function() {
var polygons = new Array(fillsegments.length);
for(i = 0; i < fillsegments.length; i++) {
polygons[i] = polygonTester(fillsegments[i]);
}
return polygons;
};

var makePolygonsToPrevious = function(prevFillsegments) {
var polygons, i;
if(!prevFillsegments || prevFillsegments.length === 0) {
// if there are no fill segments of a previous trace, stretch the
// polygon to the relevant axis
polygons = new Array(fillsegments.length);
for(i = 0; i < fillsegments.length; i++) {
var pt0 = fillsegments[i][0].slice();
var pt1 = fillsegments[i][fillsegments[i].length - 1].slice();

pt0[fillAxisIndex] = pt1[fillAxisIndex] = fillAxisZero;

var zeropoints = [pt1, pt0];
var polypoints = zeropoints.concat(fillsegments[i]);
polygons[i] = polygonTester(polypoints);
}
} else {
// if there are more than one previous fill segment, the
// way that fills work is to "self" fill all but the last segments
// of the previous and then fill from the new trace to the last
// segment of the previous.
polygons = new Array(prevFillsegments.length - 1 + fillsegments.length);
for(i = 0; i < prevFillsegments.length - 1; i++) {
polygons[i] = polygonTester(prevFillsegments[i]);
}

var reversedPrevFillsegment = prevFillsegments[prevFillsegments.length - 1].slice();
reversedPrevFillsegment.reverse();

for(i = 0; i < fillsegments.length; i++) {
polygons[prevFillsegments.length - 1 + i] = polygonTester(fillsegments[i].concat(reversedPrevFillsegment));
}
}
return polygons;
};

// draw fills and create hover detection polygons
if(segments.length) {
if(ownFillEl3) {
ownFillEl3.datum(cdscatter);
if(pt0 && pt1) {
if(pt0 && pt1) { // TODO(2023-12-10): this is always true if segments is not empty (?)
if(ownFillDir) {
if(ownFillDir === 'y') {
pt0[1] = pt1[1] = ya.c2p(0, true);
} else if(ownFillDir === 'x') {
pt0[0] = pt1[0] = xa.c2p(0, true);
}
pt0[fillAxisIndex] = pt1[fillAxisIndex] = fillAxisZero;

// fill to zero: full trace path, plus extension of
// the endpoints to the appropriate axis
Expand All @@ -308,12 +385,19 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
// animations get a little crazy if the number of points changes.
transition(ownFillEl3).attr('d', 'M' + pt1 + 'L' + pt0 + 'L' + fullpath.substr(1))
.call(Drawing.singleFillStyle, gd);

// create hover polygons that extend to the axis as well.
thisPolygons = makePolygonsToPrevious(null); // polygon to axis
} else {
// fill to self: just join the path to itself
transition(ownFillEl3).attr('d', fullpath + 'Z')
.call(Drawing.singleFillStyle, gd);

// and simply emit hover polygons for each segment
thisPolygons = makeSelfPolygons();
}
}
trace._polygons = thisPolygons;
} else if(tonext) {
if(trace.fill.substr(0, 6) === 'tonext' && fullpath && prevRevpath) {
// fill to next: full trace path, plus the previous path reversed
Expand All @@ -324,6 +408,13 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
// inside the other, but then that is a strange usage.
transition(tonext).attr('d', fullpath + 'Z' + prevRevpath + 'Z')
.call(Drawing.singleFillStyle, gd);

// and simply emit hover polygons for each segment
thisPolygons = makeSelfPolygons();

// we add the polygons of the previous trace which causes hover
// detection to ignore points contained in them.
trace._polygons = thisPolygons.concat(prevPolygons); // this does not modify thisPolygons, on purpose
} else {
// tonextx/y: for now just connect endpoints with lines. This is
// the correct behavior if the endpoints are at the same value of
Expand All @@ -332,20 +423,25 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
// existing curve or off the end of it
transition(tonext).attr('d', fullpath + 'L' + prevRevpath.substr(1) + 'Z')
.call(Drawing.singleFillStyle, gd);

// create hover polygons that extend to the previous trace.
thisPolygons = makePolygonsToPrevious(prevFillsegments);

// in this case our polygons do not cover that of previous traces,
// so must not include previous trace polygons for hover detection.
trace._polygons = thisPolygons;
}
trace._polygons = trace._polygons.concat(prevPolygons);
} else {
clearFill(tonext);
trace._polygons = null;
}
}
trace._prevRevpath = revpath;
trace._prevPolygons = thisPolygons;
} else {
if(ownFillEl3) clearFill(ownFillEl3);
else if(tonext) clearFill(tonext);
trace._polygons = trace._prevRevpath = trace._prevPolygons = null;
trace._prevRevpath = null;
}
trace._ownPolygons = thisPolygons;


function visFilter(d) {
Expand Down

0 comments on commit be658dd

Please sign in to comment.