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

refactor() change common ancenstors returned data to help isInFrontOf #7906

Merged
merged 7 commits into from
Apr 28, 2022
Merged

refactor() change common ancenstors returned data to help isInFrontOf #7906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fc1812f
i like more in this way
asturur Apr 18, 2022
ae2fd5d
just saved my work
asturur Apr 19, 2022
9034ee6
rework findCommonAncestors
asturur Apr 26, 2022
bc60e8e
no builds
asturur Apr 26, 2022
f087683
fixed isInFrontOf
asturur Apr 26, 2022
f40e5c3
that strict made no sense
asturur Apr 26, 2022
2319d4f
refactor(): return completely different forks
ShaMan123 Apr 28, 2022
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
61 changes: 49 additions & 12 deletions src/mixins/object_ancestry.mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
/**
*
* @typedef {fabric.Object[] | [...fabric.Object[], fabric.StaticCanvas]} Ancestors
*
*
* @param {boolean} [strict] returns only ancestors that are objects (without canvas)
* @returns {Ancestors} ancestors from bottom to top
*/
Expand All @@ -39,38 +39,74 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
},

/**
*
* Returns an object that represent the ancestry situation.
*
* @typedef {object} AncestryComparison
* @property {Ancestors} common ancestors of `this` and `other` (may include `this` | `other`)
* @property {Ancestors} fork ancestors that are of `this` only
* @property {Ancestors} otherFork ancestors that are of `other` only
*
* @param {fabric.Object} other
* @param {boolean} [strict] finds only ancestors that are objects (without canvas)
* @returns {{ index: number, otherIndex: number, ancestors: Ancestors } | undefined} ancestors may include the passed objects if one is an ancestor of the other resulting in index of -1
* @returns {AncestryComparison | undefined}
*
*/
findCommonAncestors: function (other, strict) {
if (this === other) {
return {
index: 0,
otherIndex: 0,
ancestors: this.getAncestors(strict)
fork: [],
otherFork: [],
common: [this].concat(this.getAncestors(strict))
};
}
else if (!other) {
// meh, warn and inform, and not my issue.
// the argument is NOT optional, we can't end up here.
return undefined;
}
var ancestors = this.getAncestors(strict);
ancestors.unshift(this);
var otherAncestors = other.getAncestors(strict);
otherAncestors.unshift(other);
// if `this` has no ancestors and `this` is top ancestor of `other` we must handle the following case
Copy link
Contributor

@ShaMan123 ShaMan123 Apr 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

maybe this condition should come after the for loop for readability

if (ancestors.length === 0 && otherAncestors.length > 0 && this === otherAncestors[otherAncestors.length - 1]) {
return {
fork: [],
otherFork: [other].concat(otherAncestors.slice(0, otherAncestors.length - 1)),
common: [this]
};
}
// compare ancestors
for (var i = 0, ancestor; i < ancestors.length; i++) {
ancestor = ancestors[i];
if (ancestor === other) {
return {
fork: [this].concat(ancestors.slice(0, i)),
otherFork: [],
common: ancestors.slice(i)
};
}
for (var j = 0; j < otherAncestors.length; j++) {
if (this === otherAncestors[j]) {
return {
fork: [],
otherFork: [other].concat(otherAncestors.slice(0, j)),
common: [this].concat(ancestors)
};
}
if (ancestor === otherAncestors[j]) {
return {
index: i - 1,
otherIndex: j - 1,
ancestors: ancestors.slice(i)
fork: [this].concat(ancestors.slice(0, i)),
otherFork: [other].concat(otherAncestors.slice(0, j)),
common: ancestors.slice(i)
};
}
}
}
// nothing shared
return {
fork: [this].concat(ancestors),
otherFork: [other].concat(otherAncestors),
common: []
};
},

