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.

Sign up for GitHub

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

Jump to bottom

Record group selection in transformed coordinates to support panning/zooming while group selecting #7088

Merged
merged 3 commits into from
Jul 4, 2021
Merged

Record group selection in transformed coordinates to support panning/zooming while group selecting #7088

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c874f9d
Record group selection in transformed coordinates to support panning/…
May 28, 2021
89b0f0b
remove ie10 block
asturur Jul 3, 2021
32283a7
Update canvas.class.js
asturur Jul 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 21 additions & 29 deletions src/canvas.class.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@

var getPointer = fabric.util.getPointer,
degreesToRadians = fabric.util.degreesToRadians,
abs = Math.abs,
supportLineDash = fabric.StaticCanvas.supports('setLineDash'),
isTouchEvent = fabric.util.isTouchEvent,
STROKE_OFFSET = 0.5;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think STROKE_OFFSET is still needed. it is used because mouse events are always round numbers, while we want to draw single pixels line at 0.5.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly, I think this actually to account for canvas drawing strokes down the center of the line. The hardcoded stroke offset of 0.5 works well for the default selectionLineWidth of 1, but doesn't look right for larger values. I think what I did accomplishes the same thing but allows for selectionLineWidth != 1

isTouchEvent = fabric.util.isTouchEvent;

