[OpenJDK 2D-Dev] X11 uniform scaled wide lines and dashed lines; STROKE_CONTROL in Pisces

Denis Lila dlila at redhat.com
Tue Dec 7 00:21:25 UTC 2010


Hi Jim.

> line 134 - what if numx or numy == 0 because only roots outside [0,1]
> were found?

In this case lines 151-162 will execute, and nothing is wrong. The only
problem is when both numx and numy are 0. This is certainly possible in
the general case (but only with quadratic curves), but the way we're using
this function, the intermediate value theorem guarantees at least one root
will be found. Of course, finite precision math doesn't necessarily care
what calculus has to say, but in this case I can't see how the root computation
could fail. On the other hand, one could argue that this is exactly why we need
to defend against this case, so I've added some checks. 

> line 145 - what if d0 and d1 are both 0?  NaN results.  What if you
> just used a simple "d0 < d1 ? tx : ty" - how far off would that be?

I tried that out on a curve with very high acceleration, and it makes a noticeable
difference. So, instead I'm using 
        	if (w0 == Float.NaN) {
        		return tx;
        	}

> line 152,154 - you are likely picking the most negative distance here,
> not the closest to zero.  abs() might help.

Good point.

> line 187 - is that correct?  Shouldn't it be "(...) + 0.5*(...)"?
>           - if that is true then perhaps the change in scale didn't
>             really reduce the number of multiplies?

You're right.

> line 220 - any reason to move this method?

No, just an accident while I was experimenting with different algorithms.

> line 238 - ah, perhaps the lion's share of the performance
> improvement?

Indeed. Well, maybe about 75% of it. A surprisingly large part of the
improvement came from changing all the Math.hypot calls to Math.sqrt calls.

> line 357 - another optimization would be to check the acceleration in
> the range and if it is below a threshold then simply use the t from
> line 348 as the t for the split

I like this. I tried implementing it. I haven't tested it yet though, and
I still have to pick a good cutoff acceleration. For now I'm using 1/leaflen.
We definitely don't want it to just be a constant, since the longer the leaf,
the worse it will be to allow acceleration, so for longer leaves we want to
skip the getTCloseTo call only if the acceleration is smaller.

> line 83 - conflicts with test at line 89

Fixed it.

> Renderer.java:  Is this just a straight copy of what I was working on?

I'm pretty sure it is, yes.

> I hate to be a whiner, but I'd rather avoid having to go over every
> line and check it... ;-)

I can't blame you :)

> TransformingPathConsumer2D:
> 
> Any thoughts on whether we need translation in the scale filter and 
> whether a 4-element non-translation filter would be useful?  I think
> the current code that drives this passes in the full transform and its 
> inverse, but it could just as easily do delta transforms instead and 
> save the adding of the translation components...

I thought about this long ago, but I wasn't sure it wouldn't break anything.
Today, I got a bit more formal with the math, and I think it's ok
to eliminate the translation. I've implemented this (though, I haven't had
time to test it yet. That's for tomorrow).

The link is: http://icedtea.classpath.org/~dlila/webrevs/perfWebrev/webrev/

Thank you,
Denis.

----- "Jim Graham" <james.graham at oracle.com> wrote:

> 
> 		...jim
> 
> On 12/1/2010 9:19 AM, Denis Lila wrote:
> > Hi Jim.
> >
> > About two weeks or so ago I replied to one of your very old e-mails
> > about dashing performance and I included a link to the webrev:
> > http://icedtea.classpath.org/~dlila/webrevs/perfWebrev/webrev/
> >
> > I suspect you might have missed it, so that's why I'm writing this.
> > If you haven't, I apologize for this e-mail.
> > Anyway, here's the e-mail:
> >
> http://mail.openjdk.java.net/pipermail/2d-dev/2010-November/001654.html
> >
> > I also just found a bug in the cubic roots math: in the case where
> the function
> > was quadratic, I was forgetting to filter the computed roots to
> include
> > only ones in the range [A, B).
> >
> > Regards,
> > Denis.
> >
> > ----- "Jim Graham"<james.graham at oracle.com>  wrote:



More information about the 2d-dev mailing list