/**
Expand All @@ -80,6 +116,7 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
* @returns {boolean}
*/
hasCommonAncestors: function (other, strict) {
return !!this.findCommonAncestors(other, strict);
var commonAncestors = this.findCommonAncestors(other, strict);
return commonAncestors && !!commonAncestors.ancestors.length;
}
});
45 changes: 16 additions & 29 deletions src/mixins/object_stacking.mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,37 +87,24 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
if (this === other) {
return undefined;
}
var ancestors = this.getAncestors().reverse().concat(this);
var otherAncestors = other.getAncestors().reverse().concat(other);
var i, j, found = false;
// find the common ancestor
for (i = 0; i < ancestors.length; i++) {
for (j = 0; j < otherAncestors.length; j++) {
if (ancestors[i] === otherAncestors[j]) {
found = true;
break;
}
}
if (found) {
break;
}
}
if (!found) {
var ancestorData = this.findCommonAncestors(other);
if (!ancestorData) {
return undefined;
}
// compare trees from the common ancestor down
var tree = ancestors.slice(i),
otherTree = otherAncestors.slice(j),
a, b, parent;
for (i = 1; i < Math.min(tree.length, otherTree.length); i++) {
a = tree[i];
b = otherTree[i];
if (a !== b) {
parent = tree[i - 1];
return parent._objects.indexOf(a) > parent._objects.indexOf(b);
}
if (ancestorData.fork.includes(other)) {
return true;
}
if (ancestorData.otherFork.includes(this)) {
return false;
}
var firstCommonAncestor = ancestorData.common[0];
if (!firstCommonAncestor) {
return undefined;
}
// happens if a is ancestor of b or vice versa
return tree.length > otherTree.length;
var headOfFork = ancestorData.fork.pop(),
headOfOtherFork = ancestorData.otherFork.pop(),
thisIndex = firstCommonAncestor._objects.indexOf(headOfFork),
otherIndex = firstCommonAncestor._objects.indexOf(headOfOtherFork);
return thisIndex > -1 && thisIndex > otherIndex;
}
});
131 changes: 74 additions & 57 deletions test/unit/object.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@
});