/**
* Canvas class
Expand Down Expand Up @@ -689,21 +687,20 @@
* @param {CanvasRenderingContext2D} ctx to draw the selection on
*/
_drawSelection: function (ctx) {
var groupSelector = this._groupSelector,
left = groupSelector.left,
top = groupSelector.top,
aleft = abs(left),
atop = abs(top);
var selector = this._groupSelector,
viewportStart = new fabric.Point(selector.ex, selector.ey),
start = fabric.util.transformPoint(viewportStart, this.viewportTransform),
viewportExtent = new fabric.Point(selector.ex + selector.left, selector.ey + selector.top),
extent = fabric.util.transformPoint(viewportExtent, this.viewportTransform),
minX = Math.min(start.x, extent.x),
minY = Math.min(start.y, extent.y),
maxX = Math.max(start.x, extent.x),
maxY = Math.max(start.y, extent.y),
strokeOffset = this.selectionLineWidth / 2;

if (this.selectionColor) {
ctx.fillStyle = this.selectionColor;

ctx.fillRect(
groupSelector.ex - ((left > 0) ? 0 : -left),
groupSelector.ey - ((top > 0) ? 0 : -top),
aleft,
atop
);
ctx.fillRect(minX, minY, maxX - minX, maxY - minY);
}

if (!this.selectionLineWidth || !this.selectionBorderColor) {
Expand All @@ -712,30 +709,25 @@
ctx.lineWidth = this.selectionLineWidth;
ctx.strokeStyle = this.selectionBorderColor;

minX += strokeOffset;
minY += strokeOffset;
maxX -= strokeOffset;
maxY -= strokeOffset;
// selection border
if (this.selectionDashArray.length > 1 && !supportLineDash) {

var px = groupSelector.ex + STROKE_OFFSET - ((left > 0) ? 0 : aleft),
py = groupSelector.ey + STROKE_OFFSET - ((top > 0) ? 0 : atop);

ctx.beginPath();

fabric.util.drawDashedLine(ctx, px, py, px + aleft, py, this.selectionDashArray);
fabric.util.drawDashedLine(ctx, px, py + atop - 1, px + aleft, py + atop - 1, this.selectionDashArray);
fabric.util.drawDashedLine(ctx, px, py, px, py + atop, this.selectionDashArray);
fabric.util.drawDashedLine(ctx, px + aleft - 1, py, px + aleft - 1, py + atop, this.selectionDashArray);
fabric.util.drawDashedLine(ctx, minX, minY, maxX, minY, this.selectionDashArray);
fabric.util.drawDashedLine(ctx, minX, maxY, maxX, maxY, this.selectionDashArray);
fabric.util.drawDashedLine(ctx, minX, minY, minX, maxY, this.selectionDashArray);
fabric.util.drawDashedLine(ctx, maxX, minY, maxX, maxY, this.selectionDashArray);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you check if IE11 supports dashed stroke? if it does, we can finally get rid of the block (!supportedLineDash)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the support link. Looks like IE 11 supports but I'm not sure if the support is good enough on other browsers like chrome? https://caniuse.com/mdn-api_canvasrenderingcontext2d_setlinedash

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes setLineDash is an old feature that was missing on ie10 only.
Since we do not support ie10 anymore, this is safe to go.


ctx.closePath();
ctx.stroke();
}
else {
fabric.Object.prototype._setLineDash.call(this, ctx, this.selectionDashArray);
ctx.strokeRect(
groupSelector.ex + STROKE_OFFSET - ((left > 0) ? 0 : aleft),
groupSelector.ey + STROKE_OFFSET - ((top > 0) ? 0 : atop),
aleft,
atop
);
ctx.strokeRect(minX, minY, maxX - minX, maxY - minY);
}
},

Expand Down
6 changes: 3 additions & 3 deletions src/mixins/canvas_events.mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,8 +712,8 @@
if (this.selection && (!target ||
(!target.selectable && !target.isEditing && target !== this._activeObject))) {
this._groupSelector = {
ex: pointer.x,
ey: pointer.y,
ex: this._absolutePointer.x,
ey: this._absolutePointer.y,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dream a day in which the local pointer goes away entirely and fabric uses only the absolute values, and we remove most of the conversions.

top: 0,
left: 0
};
Expand Down Expand Up @@ -806,7 +806,7 @@

// We initially clicked in an empty area, so we draw a box for multiple selection
if (groupSelector) {
pointer = this._pointer;
pointer = this._absolutePointer;

groupSelector.left = pointer.x - groupSelector.ex;
groupSelector.top = pointer.y - groupSelector.ey;
Expand Down
8 changes: 4 additions & 4 deletions src/mixins/canvas_grouping.mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,10 @@
continue;
}

if ((allowIntersect && currentObject.intersectsWithRect(selectionX1Y1, selectionX2Y2)) ||
currentObject.isContainedWithinRect(selectionX1Y1, selectionX2Y2) ||
(allowIntersect && currentObject.containsPoint(selectionX1Y1)) ||
(allowIntersect && currentObject.containsPoint(selectionX2Y2))
if ((allowIntersect && currentObject.intersectsWithRect(selectionX1Y1, selectionX2Y2, true)) ||
currentObject.isContainedWithinRect(selectionX1Y1, selectionX2Y2, true) ||
(allowIntersect && currentObject.containsPoint(selectionX1Y1, null, true)) ||
(allowIntersect && currentObject.containsPoint(selectionX2Y2, null, true))
) {
group.push(currentObject);
// only add one object if it's a click
Expand Down
16 changes: 13 additions & 3 deletions test/unit/canvas_events.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,10 @@
});

QUnit.test('mouse:down and group selector', function(assert) {
var e = { clientX: 30, clientY: 30, which: 1, target: canvas.upperCanvasEl };
var rect = new fabric.Rect({ width: 60, height: 60 });
var expectedGroupSelector = { ex: 30, ey: 30, top: 0, left: 0 };
var e = { clientX: 30, clientY: 40, which: 1, target: canvas.upperCanvasEl };
var rect = new fabric.Rect({ width: 150, height: 150 });
var expectedGroupSelector = { ex: 80, ey: 120, top: 0, left: 0 };
canvas.absolutePan(new fabric.Point(50, 80));
canvas.__onMouseDown(e);
assert.deepEqual(canvas._groupSelector, expectedGroupSelector, 'a new groupSelector is created');
canvas.add(rect);
Expand Down Expand Up @@ -654,6 +655,15 @@
// TODO: QUnit.test('mousemove: subTargetCheck: setCursorFromEvent considers subTargets')
// TODO: QUnit.test('mousemove: subTargetCheck: setCursorFromEvent considers subTargets in reverse order, so the top-most subTarget's .hoverCursor takes precedence')

QUnit.test('mouse move and group selector', function(assert){
var e = { clientX: 30, clientY: 40, which: 1, target: canvas.upperCanvasEl };
var expectedGroupSelector = { ex: 15, ey: 30, left: 65, top: 90};
canvas.absolutePan(new fabric.Point(50, 80));
canvas._groupSelector = {ex: 15, ey: 30, top: 0, left: 0};
canvas.__onMouseMove(e);
assert.deepEqual(canvas._groupSelector, expectedGroupSelector, 'groupSelector is updated');
});

['MouseDown', 'MouseMove', 'MouseOut', 'MouseEnter', 'MouseWheel', 'DoubleClick'].forEach(function(eventType) {
QUnit.test('avoid multiple registration - ' + eventType, function(assert) {
var funcName = '_on' + eventType;
Expand Down