[OpenJDK 2D-Dev] Review Reqeust for Bug 100068 - SunGraphics2D exposes a reference to itself while non fully initialised

[Jim Graham]

Hi Mario,

Mario Torre wrote:
> So here is the deal. I'm going back to the original plan to fix the 
> issue, this time without accessors. I'll just invalidate the loops and 
> revalidate them elsewhere. The question that still remains is when to 
> re-validate the loops? My idea is to make the pipes that use the loops 
> to tell this themselves, so we cover future problems if we're going to 
> refactor this stuff more. I'm going to work on this a bit this week, of 
> course, if you already planned some direction, just tell me (maybe I 
> wait your reply this time :)

And I'll try to give it in a timely fashion.  ;-)

I think it would be nice to have the pipes manage the need for the 
loops, but it could be touchy to try to work this into the logic in some 
of the spaghetti validate methods.  (I'm not recommending against it, 
just warning you to keep an eye out for if you start rat-holing trying 
to get that to work.)

A couple of ways I can see this working (in order of my estimation of 
performance impact, low to high):

- pipe interfaces implement a new interface "LoopBasedPipes" which has 
no methods, it is just a "marker" interface.  The test in validate() 
just uses instanceof in this case which might be faster than an 
interface method dispatch (actually, it almost certainly would be).

- pipe interfaces add "public boolean needsLoops()" which requires an 
interface method call per pipe.

- pipe interfaces add "public void setup(SG2D)" and they add the loops 
if they are null (loops are set to null on invalidate), but this would 
require lots of "if null" checks and the implementation would be pretty 
much the same everywhere.  Validate would have to call the setup on 
every pipe before it returns.

To test if you've impacted performance I don't think we have any test 
cases that measure this particular code path.  I would use a test case 
that does something cheap to induce a revalidate, like:

      setRH(AA_ON)
      // No need to render, just use it to invalidate
      setRH(AA_OFF)
      // Now that the AA is set back to the fast mode,
      // render something tiny
      fillRect(1x1)

in a tight loop.  If there is a cheaper attribute to tweak to cause 
invalidations that would be better - I think the AA hint is trapped and 
just sets a field internally, but maybe setting a Composite rule back 
and forth might be even cheaper.  Simply changing the color would not 
tend to cause an invalidate (though changing it from solid to 
translucent might, but the pixel value would have to be revalidated as 
well).

			...jim