Hello.
I think I have a fix for this bug:
http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=504
The problem is caused by the "symmetric" variable in pisces/Dasher.java.
symmetric is set to (m00 == m11 && m10 == -m01), and never changed.
It is only used in one place (in lineTo) to simplify the computation of
the length of the line before an affine transformation A was applied to it.
This is why it causes a problem:
If A = [[a00, a01], [a10, a11]] and (x,y) is a point obtained by applying
A to some other point (x',y'), then what we want is the length of the vector
(x',y'), which is ||Ainv*(x,y)||. Ainv = (1/det(A)) * [[a11, -a01],[-a10, a00]],
so, after some calculations, ||Ainv*(x,y)|| ends up being equal to
sqrt(x^2*(a11^2 + a10^2) + y^2*(a00^2 + a01^2) - x*y*(a11*a01 + a00*a10)) *
1/|det(A)|.
If symmetric==true, this simplifies to:
sqrt((a11^2 + a01^2) * (x^2 + y^2)) * 1/|det(A)|, and
|det(A)| = a11^2 + a01^2, so, the final answer is:
sqrt((x^2 + y^2)) / sqrt(det(A)). Therefore the problem in Dasher.java
is that it divides by det(A), not sqrt(det(A)).
My fix for this was to remove the "symmetric" special case. Another possible fix
would have been to introduce an instance "sqrtldet" and set it to sqrt(det(A)),
and divide by that instead of det(A). This didn't seem worth it, because the
only
benefit we gain by having the "symmetric" variable is to save 3 multiplications
and 1 division per iteration of the while(true) loop, at the expense of making
the
code more complex, harder to read, introducing more opportunity for bugs, and
adding
hundreds of operations of overhead (since PiscesMath.sqrt would have to be
called to
initialize sqrtldet).
To make up for this slight performance loss I have moved the code that computes
the transformed dash vectors outside of the while loop, since they are constant
and they only need to be computed once for any one line.
Moreover, computing the constant dash vectors inside the loop causes
them to not really be constant (since they're computed by dividing numbers that
aren't constant). This can cause irregularities in dashes (see comment 14 in
http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=197).
I would very much appreciate any comments/suggestions.
Thank you,
Denis Lila.
exporting patch:
# HG changeset patch
# User Denis Lila <dl...@redhat.com>
# Date 1276630286 14400
# Node ID bf98ef6947227339cc951e0513af81b207d65af9
# Parent ae887ea4c7726c73aea4683a4b5cd921ec87bdb5
Fixed bug 504: http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=504
I also took the chance to fix the first issue discussed here:
http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=197 in comment 14, and
make some permance improvements to Dasher.java.
diff --git a/src/share/classes/sun/java2d/pisces/Dasher.java b/src/share/classes/sun/java2d/pisces/Dasher.java
--- a/src/share/classes/sun/java2d/pisces/Dasher.java
+++ b/src/share/classes/sun/java2d/pisces/Dasher.java
@@ -56,7 +56,6 @@
Transform4 transform;
- boolean symmetric;
long ldet;
boolean firstDashOn;
@@ -143,7 +142,6 @@
this.m10 = transform.m10;
this.m11 = transform.m11;
this.ldet = ((long)m00*m11 - (long)m01*m10) >> 16;
- this.symmetric = (m00 == m11 && m10 == -m01);
}
public void moveTo(int x0, int y0) {
@@ -181,51 +179,63 @@
}
public void lineTo(int x1, int y1) {
+ int lx = x1 - x0;
+ int ly = y1 - y0;
+
+ // Compute segment length in the untransformed
+ // coordinate system
+ // IMPL NOTE - use fixed point
+
+ int l;
+ long la = ((long)ly*m00 - (long)lx*m10)/ldet;
+ long lb = ((long)ly*m01 - (long)lx*m11)/ldet;
+ l = (int)PiscesMath.hypot(la, lb);
+
+ // Compute the number of transformed dash lengths that will
+ // be needed in the while(true) loop. We carry out the first
+ // iteration outside the loop so we can take into account phase.
+ int lenAcc = dash[idx] - phase;
+ int dashesToCompute = (lenAcc > l) ? 0 : 1;
+ for (int i = 1; i < dash.length; i++) {
+ lenAcc += dash[(i + idx) % dash.length];
+ if (lenAcc > l) break;
+ dashesToCompute += 1;
+ }
+
+ long t;
+ int[] dxsplit = new int[dash.length];
+ int[] dysplit = new int[dash.length];
+
+ // For each dash section, we compute the x and y component of the
+ // transformed dash.
+ for (int i = 0; i < dashesToCompute; i++) {
+ int j = (i + idx) % dash.length;
+ t = ((long)dash[j] << 16)/l;
+ dxsplit[j] = (int)(t * lx >> 16);
+ dysplit[j] = (int)(t * ly >> 16);
+ }
+
while (true) {
- int d = dash[idx] - phase;
- int lx = x1 - x0;
- int ly = y1 - y0;
-
- // Compute segment length in the untransformed
- // coordinate system
- // IMPL NOTE - use fixed point
-
- int l;
- if (symmetric) {
- l = (int)((PiscesMath.hypot(lx, ly)*65536L)/ldet);
- } else{
- long la = ((long)ly*m00 - (long)lx*m10)/ldet;
- long lb = ((long)ly*m01 - (long)lx*m11)/ldet;
- l = (int)PiscesMath.hypot(la, lb);
- }
-
- if (l < d) {
+ if (l < dash[idx] - phase) {
goTo(x1, y1);
// Advance phase within current dash segment
phase += l;
return;
}
- long t;
int xsplit, ysplit;
-// // For zero length dashses, SE appears to move 1/8 unit
-// // in device space
-// if (d == 0) {
-// double dlx = lx/65536.0;
-// double dly = ly/65536.0;
-// len = PiscesMath.hypot(dlx, dly);
-// double dt = 1.0/(8*len);
-// double dxsplit = (x0/65536.0) + dt*dlx;
-// double dysplit = (y0/65536.0) + dt*dly;
-// xsplit = (int)(dxsplit*65536.0);
-// ysplit = (int)(dysplit*65536.0);
-// } else {
- t = ((long)d << 16)/l;
- xsplit = x0 + (int)(t*(x1 - x0) >> 16);
- ysplit = y0 + (int)(t*(y1 - y0) >> 16);
-// }
+ if (phase == 0) {
+ xsplit = x0 + dxsplit[idx];
+ ysplit = y0 + dysplit[idx];
+ } else {
+ long p = (((long)dash[idx] - phase)<<16) / dash[idx];
+ xsplit = x0 + (int)((p * dxsplit[idx]) >> 16);
+ ysplit = y0 + (int)((p * dysplit[idx]) >> 16);
+ }
+
goTo(xsplit, ysplit);
+ l -= (dash[idx] - phase);
// Advance to next dash segment
idx = (idx + 1) % dash.length;
dashOn = !dashOn;
