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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(fabric.Canvas) Remove controls check in the pixel accuracy target find #6798

Merged
merged 14 commits into from
Jan 30, 2021
2 changes: 1 addition & 1 deletion .github/workflows/browser-chrome.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:
branches: [ master ]

jobs:
build:
chrome-tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/browser-firefox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:
branches: [ master ]

jobs:
build:
firefox-tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Expand All @@ -23,4 +23,3 @@ jobs:
uses: GabrielBB/xvfb-action@v1
with:
run: npm run testem:ci -- --port 8080 -f testem.json -l Firefox

30 changes: 27 additions & 3 deletions .github/workflows/build.js.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This workflow will install Uglify and verify that fabric can be build with it

name: Building with uglify
name: Pr quality check

on:
push:
Expand All @@ -10,13 +10,37 @@ on:

jobs:
build:

runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Uglyfied build
uses: actions/setup-node@v1
with:
node-version: 14.x
node-version: 12.x
- run: npm install uglify-js@3.3.x
- run: npm run build
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Linting
uses: actions/setup-node@v1
with:
node-version: 12.x
- run: npm install eslint@4.7.x
- run: npm run lint
coverage:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Coverage report
uses: actions/setup-node@v1
with:
node-version: 12.x
- run: npm ci
- run: npm run build:fast
- run: npm run test:coverage && npm run test:visual:coverage
- uses: ChristiaanScheermeijer/jest-reporter-action@v0.4.0
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
test-command: "npm run coverage:report"
20 changes: 0 additions & 20 deletions .github/workflows/linting.yml

This file was deleted.

4 changes: 2 additions & 2 deletions .github/workflows/node-unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ on:
branches: [ master ]

jobs:
build:
node-unit-tests:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
node-version: [12.x, 14.x, 15.x]
node-version: [14.x, 15.x]
# See supported Node.js release schedule at https://nodejs.org/en/about/releases/
steps:
- uses: actions/checkout@v2
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/visual-node.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ on:
branches: [ master ]

jobs:
build:
node-visual-tests:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
node-version: [12.x, 14.x, 15.x]
node-version: [14.x, 15.x]
# See supported Node.js release schedule at https://nodejs.org/en/about/releases/
steps:
- uses: actions/checkout@v2
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/visual-test-chrome.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:
branches: [ master ]

jobs:
build:
chrome-visual-tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/visual-test-firefox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:
branches: [ master ]

