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

[Jim Graham]

Hi Denis,

On 10/19/2010 10:40 AM, Denis Lila wrote:
>> ROCsq - I looked at the wikipedia article and it wasn't clear how it
>> directly related to your function since the article is dealing with
>> the curvature of a function graphed against its own t, but you are dealing
>> with 2 parametric equations combined and graphed against each other.
>> I think I'm going to have to just trust you on this one for now.  :-(
>
> http://en.wikipedia.org/wiki/Radius_of_curvature_%28applications%29
> Did you look at the above wikipedia article? When researching I came across
> 2 of them, and one of them only mentions natural parameterizations, but
> the above has the general equation for a R->Rn function, then below that
> they have the special case where n=2, x(t)=t, and y(t)=f(t). I used the
> first equation on that page.
> Actually, I wrote a simple program to make sure the radius of curvature
> function was correct. I have attached it. It's not a proof, but I think
> it is convincing. Just hold down the mouse button and move it horizontally.
> This will change the t value on the curve and the circle drawn will have
> radius equal to Math.sqrt(ROCsq). You can also change the control points of
> the curve. There's a bug where when you run it the window is really tiny, so
> you have to manually resize it and maximize it.

I actually did read that article, but I wasn't seeing the fact that it 
was a multiple parametric equation and that the || were distance 
calculations rather than simply absolute values.  Now I see it. 
Plugging those concepts in to the first equation the mapping is very direct.

One thing that confused me when I was proof-reading it was that the 
numerator seemed to be dx2dy2 squared when it should be cubed.  Then I 
spotted the final "*dx2dy2" term at the end which makes it cubed.  I 
wasn't sure why you isolated that term out there instead of just 
grouping it with the rest of the numerator - is there a danger of 
overflow if you multiply it before you do the division?  If so, then 
that is fine since it doesn't actually affect the number of fp ops so it 
should be the same performance.

>> lines 621-626 and 643-646 - I'm not sure what the derivation of these
>> lines are.  I tried to do my own equations, but I seem to be heading
>> in a different direction than you used and it didn't seem like I was
>> going to converge.  What equations did you set up to solve to get these
>> calculations?  From what I can see we have:
>>     - new p1,p4 are calculated
>>     - new p(0.5) is calculated
>>     - the corresponding dx,dy at t=0,0.5,1 (gives slopes)
>>     - slopes at t=0, 0.5 and 1 should be the same for all curves
>> and what you are trying to compute are two scaling constants c1 and c2
>> that represent how much to scale the dx1,dy1 and dx4,dy4 values to get
>> a curve that interpolates both position and slope at t=0.5.  A comment
>> might help here...  :-(
>
> I see how (dxm,dym) was confusing. The only reason for computing the slope
> at t=0.5 is to get the point of the offset curve at t=0.5. We don't make
> the computed curve and the input curve have equal slopes at t=0.5 because
> this would give us an overdetermined system. What we're trying to do in this
> function is to approximate an ideal offset curve (call it I) of the input
> curve B using a bezier curve Bp. The constraints I use to get the equations are:

It does help *a lot*, though, so thank you for writing it up.  I would 
move it closer to the code in question since the function has such a 
long preamble that separates the comment from the code that implements 
it (also method comments are usually reserved for API documentation 
purposes).

lines 544,559 - I'd remove the line numbers from the comment.  They are 
already wrong and they won't survive any more edits any better.  ;-)

In #2, you have a bunch of "I'() || B'()" which I read as "the slope of 
the derivative (i.e. acceleration) is equal", don't you really mean "I() 
|| B()" which would mean the original curves should be parallel? 
Otherwise you could say "I'() == B'()", but I think you want to show 
parallels because that shows how you can use the dxy1,dxy4 values as the 
parallel equivalents.

I would rename det43 to invdet43 to indicate that it is the inverse of 
the determinant.  I kept looking at it and thinking "he has the 
determinant in the wrong side" until I noticed that it was in the 
denominator of det43 (which is hard to read in parenthesized C-math).

One side note.  At first glance I would have thought that the final 
equations would have subtracted the c2*dxy4 terms rather than adding 
them (since dxy4 represent p4-p3, not p3-p4 and so the linear 
interpolation equation "looks" backwards), but that isn't true because 
you did all of your math looking to find the c2 that belongs in this 
equation (as "backwards" as it seems) and so you got that answer. 
Interestingly if you look at the effect on the results if you calculate 
the dxy4 terms the other way around, they are simply negated and the 
impact would be that c1 would be unaffected (both num and denom are 
negated) and c2 would be negated (only det43 is negated), so it works 
out the same either way.  Fun...

> I think using Bp'(0.5) || I'(0.5) instead of Bp(0.5)==I(0.5) only eliminates
> 1 variable and leaves us with 1 unknown. This brings up the interesting possibility
> of replacing constraint 3 with: Bp'(1/3) || I'(1/3) and B'(2/3) || I'(2/3). I guess
> this is something to implement sometime in the future just to compare with the
> current equations. But what we have now works pretty well, so I'm in no rush.

No, the existing stuff is clean and works fine so let's leave it - for 
now at the very least...

			...jim