function prepareObjectsForTreeTesting() {
var Object = fabric.util.createClass(fabric.Object, {
var TObject = fabric.util.createClass(fabric.Object, {
toJSON: function () {
return {
id: this.id,
Expand All @@ -861,8 +861,9 @@
return JSON.stringify(this.toJSON(), null, '\t');
}
});
var Collection = fabric.util.createClass(Object, fabric.Collection, {
initialize: function () {
var Collection = fabric.util.createClass(TObject, fabric.Collection, {
initialize: function ({ id }) {
this.id = id;
this._objects = [];
},
add: function () {
Expand All @@ -882,7 +883,7 @@
},
removeAll: function () {
this.remove.apply(this, this._objects);
}
},
});
var canvas = fabric.util.object.extend(new Collection({ id: 'canvas' }), {
_onObjectAdded: function (object) {
Expand All @@ -893,8 +894,8 @@
},
});
return {
object: new Object({ id: 'object' }),
other: new Object({ id: 'other' }),
object: new TObject({ id: 'object' }),
other: new TObject({ id: 'other' }),
a: new Collection({ id: 'a' }),
b: new Collection({ id: 'b' }),
c: new Collection({ id: 'c' }),
Expand All @@ -904,91 +905,107 @@

QUnit.test('findCommonAncestors', function (assert) {
function findCommonAncestors(object, other, strict, expected, message) {
var common = object.findCommonAncestors(other, strict);
assert.deepEqual(
common.fork.map((obj) => obj.id),
expected.fork.map((obj) => obj.id),
message || `fork property should match check between '${object.id}' and '${other.id}'`
);
assert.deepEqual(
common.otherFork.map((obj) => obj.id),
expected.otherFork.map((obj) => obj.id),
message || `otherFork property should match check between '${object.id}' and '${other.id}'`
);
assert.deepEqual(
object.findCommonAncestors(other, strict),
expected,
message || `should match check between '${object.id}' and '${other.id}'`
common.common.map((obj) => obj.id),
expected.common.map((obj) => obj.id),
message || `common property should match check between '${object.id}' and '${other.id}'`
);
var oppositeCommon = other.findCommonAncestors(object, strict);
assert.deepEqual(
other.findCommonAncestors(object, strict),
typeof expected === 'object' ?
{
index: expected.otherIndex,
otherIndex: expected.index,
ancestors: expected.ancestors
} :
expected,
`should match opposite check between '${object.id}' and '${other.id}'`
oppositeCommon.fork.map((obj) => obj.id),
expected.otherFork.map((obj) => obj.id),
message || `fork property should match opposite check between '${other.id}' and '${object.id}'`
);
assert.deepEqual(
oppositeCommon.otherFork.map((obj) => obj.id),
expected.fork.map((obj) => obj.id),
message || `otherFork property should match opposite check between '${other.id}' and '${object.id}'`
);
assert.deepEqual(
oppositeCommon.common.map((obj) => obj.id),
expected.common.map((obj) => obj.id),
message || `common property should match opposite check between '${other.id}' and '${object.id}'`
);
}
var { object, other, a, b, c, canvas } = prepareObjectsForTreeTesting();
assert.ok(typeof object.findCommonAncestors === 'function');
assert.ok(Array.isArray(a._objects));
assert.ok(a._objects !== b._objects);
// same object
findCommonAncestors(object, object, false, { index: 0, otherIndex: 0, ancestors: [] });
findCommonAncestors(object, object, false, { fork: [], otherFork: [] , common: [object] });
// foreign objects
findCommonAncestors(object, other, false, undefined);
findCommonAncestors(object, other, false, { fork: [object], otherFork: [other] , common: [] });
// same level
a.add(object, other);
findCommonAncestors(object, other, false, { index: 0, otherIndex: 0, ancestors: [a] });
findCommonAncestors(object, a, false, { index: 0, otherIndex: -1, ancestors: [a] });
findCommonAncestors(other, a, false, { index: 0, otherIndex: -1, ancestors: [a] });
findCommonAncestors(a, object, false, { index: -1, otherIndex: 0, ancestors: [a] });
findCommonAncestors(a, object, true, { index: -1, otherIndex: 0, ancestors: [a] }, 'strict option should have no effect');
findCommonAncestors(object, other, false, { fork: [object], otherFork: [other], common: [a] });
findCommonAncestors(object, a, false, { fork: [object], otherFork: [], common: [a] });
findCommonAncestors(other, a, false, { fork: [other], otherFork: [], common: [a] });
findCommonAncestors(a, object, false, { fork: [], otherFork: [object], common: [a] });
findCommonAncestors(a, object, true, { fork: [], otherFork: [object], common: [a] }, 'strict option should have no effect when outside canvas');
// different level
a.remove(object);
b.add(object);
a.add(b);
findCommonAncestors(object, b, false, { index: 0, otherIndex: -1, ancestors: [b, a] });
findCommonAncestors(b, a, false, { index: 0, otherIndex: -1, ancestors: [a] });
findCommonAncestors(object, other, false, { index: 1, otherIndex: 0, ancestors: [a] });
// with common ancestor
findCommonAncestors(object, b, false, { fork: [object], otherFork: [], common: [b, a] });
findCommonAncestors(b, a, false, { fork: [b], otherFork: [], common: [a] });
findCommonAncestors(object, other, false, { fork: [object, b], otherFork: [other], common: [a] });
// with common ancestor
assert.equal(c.size(), 0, 'c should be empty');
c.add(a);
assert.equal(c.size(), 1, 'c should contain a');
findCommonAncestors(object, b, false, { index: 0, otherIndex: -1, ancestors: [b, a, c] });
findCommonAncestors(b, a, false, { index: 0, otherIndex: -1, ancestors: [a, c] });
findCommonAncestors(object, other, false, { index: 1, otherIndex: 0, ancestors: [a, c] });
findCommonAncestors(object, c, false, { index: 2, otherIndex: -1, ancestors: [c] });
findCommonAncestors(other, c, false, { index: 1, otherIndex: -1, ancestors: [c] });
findCommonAncestors(b, c, false, { index: 1, otherIndex: -1, ancestors: [c] });
findCommonAncestors(a, c, false, { index: 0, otherIndex: -1, ancestors: [c] });
findCommonAncestors(object, b, false, { fork: [object], otherFork: [], common: [b, a, c] });
findCommonAncestors(b, a, false, { fork: [b], otherFork: [], common: [a, c] });
findCommonAncestors(object, other, false, { fork: [object, b], otherFork: [other], common: [a, c] });
findCommonAncestors(object, c, false, { fork: [object, b, a], otherFork: [], common: [c] });
findCommonAncestors(other, c, false, { fork: [other, a], otherFork: [], common: [c] });
findCommonAncestors(b, c, false, { fork: [b, a], otherFork: [], common: [c] });
findCommonAncestors(a, c, false, { fork: [a], otherFork: [], common: [c] });
// deeper asymmetrical
c.removeAll();
assert.equal(c.size(), 0, 'c should be cleared');
a.remove(other);
c.add(other, a);
findCommonAncestors(object, b, false, { index: 0, otherIndex: -1, ancestors: [b, a, c] });
findCommonAncestors(b, a, false, { index: 0, otherIndex: -1, ancestors: [a, c] });
findCommonAncestors(a, other, false, { index: 0, otherIndex: 0, ancestors: [c] });
findCommonAncestors(object, other, false, { index: 2, otherIndex: 0, ancestors: [c] });
findCommonAncestors(object, c, false, { index: 2, otherIndex: -1, ancestors: [c] });
findCommonAncestors(other, c, false, { index: 0, otherIndex: -1, ancestors: [c] });
findCommonAncestors(b, c, false, { index: 1, otherIndex: -1, ancestors: [c] });
findCommonAncestors(a, c, false, { index: 0, otherIndex: -1, ancestors: [c] });
findCommonAncestors(object, b, false, { fork: [object], otherFork: [], common: [b, a, c] });
findCommonAncestors(b, a, false, { fork: [b], otherFork: [], common: [a, c] });
findCommonAncestors(a, other, false, { fork: [a], otherFork: [other], common: [c] });
findCommonAncestors(object, other, false, { fork: [object, b, a], otherFork: [other], common: [c] });
findCommonAncestors(object, c, false, { fork: [object, b, a], otherFork: [], common: [c] });
findCommonAncestors(other, c, false, { fork: [other], otherFork: [], common: [c] });
findCommonAncestors(b, c, false, { fork: [b, a], otherFork: [], common: [c] });
findCommonAncestors(a, c, false, { fork: [a], otherFork: [], common: [c] });
// with canvas
a.removeAll();
b.removeAll();
c.removeAll();
canvas.add(object, other);
findCommonAncestors(object, other, true, undefined);
findCommonAncestors(object, other, false, { index: 0, otherIndex: 0, ancestors: [canvas] });
findCommonAncestors(object, canvas, true, undefined);
findCommonAncestors(object, canvas, false, { index: 0, otherIndex: -1, ancestors: [canvas] });
findCommonAncestors(other, canvas, false, { index: 0, otherIndex: -1, ancestors: [canvas] });
findCommonAncestors(object, other, true, { fork: [object], otherFork: [other], common: [] });
findCommonAncestors(object, other, false, { fork: [object], otherFork: [other], common: [canvas] });
findCommonAncestors(object, canvas, true, { fork: [object], otherFork: [canvas], common: [] });
findCommonAncestors(object, canvas, false, { fork: [object], otherFork: [], common: [canvas] });
findCommonAncestors(other, canvas, false, { fork: [other], otherFork: [], common: [canvas] });
// parent precedes canvas when checking ancestor
a.add(object);
assert.ok(object.canvas === canvas, 'object should have canvas set');
findCommonAncestors(object, other, true, undefined);
findCommonAncestors(object, other, false, undefined);
findCommonAncestors(object, other, true, { fork: [object, a], otherFork: [other], common: [] });
findCommonAncestors(object, other, false, { fork: [object, a], otherFork: [other, canvas], common: [] });
canvas.insertAt(a, 0);
findCommonAncestors(object, other, true, undefined);
findCommonAncestors(object, other, false, { index: 1, otherIndex: 0, ancestors: [canvas] });
findCommonAncestors(a, other, false, { index: 0, otherIndex: 0, ancestors: [canvas] });
findCommonAncestors(a, canvas, false, { index: 0, otherIndex: -1, ancestors: [canvas] });
findCommonAncestors(object, canvas, false, { index: 1, otherIndex: -1, ancestors: [canvas] });
findCommonAncestors(other, canvas, false, { index: 0, otherIndex: -1, ancestors: [canvas] });
findCommonAncestors(object, other, true, { fork: [object, a], otherFork: [other], common: [] });
findCommonAncestors(object, other, false, { fork: [object, a], otherFork: [other], common: [canvas] });
findCommonAncestors(a, other, false, { fork: [a], otherFork: [other], common: [canvas] });
findCommonAncestors(a, canvas, false, { fork: [a], otherFork: [], common: [canvas] });
findCommonAncestors(object, canvas, false, { fork: [object, a], otherFork: [], common: [canvas] });
findCommonAncestors(other, canvas, false, { fork: [other], otherFork: [], common: [canvas] });
});

QUnit.assert.isInFrontOf = function (object, other, expected) {
Expand Down