jobs:
build:
firefox-visual-tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Expand Down
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@
"build_with_gestures": "node build.js modules=ALL exclude=accessors",
"build_export": "npm run build:fast && npm run export_dist_to_site",
"test:single": "qunit test/node_test_setup.js test/lib",
"test": "nyc qunit test/node_test_setup.js test/lib test/unit",
"test:coverage": "nyc --silent qunit test/node_test_setup.js test/lib test/unit",
"test:visual:coverage": "nyc --silent --no-clean qunit test/node_test_setup.js test/lib test/visual",
"coverage:report": "nyc report --reporter=lcov --reporter=text",
"test": "qunit test/node_test_setup.js test/lib test/unit",
"test:visual": "qunit test/node_test_setup.js test/lib test/visual",
"test:visual:single": "qunit test/node_test_setup.js test/lib",
"test:all": "npm run test && npm run test:visual",
Expand All @@ -58,7 +61,6 @@
"export_tests_to_site": "cp test/unit/*.js ../fabricjs.com/test/unit && cp -r test/visual/* ../fabricjs.com/test/visual && cp -r test/fixtures/* ../fabricjs.com/test/fixtures && cp -r test/lib/* ../fabricjs.com/test/lib",
"all": "npm run build && npm run test && npm run test:visual && npm run lint && npm run lint_tests && npm run export_dist_to_site && npm run export_tests_to_site",
"testem": "testem .",
"testem:visual": "testem --file testem-visual.json",
"testem:ci": "testem ci"
},
"optionalDependencies": {
Expand Down
18 changes: 6 additions & 12 deletions src/canvas.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -497,13 +497,6 @@
target.render(ctx);
ctx.restore();

target === this._activeObject && target._renderControls(ctx, {
hasBorders: false,
transparentCorners: false
}, {
hasBorders: false,
});

target.selectionBackgroundColor = originalColor;

var isTransparent = fabric.util.isTransparent(
Expand Down Expand Up @@ -765,18 +758,19 @@
activeObject = this._activeObject,
aObjects = this.getActiveObjects(),
activeTarget, activeTargetSubs,
isTouch = isTouchEvent(e);
isTouch = isTouchEvent(e),
shouldLookForActive = (aObjects.length > 1 && !skipGroup) || aObjects.length === 1;

// first check current group (if one exists)
// active group does not check sub targets like normal groups.
// if active group just exits.
this.targets = [];

if (aObjects.length > 1 && !skipGroup && activeObject === this._searchPossibleTargets([activeObject], pointer)) {
// if we hit the corner of an activeObject, let's return that.
if (shouldLookForActive && activeObject._findTargetCorner(pointer, isTouch)) {
return activeObject;
}
// if we hit the corner of an activeObject, let's return that.
if (aObjects.length === 1 && activeObject._findTargetCorner(pointer, isTouch)) {
if (aObjects.length > 1 && !skipGroup && activeObject === this._searchPossibleTargets([activeObject], pointer)) {
return activeObject;
}
if (aObjects.length === 1 &&
Expand Down Expand Up @@ -812,7 +806,7 @@
obj.evented &&
// http://www.geog.ubc.ca/courses/klink/gis.notes/ncgia/u32.html
// http://idav.ucdavis.edu/~okreylos/TAship/Spring2000/PointInPolygon.html
(obj.containsPoint(pointer) || !!obj._findTargetCorner(pointer))
obj.containsPoint(pointer)
) {
if ((this.perPixelTargetFind || obj.perPixelTargetFind) && !obj.isEditing) {
var isTransparent = this.isTargetTransparent(obj, globalPointer.x, globalPointer.y);
Expand Down
26 changes: 13 additions & 13 deletions test/unit/canvas.js
Original file line number Diff line number Diff line change
Expand Up @@ -2452,9 +2452,9 @@
assert.equal(canvas.isTargetTransparent(rect, 1, 1), false, 'opaque on 1,1');
assert.equal(canvas.isTargetTransparent(rect, 2, 2), false, 'opaque on 2,2');
assert.equal(canvas.isTargetTransparent(rect, 3, 3), false, 'opaque on 3,3');
assert.equal(canvas.isTargetTransparent(rect, 4, 4), false, 'opaque on 4,4');
assert.equal(canvas.isTargetTransparent(rect, 5, 5), false, 'opaque on 5, 5');
assert.equal(canvas.isTargetTransparent(rect, 6, 6), false, 'opaque on 6, 6');
assert.equal(canvas.isTargetTransparent(rect, 4, 4), true, 'transparent on 4,4');
assert.equal(canvas.isTargetTransparent(rect, 5, 5), true, 'transparent on 5, 5');
assert.equal(canvas.isTargetTransparent(rect, 6, 6), true, 'transparent on 6, 6');
assert.equal(canvas.isTargetTransparent(rect, 7, 7), true, 'transparent on 7, 7');
assert.equal(canvas.isTargetTransparent(rect, 8, 8), true, 'transparent on 8, 8');
assert.equal(canvas.isTargetTransparent(rect, 9, 9), true, 'transparent on 9, 9');
Expand All @@ -2465,20 +2465,20 @@
assert.equal(canvas.isTargetTransparent(rect, 14, 14), true, 'transparent 14, 14');
assert.equal(canvas.isTargetTransparent(rect, 15, 15), true, 'transparent 15, 15');
assert.equal(canvas.isTargetTransparent(rect, 16, 16), true, 'transparent 16, 16');
assert.equal(canvas.isTargetTransparent(rect, 17, 17), false, 'opaque 17, 17');
assert.equal(canvas.isTargetTransparent(rect, 18, 18), false, 'opaque 18, 18');
assert.equal(canvas.isTargetTransparent(rect, 19, 19), false, 'opaque 19, 19');
assert.equal(canvas.isTargetTransparent(rect, 17, 17), true, 'transparent 17, 17');
assert.equal(canvas.isTargetTransparent(rect, 18, 18), true, 'transparent 18, 18');
assert.equal(canvas.isTargetTransparent(rect, 19, 19), true, 'transparent 19, 19');
assert.equal(canvas.isTargetTransparent(rect, 20, 20), false, 'opaque 20, 20');
assert.equal(canvas.isTargetTransparent(rect, 21, 21), false, 'opaque 21, 21');
assert.equal(canvas.isTargetTransparent(rect, 22, 22), false, 'opaque 22, 22');
assert.equal(canvas.isTargetTransparent(rect, 23, 23), false, 'opaque 23, 23');
assert.equal(canvas.isTargetTransparent(rect, 24, 24), false, 'opaque 24, 24');
assert.equal(canvas.isTargetTransparent(rect, 25, 25), false, 'opaque 25, 25');
assert.equal(canvas.isTargetTransparent(rect, 26, 26), false, 'opaque 26, 26');
assert.equal(canvas.isTargetTransparent(rect, 27, 27), false, 'opaque 27, 27');
assert.equal(canvas.isTargetTransparent(rect, 28, 28), false, 'opaque 28, 28');
assert.equal(canvas.isTargetTransparent(rect, 29, 29), false, 'opaque 29, 29');
assert.equal(canvas.isTargetTransparent(rect, 30, 30), false, 'opaque 30, 30');
assert.equal(canvas.isTargetTransparent(rect, 24, 24), true, 'transparent 24, 24');
assert.equal(canvas.isTargetTransparent(rect, 25, 25), true, 'transparent 25, 25');
assert.equal(canvas.isTargetTransparent(rect, 26, 26), true, 'transparent 26, 26');
assert.equal(canvas.isTargetTransparent(rect, 27, 27), true, 'transparent 27, 27');
assert.equal(canvas.isTargetTransparent(rect, 28, 28), true, 'transparent 28, 28');
assert.equal(canvas.isTargetTransparent(rect, 29, 29), true, 'transparent 29, 29');
assert.equal(canvas.isTargetTransparent(rect, 30, 30), true, 'transparent 30, 30');
assert.equal(canvas.isTargetTransparent(rect, 31, 31), true, 'transparent 31, 31');

});
Expand Down