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

[Denis Lila]

Hi Jim.

> How about this:
> 
> 	(Math.abs(len-leftLen) < err*len)
> 
> (noting that err*len can be calculated outside of the loop).

This is what I use now.

> Note that a custom shape can send segments in any order that it wants
> so close,close can happen from a custom shape even if Path2D won't do
> it.

Right, of course. For some reason I only considered the test case I was
using. Sorry for slip up.

> There is only one question on the board from my perspective - the 
> question about dash length errors at the start of this message. 

I'm ok with the accuracy of dash length computations. My main concerns
right now are dashing performance and AA rendering performance. The
latter has improved, but not by as much as I was expecting. Also, it was
bothering me that when I removed PiscesCache 1-2 weeks ago performance
got worse.
It would also be nice to find a method that is guaranteed to find
all offset curve cusps.

> Depending on how you feel about that I think we're ready to go

That's great. I will be pushing today.

> (and I have some ideas on further optimizations to consider if you are still
> game after this goes in)...

I'd love to hear what they are.

Thank you for all your time,
Denis.

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

> On 10/22/2010 12:22 PM, Denis Lila wrote:
> > Because the error is meant to be relative. What I use is supposed to
> be
> > equivalent to difference/AverageOfCorrectValueAndApproximation< 
> err,
> > where, in our case,
> AverageOfCorrectValueAndApproximation=(len+leafLen)/2,
> > so that multiplication by 2 should have been a division.
> > Should I use the less fancy differece/CorrectValue<  err and skip
> > a division by 2?
> 
> If it is relative, then shouldn't it be relative to the desired
> length? 
>   Why does the computed approximation factor in to the size of the
> error 
> you are looking for?  If you use the average then the amount of error
> 
> that you will "accept" will be larger if your estimate is wronger.  I
> 
> don't think the "wrongness" of your approximation should have any
> effect 
> on your error.
> 
> >> lines 98-101 - somewhere in the confusion moveToImpl became
> redundant
> >> with moveTo.
> >
> > I thought I'd leave these in because they're interface methods,
> > and it's usually not a great idea to call these from private
> methods. I've
> > removed them anyway, because you said so and because I suppose
> > if ever we need to do something to the user input that shouldn't be
> done
> > to input coming from private methods in the class (such as the
> scaling
> > of the input coordinates done in Renderer) we can always easily
> > reintroduce them.
> 
> I actually thought about the interface concept after I sent that and
> was 
> at peace with them, but I'm also at peace with them being gone. ;-)
> 
> > That's right. I don't think it's what should happen, but it's what
> closed
> > jre does, so I've left it in (with some changes to make it actually
> > replicate the behaviour of closed java, since it was buggy - the
> moveTo
> > was missing). I don't draw anything on the second close in the
> close,close
> > case, since that would look horrible with round joins and square
> caps.
> > However, the way path2D's are implemented this safeguard will not
> be
> > activated since a closePath() following another closePath() is
> ignored.
> 
> > I also now initialize the *dxy *mxy variables in moveTo. This should
> be an
> > inconsequential change, but I've put it in just so the state of
> > Stroker after a moveTo is well defined.
> 
> New code looks good (even if we think it's a silly side effect of
> closed 
> JDK to have to implement).
> 
> >> line 368 - does this cause a problem if t==1 because lastSegLen
> will
> >> now return the wrong value?  Maybe move this into an else clause on
> the
> >> "t>=1" test?
> >
> > It does cause a problem. I've fixed it by adding a variable that
> keeps track
> > of the length of the last returned segment. The way it was being
> done was
> > a dirty hack because it assumed that if the method didn't return in
> the loop,
> > done would remain false.
> 
> Woohoo!  I was actually bothered by the side-effecting in the old code
> - 
> this is a better approach.
> 
> > I made one more change in dasher: in somethingTo I removed the long
> comment
> > near the end, and I handle the case where phase == dash[idx]
> immediately.
> > I do this for consistency with lineTo. The only instances where this
> makes
> > a difference is when we have a path that starts with a dash, ends at
> exactly
> > the end of a dash, and has a closePath. So something like
> move(100,0),
> > line(0,0),line(0,50),line(100,50),line(100,0),close() with a dash
> pattern {60,60}.
> > In closed jdk (and also in openjdk with my patch), no join would be
> drawn
> > at 100,0 on this path with this dash pattern.
> > Whether that's correct behaviour is arguable, imho. On one hand, if
> we don't do it
> > , but I think it is because if
> > it isn't done certain dashed rectangles start looking weird at the
> top left.
> 
> I think it probably should not have a join since we don't join two 
> segments if one of the "OFF" lengths is 0.  We should probably only
> save 
> the first dash if it starts in the middle of a dash.
> 
> Perhaps the closed JDK is wrong here.
> 
> > Now, consider a path like
> move(100,0),line(0,0),curve(0,100,100,100,100,0),close()
> > with a dash pattern of {60,60}. The length of the curve in this is
> exactly 200 (I
> > have not proven this, but it seems clear since the more I increase
> precision, the
> > closer to 200 the computed length becomes. Also, if you render it
> with a dash pattern
> > of {10,10} in closed jdk, exactly 10 full dashes are drawn). So,
> this path has exactly the same
> > length as the above path. However, if this path is rendered with
> closed jdk a join
> > will be drawn at (100,0). This behaviour is inconsistent with the
> line only path
> > For this reason, I think this is a bug in closed jdk since dashing
> > should depend only on the path's length, not on the kind of segments
> in a path.
> 
> Note that curve length is imperfect so their behavior may differ
> because 
> the above path had exact lengths that didn't rely on fp calculations 
> with errors to get it right, but the curved case had a lot of 
> computations.  Even if they believe the last dash had .000000001 left
> on 
> it, they would close, even though they believed the length of the
> curve 
> to be "about 200".
> 
> > Handling the case where phase==dash[idx] immediately fixes this
> problem.
> 
> Good point.
> 
> > I also moved the second if statement in the loop of lineTo inside
> the first if
> > for symmetry with how this case is handled in somethingTo. The logic
> is the same.
> 
> Looks good.  I was going to say that, but figured it was too "nitty"
> to 
> even mention (perhaps you're getting the hint that I'm a bit OCD about
> 
> code... ;-)
> 
> > I reran all the gfx tests. Nothing has changed.
> 
> Woohoo!
>
> 
> 			...jim