Skip to content

Commit 1106180

Browse files
committed
towards better handling of closing point
1 parent 47870fa commit 1106180

File tree

5 files changed

+97
-99
lines changed

5 files changed

+97
-99
lines changed

src/components/shapes/draw.js

-1
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ function drawOne(gd, index) {
144144
element: path.node(),
145145
plotinfo: plotinfo,
146146
gd: gd,
147-
dragmode: gd._fullLayout.dragmode,
148147
isActiveShape: true // i.e. to enable controllers
149148
};
150149

src/plots/cartesian/new_shape.js

+70-81
Original file line numberDiff line numberDiff line change
@@ -39,27 +39,21 @@ var handleOutline = require('./handle_outline');
3939
var clearOutlineControllers = handleOutline.clearOutlineControllers;
4040
var clearSelect = handleOutline.clearSelect;
4141

42-
function recordPositions(polygonsOut, polygonsIn) { // copy & clean (i.e. skip duplicates)
42+
function recordPositions(polygonsOut, polygonsIn) {
4343
for(var i = 0; i < polygonsIn.length; i++) {
44+
var cell = polygonsIn[i];
4445
polygonsOut[i] = [];
45-
var len = polygonsIn[i].length;
46-
for(var newJ = 0, j = 0; j < len; j++) {
47-
// skip close points
48-
if(j > 0 && dist(polygonsIn[i][j], polygonsIn[i][(j + 1) % len]) < 1) continue;
49-
50-
polygonsOut[i][newJ] = [];
51-
for(var k = 0; k < polygonsIn[i][newJ].length; k++) {
52-
polygonsOut[i][newJ][k] = polygonsIn[i][j][k];
46+
for(var j = 0; j < cell.length; j++) {
47+
polygonsOut[i][j] = [];
48+
for(var k = 0; k < cell[j].length; k++) {
49+
polygonsOut[i][j][k] = cell[j][k];
5350
}
54-
newJ++;
5551
}
5652
}
5753
return polygonsOut;
5854
}
5955

60-
function displayOutlines(polygonsIn, outlines, dragOptions, nCalls) {
61-
var polygons = recordPositions([], polygonsIn);
62-
56+
function displayOutlines(polygons, outlines, dragOptions, nCalls) {
6357
if(!nCalls) nCalls = 0;
6458

6559
var gd = dragOptions.gd;
@@ -87,12 +81,11 @@ function displayOutlines(polygonsIn, outlines, dragOptions, nCalls) {
8781

8882
var dragmode = dragOptions.dragmode;
8983
var isDrawMode = drawMode(dragmode);
90-
var isOpenMode = openMode(dragmode);
9184

9285
if(isDrawMode) gd._fullLayout._drawing = true;
9386

9487
// make outline
95-
outlines.attr('d', writePaths(polygons, isOpenMode));
88+
outlines.attr('d', writePaths(polygons));
9689

9790
// add controllers
9891
var rVertexController = MINSELECT * 1.5; // bigger vertex buttons
@@ -337,24 +330,29 @@ function displayOutlines(polygonsIn, outlines, dragOptions, nCalls) {
337330
}
338331
}
339332

340-
function writePaths(polygons, isOpenMode) {
333+
function writePaths(polygons) {
341334
var nI = polygons.length;
342335
if(!nI) return 'M0,0Z';
343336

344337
var str = '';
345338
for(var i = 0; i < nI; i++) {
346339
var nJ = polygons[i].length;
347340
for(var j = 0; j < nJ; j++) {
348-
var nK = polygons[i][j].length;
349-
for(var k = 0; k < nK; k++) {
350-
str += polygons[i][j][k];
351-
if(k > 0 && k < nK - 1) {
352-
str += ',';
341+
if(polygons[i][j][0] === 'Z') {
342+
str += 'Z';
343+
break;
344+
} else {
345+
var nK = polygons[i][j].length;
346+
for(var k = 0; k < nK; k++) {
347+
str += polygons[i][j][k];
348+
if(k > 0 && k < nK - 1) {
349+
str += ',';
350+
}
353351
}
354352
}
355353
}
356-
if(!isOpenMode) str += 'Z';
357354
}
355+
358356
return str;
359357
}
360358

@@ -434,41 +432,44 @@ function readPaths(str, plotinfo, size, isActiveShape) {
434432
}
435433

436434
break;
437-
}
438435

439-
if(c === 'Z') {
440-
x = initX;
441-
y = initY;
442-
} else {
443-
for(var j = 0; j < newPos.length; j++) {
444-
x = newPos[j][0];
445-
y = newPos[j][1];
446-
447-
if(!plotinfo || !(plotinfo.xaxis && plotinfo.yaxis)) {
448-
polys[n].push([
449-
w,
450-
x,
451-
y
452-
]);
453-
} else if(plotinfo.domain) {
454-
polys[n].push([
455-
w,
456-
plotinfo.domain.x[0] + x / size.w,
457-
plotinfo.domain.y[1] - y / size.h
458-
]);
459-
} else if(isActiveShape === false) {
460-
polys[n].push([
461-
w,
462-
p2r(plotinfo.xaxis, x - plotinfo.xaxis._offset),
463-
p2r(plotinfo.yaxis, y - plotinfo.yaxis._offset)
464-
]);
465-
} else {
466-
polys[n].push([
467-
w,
468-
p2r(plotinfo.xaxis, x),
469-
p2r(plotinfo.yaxis, y)
470-
]);
436+
case 'Z':
437+
if(x !== initX || y !== initY) {
438+
x = initX;
439+
y = initY;
440+
newPos.push([x, y]);
471441
}
442+
break;
443+
}
444+
445+
for(var j = 0; j < newPos.length; j++) {
446+
x = newPos[j][0];
447+
y = newPos[j][1];
448+
449+
if(!plotinfo || !(plotinfo.xaxis && plotinfo.yaxis)) {
450+
polys[n].push([
451+
w,
452+
x,
453+
y
454+
]);
455+
} else if(plotinfo.domain) {
456+
polys[n].push([
457+
w,
458+
plotinfo.domain.x[0] + x / size.w,
459+
plotinfo.domain.y[1] - y / size.h
460+
]);
461+
} else if(isActiveShape === false) {
462+
polys[n].push([
463+
w,
464+
p2r(plotinfo.xaxis, x - plotinfo.xaxis._offset),
465+
p2r(plotinfo.yaxis, y - plotinfo.yaxis._offset)
466+
]);
467+
} else {
468+
polys[n].push([
469+
w,
470+
p2r(plotinfo.xaxis, x),
471+
p2r(plotinfo.yaxis, y)
472+
]);
472473
}
473474
}
474475
}
@@ -518,9 +519,10 @@ function calcMax(cell, dim) {
518519
return v;
519520
}
520521

521-
function pointsShapeRectangle(cell, len) {
522-
if(!len) len = cell.length;
523-
if(len !== 4) return false;
522+
function pointsShapeRectangle(cell) {
523+
var len = cell.length;
524+
if(len !== 5) return false;
525+
524526
for(var j = 1; j < 3; j++) {
525527
var e01 = cell[0][j] - cell[1][j];
526528
var e32 = cell[3][j] - cell[2][j];
@@ -545,10 +547,12 @@ function pointsShapeRectangle(cell, len) {
545547
);
546548
}
547549

548-
function pointsShapeEllipse(cell, len) {
549-
if(!len) len = cell.length;
550-
if(len !== CIRCLE_SIDES) return false;
550+
function pointsShapeEllipse(cell) {
551+
var len = cell.length;
552+
if(len !== CIRCLE_SIDES + 1) return false;
553+
551554
// opposite diagonals should be the same
555+
len = CIRCLE_SIDES;
552556
for(var i = 0; i < len; i++) {
553557
var k = (len * 2 - i) % len;
554558

@@ -660,30 +664,15 @@ function addNewShapes(outlines, dragOptions) {
660664
}
661665
}
662666
}
667+
663668
var isOpenMode = openMode(dragmode);
664669

665670
var polygons = readPaths(d, plotinfo, gd._fullLayout._size, isActiveShape);
666-
if(isOpenMode) {
667-
var last = polygons[0].length - 1;
668-
if( // ensure first and last positions are not the same on an open path
669-
polygons[0][0][1] === polygons[0][last][1] &&
670-
polygons[0][0][2] === polygons[0][last][2]
671-
) {
672-
polygons[0].pop();
673-
}
674-
}
675671

676672
var newShapes = [];
677673
for(var i = 0; i < polygons.length; i++) {
678674
var cell = polygons[i];
679-
var len = cell.length;
680-
if(
681-
cell[0][1] === cell[len - 1][1] &&
682-
cell[0][2] === cell[len - 1][2]
683-
) {
684-
len -= 1;
685-
}
686-
if(len < 2) continue;
675+
if(cell.length < 2) continue;
687676

688677
var shape = {
689678
editable: true,
@@ -707,7 +696,7 @@ function addNewShapes(outlines, dragOptions) {
707696

708697
if(
709698
dragmode === 'rectdraw' &&
710-
pointsShapeRectangle(cell, len) // should pass len here which is equal to cell.length - 1 i.e. because of the closing point
699+
pointsShapeRectangle(cell)
711700
) {
712701
shape.type = 'rect';
713702
shape.x0 = cell[0][1];
@@ -724,7 +713,7 @@ function addNewShapes(outlines, dragOptions) {
724713
shape.y1 = cell[1][2];
725714
} else if(
726715
dragmode === 'ellipsedraw' &&
727-
(isActiveShape === false || pointsShapeEllipse(cell, len)) // should pass len here which is equal to cell.length - 1 i.e. because of the closing point
716+
(isActiveShape === false || pointsShapeEllipse(cell))
728717
) {
729718
shape.type = 'circle'; // an ellipse!
730719
var pos = {};
@@ -758,7 +747,7 @@ function addNewShapes(outlines, dragOptions) {
758747
fixDatesOnPaths(cell, xaxis, yaxis);
759748
}
760749

761-
shape.path = writePaths([cell], isOpenMode);
750+
shape.path = writePaths([cell]);
762751
}
763752

764753
newShapes.push(shape);

src/plots/cartesian/select.js

+13-3
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
282282
}
283283

284284
// display polygons on the screen
285-
displayOutlines(convertPoly(mergedPolygons), outlines, dragOptions);
285+
displayOutlines(convertPoly(mergedPolygons, isOpenMode), outlines, dragOptions);
286286

287287
if(isSelectMode) {
288288
throttle.throttle(
@@ -457,9 +457,10 @@ function selectOnClick(evt, gd, xAxes, yAxes, subplot, dragOptions, polygonOutli
457457

458458
if(polygonOutlines) {
459459
var polygons = dragOptions.mergedPolygons;
460+
var isOpenMode = openMode(dragOptions.dragmode);
460461

461462
// display polygons on the screen
462-
displayOutlines(convertPoly(polygons), polygonOutlines, dragOptions);
463+
displayOutlines(convertPoly(polygons, isOpenMode), polygonOutlines, dragOptions);
463464
}
464465

465466
if(sendEvents) {
@@ -902,7 +903,7 @@ function fillSelectionItem(selection, searchInfo) {
902903
return selection;
903904
}
904905

905-
function convertPoly(polygonsIn) { // add M and L command to draft positions
906+
function convertPoly(polygonsIn, isOpenMode) { // add M and L command to draft positions
906907
var polygonsOut = [];
907908
for(var i = 0; i < polygonsIn.length; i++) {
908909
polygonsOut[i] = [];
@@ -915,7 +916,16 @@ function convertPoly(polygonsIn) { // add M and L command to draft positions
915916
);
916917
}
917918
}
919+
920+
if(!isOpenMode) {
921+
polygonsOut[i].push([
922+
'Z',
923+
polygonsOut[i][0][1], // initial x
924+
polygonsOut[i][0][2] // initial y
925+
]);
926+
}
918927
}
928+
919929
return polygonsOut;
920930
}
921931

test/jasmine/tests/cartesian_interact_test.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -1946,15 +1946,15 @@ describe('axis zoom/pan and main plot zoom', function() {
19461946
hasDragData: true,
19471947
selectingCnt: 1,
19481948
selectedCnt: 0,
1949-
selectOutline: 'M20,20L20,220L220,220L220,20L20,20Z'
1949+
selectOutline: 'M20,20L20,220L220,220L220,20Z'
19501950
}))
19511951
.then(delay(100))
19521952
.then(_assert('while holding on mouse', {
19531953
nodeCnt: 4,
19541954
hasDragData: true,
19551955
selectingCnt: 1,
19561956
selectedCnt: 0,
1957-
selectOutline: 'M20,20L20,220L220,220L220,20L20,20Z'
1957+
selectOutline: 'M20,20L20,220L220,220L220,20Z'
19581958
}))
19591959
.then(drag.end);
19601960
})

0 commit comments

Comments
 (0)