-
Notifications
You must be signed in to change notification settings - Fork 139
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
Path and multiple shapes ordering (solves #9) #39
base: master
Are you sure you want to change the base?
Changes from all commits
d4fe4c1
6c71697
7371c16
017910c
2a155af
a455b3f
19e14d0
be8b04d
b6c590e
092ec9f
9a8a06a
de7f2f8
43e3e05
3aa9d74
b6227a0
d24b7d3
4245b72
813d587
49d81f6
bc320b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,11 @@ function Isomer(canvasId, options) { | |
*/ | ||
this.colorDifference = 0.20; | ||
this.lightColor = options.lightColor || new Color(255, 255, 255); | ||
|
||
/** | ||
* List of {path, color, shapeName} to draw | ||
*/ | ||
this.scene = []; | ||
} | ||
|
||
/** | ||
|
@@ -67,25 +72,215 @@ Isomer.prototype._translatePoint = function (point) { | |
/** | ||
* Adds a shape or path to the scene | ||
* | ||
* This method also accepts arrays | ||
* This method also accepts arrays. | ||
* By default or if expertMode=0, shapes are drawn | ||
*/ | ||
Isomer.prototype.add = function (item, baseColor) { | ||
Isomer.prototype.add = function (item, baseColor, expertMode, name) { | ||
var expertModeValid = !!expertMode; | ||
if (Object.prototype.toString.call(item) == '[object Array]') { | ||
for (var i = 0; i < item.length; i++) { | ||
this.add(item[i], baseColor); | ||
this.add(item[i], baseColor, expertModeValid, name); | ||
} | ||
} else if (item instanceof Path) { | ||
this._addPath(item, baseColor); | ||
if(expertModeValid){ | ||
this.scene[this.scene.length] = {path:item, color:baseColor, shapeName:(name||'')}; | ||
} else { | ||
this._addPath(item, baseColor); | ||
} | ||
} else if (item instanceof Shape) { | ||
/* Fetch paths ordered by distance to prevent overlaps */ | ||
var paths = item.orderedPaths(); | ||
for (var i in paths) { | ||
this._addPath(paths[i], baseColor); | ||
if(expertModeValid){ | ||
this.scene[this.scene.length] = {path:paths[i], color:baseColor, shapeName:(name||'')}; | ||
} else { | ||
this._addPath(paths[i], baseColor); | ||
} | ||
} | ||
} | ||
}; | ||
|
||
/** | ||
* Draws the content of this.scene | ||
* By default, does not sort the paths between shapes | ||
*/ | ||
Isomer.prototype.draw = function(sortPath){ | ||
var sortValid = !!sortPath; | ||
if(sortValid){ | ||
this.sortPaths(); | ||
} | ||
for (var i in this.scene){ | ||
this._addPath(this.scene[i].path, this.scene[i].color); | ||
} | ||
} | ||
|
||
|
||
/** | ||
* Sorts the paths contained in this.scene, | ||
* ordered so that distant faces are displayed first | ||
* */ | ||
Isomer.prototype.sortPaths = function () { | ||
var Point = Isomer.Point; | ||
var observer = new Point(-10, -10, 20); | ||
var pathList = []; | ||
for (var i = 0; i < this.scene.length; i++) { | ||
var currentPath = this.scene[i]; | ||
pathList[i] = { | ||
path: currentPath.path, | ||
polygon: currentPath.path.points.map(this._translatePoint.bind(this)), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't create a new function. polygon: currentPath.path.points.map(this._translatePoint, this), There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I took it from already existing _addPath, is there a difference between the two use cases ? |
||
color: currentPath.color, | ||
drawn: 0, | ||
shapeName: currentPath.shapeName | ||
|
||
}; | ||
} | ||
this.scene.length = 0; | ||
|
||
// topological sort | ||
|
||
var drawBefore = []; | ||
for (var i = 0 ; i < pathList.length ; i++){ | ||
drawBefore[i] = []; | ||
} | ||
for (var i = 0 ; i < pathList.length ; i++){ | ||
for (var j = 0 ; j < i ; j++){ | ||
if(this._hasIntersection(pathList[i].polygon, pathList[j].polygon)){ | ||
var cmpPath = pathList[i].path.closerThan(pathList[j].path, observer); | ||
if(cmpPath < 0){ | ||
drawBefore[i][drawBefore[i].length] = j; | ||
} | ||
if(cmpPath > 0){ | ||
drawBefore[j][drawBefore[j].length] = i; | ||
} | ||
} | ||
} | ||
} | ||
var drawThisTurn = 1; | ||
var index = 0; | ||
while(drawThisTurn == 1){ | ||
index++; | ||
drawThisTurn = 0; | ||
for (var i = 0 ; i < pathList.length ; i++){ | ||
if(pathList[i].drawn == 0){ | ||
var canDraw = 1; | ||
for (var j = 0 ; j < drawBefore[i].length ; j++){ | ||
if(pathList[drawBefore[i][j]].drawn == 0){canDraw = 0;} | ||
} | ||
if(canDraw == 1){ | ||
this.add(pathList[i].path, pathList[i].color, true, pathList[i].shapeName); | ||
drawThisTurn = 1; | ||
pathList[i].drawn = 1; | ||
} | ||
} | ||
} | ||
} | ||
//purge | ||
//could be done more in a smarter order, that's why drawn is an element of pathList[] and not a separate array | ||
for (var i = 0 ; i < pathList.length ; i++){ | ||
if(pathList[i].drawn == 0){ | ||
this.add(pathList[i].path, pathList[i].color, true, pathList[i].shapeName); | ||
} | ||
} | ||
}; | ||
|
||
|
||
//+ Jonas Raoni Soares Silva | ||
//@ http://jsfromhell.com/math/is-point-in-poly [rev. #0] | ||
//see also http://jsperf.com/ispointinpath-boundary-test-speed/2 | ||
function isPointInPoly(poly, pt){ | ||
for(var c = false, i = -1, l = poly.length, j = l - 1; ++i < l; j = i) | ||
((poly[i].y <= pt.y && pt.y < poly[j].y) || (poly[j].y <= pt.y && pt.y < poly[i].y)) | ||
&& (pt.x < (poly[j].x - poly[i].x) * (pt.y - poly[i].y) / (poly[j].y - poly[i].y) + poly[i].x) | ||
&& (c = !c); | ||
return c; | ||
} | ||
|
||
|
||
/** | ||
* Does polygonA has intersection with polygonB ? | ||
* Naive approach done first : approximate the polygons with a rectangle | ||
* Then more complex method : see if edges cross, or one contained in the other | ||
*/ | ||
Isomer.prototype._hasIntersection = function(pointsA, pointsB) { | ||
var i, j; | ||
var AminX = pointsA[0].x; | ||
var AminY = pointsA[0].y; | ||
var AmaxX = AminX; | ||
var AmaxY = AminY; | ||
var BminX = pointsB[0].x; | ||
var BminY = pointsB[0].y; | ||
var BmaxX = BminX; | ||
var BmaxY = BminY; | ||
for(i = 0 ; i < pointsA.length ; i++){ | ||
AminX = Math.min(AminX, pointsA[i].x); | ||
AminY = Math.min(AminY, pointsA[i].y); | ||
AmaxX = Math.max(AmaxX, pointsA[i].x); | ||
AmaxY = Math.max(AmaxY, pointsA[i].y); | ||
} | ||
for(i = 0 ; i < pointsB.length ; i++){ | ||
BminX = Math.min(BminX, pointsB[i].x); | ||
BminY = Math.min(BminY, pointsB[i].y); | ||
BmaxX = Math.max(BmaxX, pointsB[i].x); | ||
BmaxY = Math.max(BmaxY, pointsB[i].y); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we maybe clean up these lines? A big wall of
I think I'd prefer the naming convention: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't know this syntax, obviously better for clarity There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're going to be mapping over arrays and getting properties out, maybe prop would be a useful addition? |
||
} | ||
|
||
if(((AminX <= BminX && BminX <= AmaxX) || (BminX <= AminX && AminX <= BmaxX)) && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment explaining this big conditional? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I will. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So it won't cover every case? I can imagine two triangles whose bounding boxes intersect, but they themselves do not intersect. How would that case be handled? Something to worry about? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It covers every case, I was not clear enough. Let me explain in another way. IF the bounding boxes of the polygons do not intersect, return false (this is very fast to compute, and it is a common case) ELSE check if the edge of one polygon crosses an edge of the other, or if one polygon is inside the other (this takes more time to compute). So the point is that we do this bounding boxes thing to avoid doing the complex part every time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A-ha, makes sense! Maybe leave a comment to that effect? |
||
((AminY <= BminY && BminY <= AmaxY) || (BminY <= AminY && AminY <= BmaxY))) { | ||
// now let's be more specific | ||
var polyA = pointsA.slice(); | ||
var polyB = pointsB.slice(); | ||
polyA.push(pointsA[0]); | ||
polyB.push(pointsB[0]); | ||
|
||
// see if edges cross, or one contained in the other | ||
var deltaAX = []; | ||
var deltaAY = []; | ||
var deltaBX = []; | ||
var deltaBY = []; | ||
var rA = []; | ||
var rB = []; | ||
for(i = 0 ; i <= polyA.length - 2 ; i++){ | ||
deltaAX[i] = polyA[i+1].x - polyA[i].x; | ||
deltaAY[i] = polyA[i+1].y - polyA[i].y; | ||
//equation written as deltaY.x - deltaX.y + r = 0 | ||
rA[i] = deltaAX[i] * polyA[i].y - deltaAY[i] * polyA[i].x; | ||
} | ||
for(i = 0 ; i <= polyB.length - 2 ; i++){ | ||
deltaBX[i] = polyB[i+1].x - polyB[i].x; | ||
deltaBY[i] = polyB[i+1].y - polyB[i].y; | ||
rB[i] = deltaBX[i] * polyB[i].y - deltaBY[i] * polyB[i].x; | ||
} | ||
|
||
for(i = 0 ; i <= polyA.length - 2 ; i++){ | ||
for(j = 0 ; j <= polyB.length - 2 ; j++){ | ||
if(deltaAX[i] * deltaBY[j] != deltaAY[i] * deltaBX[j]){ | ||
//case when vectors are colinear, or one polygon included in the other, is covered after | ||
//two segments cross each other if and only if the points of the first are on each side of the line defined by the second and vice-versa | ||
if((deltaAY[i] * polyB[j].x - deltaAX[i] * polyB[j].y + rA[i]) * (deltaAY[i] * polyB[j+1].x - deltaAX[i] * polyB[j+1].y + rA[i]) < -0.000000001 && | ||
(deltaBY[j] * polyA[i].x - deltaBX[j] * polyA[i].y + rB[j]) * (deltaBY[j] * polyA[i+1].x - deltaBX[j] * polyA[i+1].y + rB[j]) < -0.000000001){ | ||
return true; | ||
} | ||
} | ||
} | ||
} | ||
|
||
for(i = 0 ; i <= polyA.length - 2 ; i++){ | ||
if(isPointInPoly(polyB, {x:polyA[i].x, y:polyA[i].y})){ | ||
return true; | ||
} | ||
} | ||
for(i = 0 ; i <= polyB.length - 2 ; i++){ | ||
if(isPointInPoly(polyA, {x:polyB[i].x, y:polyB[i].y})){ | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
} else { | ||
return false; | ||
} | ||
|
||
}; | ||
|
||
/** | ||
* Adds a path to the scene | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of another parameter i'd like to have a bunch of
add
calls, which maybe just add them to the scene. A finalrender
method will order and draw them, what do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This expertMode parameter was for backward compatibility,
but if it's not needed we can remove it and do the add-add-render instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah let's run with that :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're super hesitant feel free to copypasta this
add
method and I can make the transition.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we keep the "name" parameter, which is to adress #34 ? (useful to remove/modify an element of the scene during an animation)