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

[Jim Graham]

Hi Denis,

Line 1099 - I decided to check out Cordano's method and noticed a 
discrepancy.  The comment here says we are calculating the p and q for 
this equation, but the values assigned to the p and q variables in lines 
1102,1103 happen to be p/3 and q/2.  That's fine because almost all of 
the values needed in the remaining logic in Cordano's method actually 
need those values instead of the original p and q so it makes sense to 
calculate them up front.  Unfortunately, this means that the names here 
and the values assigned to them and the comment above them conflict.  If 
the variables could be named "p/3" and "q/2" then all would be clear, 
but I don't know how to do that naming very easily.  Perhaps the comment 
could be simply reworded:

     // substitute <blah blah blah>
     //    x^3 + Px + Q = 0
     // Since we actually need P/3 and Q/2 for all of the
     // calculations that follow, we will calculate
     //    p = P/3
     //    q = Q/2
     // instead and use those values for simplicity of the code.

Line 1105 - single quotes in comments freaks out my version of gnuemacs. 
  I usually try to avoid them, except in pairs, but there isn't a better 
way to word this comment.  :-(

Lines 1157-1163 - the old code used to copy the eqn before it got 
clobbered with roots.  Here it is too late.  You probably need to move 
this code up near line 1135 before the 3 roots are stuffed into the res 
array.  (Did you test the eqn==res case?)

I noticed that the "Casus irreducibilis" case isn't in Cordano's method. 
  He only finds roots for the 1 and 2 root case and punts for 3 roots. 
So, this is someone else's method.  It would be nice to figure out who 
or what and list a reference, even though the Graphics Gems and the old 
code didn't.  The closest reference I can find is unattributed on 
Wikipedia, but you could include it in a comment for reference:

http://en.wikipedia.org/wiki/Cubic_function#Trigonometric_.28and_hyperbolic.29_method

Line 1133 - I don't understand why that term has -q in it.  The above 
link and the original code both computed essentially the arccos of this 
formula without the negation of q.  ???  Since acos(-v) == pi - acos(v) 
this would seem to negate the result and bias it by pi/3.  Negating it 
won't affect the eventual cosine, but the bias by pi/3 will.  Am I 
missing something?

			...jim

On 12/20/2010 9:36 AM, Denis Lila wrote:
> Hi Jim.
>
>> Lines 1094-1096, they could also be NaN if any of the numerators were
>> also zero and these tests might fail (but only for the case of all of
>> them being zero I guess, otherwise one of the other divisions would
>> result in infinity). Are accidental infinities (caused by overflow
>> rather than d==0.0) really a problem for the code to handle?
>
> I'm not sure if they're a problem, but I thought that case should have
> been handled just for robustness. However, I've changed the test
> to d==0 because testing for infinities should be done, but not there.
> For example, if the constant term was huge and d==0.5 we could get
> an infinity but that shouldn't really be handled as a quadratic
> polynomial. I will deal better with these cases in a future webrev.
>
>> I just noticed that the code you are replacing actually used to refine
>> the roots so maybe you should do some of this magic.
>
> I missed that in the original code. I changed it now.
>
>
> Also, in the webrev you'll find five regression tests that I would like
> to push to openjdk7. They test for various problems the rendering engine
> used to have. They're all pretty simple and I would appreciate it if you
> could take a quick look at them. They're in the same webrev as cc2d because
> it was more convenient for me, but obviously when/if they're pushed they
> will be a separate changeset.
>
> One more thing: there is a regression test for the rendering engine
> called TestNPE that I think is problematic because it doesn't
> necessarily test the rendering engine. It just draws an antialiased
> line, which could be handled in any number of ways, so it's not very
> robust. In fact, after we're done with the parallelogram pipelines,
> the code that used to throw the NPE won't even execute, making this
> test useless. We should either discard it or change it to use the
> rendering engine explicitly, like my tests do. What do you think?
>
> Regards,
> Denis.