Skip to content

Commit f94b274

Browse files
committed
Make crossed quadrilaterals fill only the region containing the fill point.
This is to fix a comment I placed in the code about filling quadrilaterals, and the comment made by @Alex-Jordan in #1191 (comment). The code implemented in #1191 fills the entire interior of a quadrilateral even if it is a crossed quadrilateral (that is a quadrilateral with two edges that intersect). That is inconsistent with all of the other graph tool objects in some sense. To be more consistent only the region inside the quadrilateral that satisfies the same inequalities relative to the borders should be filled. So that is what this does. I also was doing some benchmarking of the TikZ fill code used for generating hardcopy and noticed that some things were rather slow. The slow down it turns out was using MathObject Reals in computations. There is no need for the MathObject structure in those computations, so all of the fill code now uses the perl values instead. It isn't even funny how much faster computations are with pure perl numbers versus MathObject Reals. The difference is rather extreme. Performing something like 10000 basic arithmetic operations with MathObject Reals takes more than five seconds, but with pure perl numbers it takes less than 20 milliseconds.
1 parent 8613565 commit f94b274

File tree

3 files changed

+181
-88
lines changed

3 files changed

+181
-88
lines changed

htdocs/js/GraphTool/quadrilateral.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,9 @@
6565
].join(',');
6666
},
6767

68-
// Note that this is an interior fill which is a bit inconsistent with the other fill cmp methods for other
69-
// graph objects. Other grap objects use an inequality fill.
7068
fillCmp(gt, point) {
7169
// Check to see if the point is on the border.
72-
for (i = 0, j = this.definingPts.length - 1; i < this.definingPts.length; j = i++) {
70+
for (let i = 0, j = this.definingPts.length - 1; i < this.definingPts.length; j = i++) {
7371
if (
7472
point[1] <= Math.max(this.definingPts[i].X(), this.definingPts[j].X()) &&
7573
point[1] >= Math.min(this.definingPts[i].X(), this.definingPts[j].X()) &&
@@ -89,7 +87,13 @@
8987
// Check to see if the point is inside.
9088
const scrCoords = new JXG.Coords(JXG.COORDS_BY_USER, [point[1], point[2]], gt.board).scrCoords;
9189
const isIn = JXG.Math.Geometry.pnpoly(scrCoords[1], scrCoords[2], this.baseObj.vertices);
92-
if (isIn) return 1;
90+
if (isIn) {
91+
let result = 1;
92+
for (const [i, border] of this.baseObj.borders.entries()) {
93+
if (gt.sign(JXG.Math.innerProduct(point, border.stdform)) > 0) result |= 1 << (i + 1);
94+
}
95+
return result;
96+
}
9397
return -1;
9498
},
9599

@@ -143,6 +147,8 @@
143147
Math.abs(x - groupedPoints[0].X()) < JXG.Math.eps &&
144148
Math.abs(y - groupedPoints[0].Y()) < JXG.Math.eps
145149
) {
150+
let xDir = 0,
151+
yDir = 0;
146152
// Adjust position of the point if it has the same coordinates as its only grouped point.
147153
if (e.type === 'pointermove') {
148154
const coords = gt.getMouseCoords(e);

htdocs/js/GraphTool/triangle.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,8 @@
135135
Math.abs(x - groupedPoints[0].X()) < JXG.Math.eps &&
136136
Math.abs(y - groupedPoints[0].Y()) < JXG.Math.eps
137137
) {
138+
let xDir = 0,
139+
yDir = 0;
138140
// Adjust position of the point if it has the same coordinates as its paired point.
139141
if (e.type === 'pointermove') {
140142
const coords = gt.getMouseCoords(e);

0 commit comments

Comments
 (0)