Skip to content

Commit

Permalink
fix perPixelTargetFind (#3109)
Browse files Browse the repository at this point in the history
  • Loading branch information
@asturur
asturur authored Jul 16, 2016
1 parent 6e1d2e2 commit 51995a7
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 49 deletions.
11 changes: 5 additions & 6 deletions dist/fabric.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8676,16 +8676,12 @@ fabric.PatternBrush = fabric.util.createClass(fabric.PencilBrush, /** @lends fab
isTargetTransparent: function (target, x, y) {
var hasBorders = target.hasBorders,
transparentCorners = target.transparentCorners,
ctx = this.contextCache,
shouldTransform = target.group && target.group === this.getActiveGroup();
ctx = this.contextCache;

target.hasBorders = target.transparentCorners = false;

ctx.save();
ctx.transform.apply(ctx, this.viewportTransform);
if (shouldTransform) {
ctx.transform.apply(ctx, target.group.calcTransformMatrix());
}
target.render(ctx);
ctx.restore();

Expand Down Expand Up @@ -25292,7 +25288,8 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot
nodeCanvasOptions = nodeCanvasOptions || options;

var canvasEl = fabric.document.createElement('canvas'),
nodeCanvas = new Canvas(width || 600, height || 600, nodeCanvasOptions);
nodeCanvas = new Canvas(width || 600, height || 600, nodeCanvasOptions),
nodeCacheCanvas = new Canvas(width || 600, height || 600, nodeCanvasOptions);

// jsdom doesn't create style on canvas element, so here be temp. workaround
canvasEl.style = { };
Expand All @@ -25305,6 +25302,8 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot

fabricCanvas.contextContainer = nodeCanvas.getContext('2d');
fabricCanvas.nodeCanvas = nodeCanvas;
fabricCanvas.contextCache = nodeCacheCanvas.getContext('2d');
fabricCanvas.nodeCacheCanvas = nodeCacheCanvas;
fabricCanvas.Font = Canvas.Font;

return fabricCanvas;
Expand Down
12 changes: 6 additions & 6 deletions dist/fabric.min.js
View file

Large diffs are not rendered by default.

Binary file modified dist/fabric.min.js.gz
View file
Binary file not shown.
9 changes: 4 additions & 5 deletions dist/fabric.require.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4287,13 +4287,10 @@ fabric.PatternBrush = fabric.util.createClass(fabric.PencilBrush, {
};
},
isTargetTransparent: function(target, x, y) {
var hasBorders = target.hasBorders, transparentCorners = target.transparentCorners, ctx = this.contextCache, shouldTransform = target.group && target.group === this.getActiveGroup();
var hasBorders = target.hasBorders, transparentCorners = target.transparentCorners, ctx = this.contextCache;
target.hasBorders = target.transparentCorners = false;
ctx.save();
ctx.transform.apply(ctx, this.viewportTransform);
if (shouldTransform) {
ctx.transform.apply(ctx, target.group.calcTransformMatrix());
}
target.render(ctx);
ctx.restore();
target.active && target._renderControls(ctx);
Expand Down Expand Up @@ -12116,13 +12113,15 @@ fabric.util.object.extend(fabric.IText.prototype, {
};
fabric.createCanvasForNode = function(width, height, options, nodeCanvasOptions) {
nodeCanvasOptions = nodeCanvasOptions || options;
var canvasEl = fabric.document.createElement("canvas"), nodeCanvas = new Canvas(width || 600, height || 600, nodeCanvasOptions);
var canvasEl = fabric.document.createElement("canvas"), nodeCanvas = new Canvas(width || 600, height || 600, nodeCanvasOptions), nodeCacheCanvas = new Canvas(width || 600, height || 600, nodeCanvasOptions);
canvasEl.style = {};
canvasEl.width = nodeCanvas.width;
canvasEl.height = nodeCanvas.height;
var FabricCanvas = fabric.Canvas || fabric.StaticCanvas, fabricCanvas = new FabricCanvas(canvasEl, options);
fabricCanvas.contextContainer = nodeCanvas.getContext("2d");
fabricCanvas.nodeCanvas = nodeCanvas;
fabricCanvas.contextCache = nodeCacheCanvas.getContext("2d");
fabricCanvas.nodeCacheCanvas = nodeCacheCanvas;
fabricCanvas.Font = Canvas.Font;
return fabricCanvas;
};
Expand Down
6 changes: 1 addition & 5 deletions src/canvas.class.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,16 +350,12 @@
isTargetTransparent: function (target, x, y) {
var hasBorders = target.hasBorders,
transparentCorners = target.transparentCorners,
ctx = this.contextCache,
shouldTransform = target.group && target.group === this.getActiveGroup();
ctx = this.contextCache;

target.hasBorders = target.transparentCorners = false;

ctx.save();
ctx.transform.apply(ctx, this.viewportTransform);
if (shouldTransform) {
ctx.transform.apply(ctx, target.group.calcTransformMatrix());
}
target.render(ctx);
ctx.restore();

Expand Down
5 changes: 4 additions & 1 deletion src/node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@
nodeCanvasOptions = nodeCanvasOptions || options;

var canvasEl = fabric.document.createElement('canvas'),
nodeCanvas = new Canvas(width || 600, height || 600, nodeCanvasOptions);
nodeCanvas = new Canvas(width || 600, height || 600, nodeCanvasOptions),
nodeCacheCanvas = new Canvas(width || 600, height || 600, nodeCanvasOptions);

// jsdom doesn't create style on canvas element, so here be temp. workaround
canvasEl.style = { };
Expand All @@ -166,6 +167,8 @@

fabricCanvas.contextContainer = nodeCanvas.getContext('2d');
fabricCanvas.nodeCanvas = nodeCanvas;
fabricCanvas.contextCache = nodeCacheCanvas.getContext('2d');
fabricCanvas.nodeCacheCanvas = nodeCacheCanvas;
fabricCanvas.Font = Canvas.Font;

return fabricCanvas;
Expand Down
88 changes: 62 additions & 26 deletions test/unit/canvas.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
}

function makeTriangle(options) {
var defaultOptions = { width: 10, height: 10 };
var defaultOptions = { width: 30, height: 30 };
return new fabric.Triangle(fabric.util.object.extend(defaultOptions, options || { }));
}

Expand Down Expand Up @@ -225,11 +225,11 @@
canvas.add(rect);
target = canvas.findTarget({
clientX: 5, clientY: 5
}, true);
});
equal(target, rect, 'Should return the rect');
target = canvas.findTarget({
clientX: 30, clientY: 30
}, true);
});
equal(target, null, 'Should not find target');
canvas.remove(rect);
});
Expand All @@ -240,21 +240,21 @@
canvas.add(rect);
target = canvas.findTarget({
clientX: 5, clientY: 5
}, true);
});
canvas.setActiveObject(target);
equal(target, rect, 'Should return the rect');
canvas.renderAll();
equal(canvas.lastRenderedWithControls, rect);
canvas.remove(rect);
target = canvas.findTarget({
clientX: 5, clientY: 5
}, true);
});
equal(target, null, 'Should not find target');
equal(canvas.lastRenderedWithControls, undefined, 'lastRendereWithControls reference should disappear');
});

test('findTarget with subTargetCheck', function() {
var rect = makeRect({ left: 0, top: 0 }),
var rect = makeRect({ left: 0, top: 0 }),
rect2 = makeRect({ left: 30, top: 30}), target,
group = new fabric.Group([rect, rect2]);

Expand All @@ -266,22 +266,22 @@
equal(canvas.targets[0], undefined, 'no subtarget should return');
target = canvas.findTarget({
clientX: 30, clientY: 30
}, true);
});
equal(target, group, 'Should return the group');
group.subTargetCheck = true;
target = canvas.findTarget({
clientX: 5, clientY: 5
}, true);
});
equal(target, group, 'Should return the group');
equal(canvas.targets[0], rect, 'should return the rect');
target = canvas.findTarget({
clientX: 15, clientY: 15
}, true);
});
equal(target, group, 'Should return the group');
equal(canvas.targets[0], undefined, 'no subtarget should return');
target = canvas.findTarget({
clientX: 32, clientY: 32
}, true);
});
equal(target, group, 'Should return the group');
equal(canvas.targets[0], rect2, 'should return the rect2');
canvas.remove(group);
Expand All @@ -292,36 +292,72 @@
var triangle = makeTriangle({ left: 0, top: 0 }), target;
canvas.add(triangle);
target = canvas.findTarget({
clientX: 2, clientY: 1
}, true);
clientX: 5, clientY: 5
});
equal(target, triangle, 'Should return the triangle by bounding box');
//TODO find out why this stops the tests
//canvas.perPixelTargetFind = true;
//target = canvas.findTarget({
// clientX: 2, clientY: 1
//}, true);
//equal(target, null, 'Should return null because of transparency checks');
canvas.perPixelTargetFind = true;
target = canvas.findTarget({
clientX: 5, clientY: 5
}, true);
});
equal(target, null, 'Should return null because of transparency checks');
target = canvas.findTarget({
clientX: 15, clientY: 15
});
equal(target, triangle, 'Should return the triangle now');
canvas.perPixelTargetFind = false;
canvas.remove(triangle);
});

test('findTarget on activegroup', function() {
var rect1 = makeRect({ left: 0, top: 0 }), target;
var rect2 = makeRect({ left: 20, top: 0 });
var rect2 = makeRect({ left: 20, top: 20 });
var rect3 = makeRect({ left: 20, top: 0 });
canvas.add(rect1);
canvas.add(rect2);
canvas.add(rect3);
var group = new fabric.Group([ rect1, rect2 ]);
canvas.add(group);
canvas.setActiveGroup(group);
target = canvas.findTarget({
clientX: 5, clientY: 5
});
equal(target, group, 'Should return the activegroup');
target = canvas.findTarget({
clientX: 40, clientY: 15
});
equal(target, null, 'Should miss the activegroup');
target = canvas.findTarget({
clientX: 5, clientY: 5
}, true);
equal(target, rect1, 'Should return the rect inside activegroup');
target = canvas.findTarget({
clientX: 25, clientY: 5
});
equal(target, group, 'Should return the activegroup');
target = canvas.findTarget({
clientX: 25, clientY: 5
}, true);
equal(target, rect3, 'Should return the rect behind activegroup');
});

test('findTarget on activegroup with perPixelTargetFind', function() {
var rect1 = makeRect({ left: 0, top: 0 }), target;
var rect2 = makeRect({ left: 20, top: 20 });
canvas.perPixelTargetFind = true;
canvas.add(rect1);
canvas.add(rect2);
var group = new fabric.Group([ rect1, rect2 ]);
canvas.setActiveGroup(group);
target = canvas.findTarget({
clientX: 10, clientY: 10
});
equal(target, group, 'Should return the activegroup');
//TODO: make it work with perPixelTargetFind

target = canvas.findTarget({
clientX: 15, clientY: 15
});
equal(target, null, 'Should miss the activegroup');
canvas.perPixelTargetFind = false;
});

test('activeGroup sendToBack', function() {
Expand All @@ -343,7 +379,7 @@
equal(canvas._objects[2], rect1, 'rect1 should be the third object');
equal(canvas._objects[3], rect2, 'rect2 should be on top now');
});

test('activeGroup bringToFront', function() {

var rect1 = makeRect(),
Expand Down Expand Up @@ -656,7 +692,7 @@
ok(fired, 'Callback should be fired even if no objects');
equal(c2.backgroundColor, 'green', 'Color should be set properly');
equal(c2.overlayColor, 'yellow', 'Color should be set properly');

});
});

Expand All @@ -675,7 +711,7 @@
ok(fired, 'Callback should be fired even if no "objects" property exists');
equal(c2.backgroundColor, 'green', 'Color should be set properly');
equal(c2.overlayColor, 'yellow', 'Color should be set properly');

});
});

Expand Down Expand Up @@ -1057,7 +1093,7 @@
equal(aGroup._objects[2], circle1);
equal(aGroup._objects[3], circle2);
});

test('dispose', function() {
//made local vars to do not dispose the external canvas
var el = fabric.document.createElement('canvas'),
Expand Down Expand Up @@ -1100,7 +1136,7 @@
equal(canvas.wrapperEl, null, 'wrapperEl should be deleted');
equal(canvas.upperCanvasEl, null, 'upperCanvas should be deleted');
});

// test('dispose', function() {
// function invokeEventsOnCanvas() {
// // nextSibling because we need to invoke events on upper canvas
Expand Down

0 comments on commit 51995a7

Please sign in to comment.