Hi Mario,
The changes look safe, but I wanted to bring up the following issues:
- There are a dozen or so fields in SG2D that are commonly accessed
everywhere in the pipelines including loops. These changes introduce an
accessor for loops, but that now looks out of place with no accessors
for any of the other fields that get accessed directly. Personally the
lack of accessors hasn't bothered me, particularly since this is
performance sensitive code and accessors can sap performance by nickels
and dimes if you don't implement them correctly - to wit:
- Accessors can be inlined if they are final, but these aren't. As it
turns out, SG2D itself is final and so I believe that is enough for them
to be inlined, but I tend to make methods final as well to underscore
that they are intended to be inlined, and also in case we eventually
decide to make the class non-final. On the other hand, we haven't
really bothered with accessors in the first place in this code to avoid
the code bloat and the potential points of heuristic failure.
- I agree that it would be nice to ask the pipes if they need loops, I'm
not sure why your solution didn't work, but I prefer that to making them
a side effect of getTextPipe() since it makes it harder to know when the
code you are editing needs to get the loops or not, and it makes it hard
to see where they come from when editing the many validatePipe() methods.
...jim
Mario Torre wrote:
Il 18/06/2009 21:52, Jim Graham ha scritto:
One solution would be to always set the loops for the validations that
install one of these pipes. That could have potential performance
impact, but it would be no worse than the validation sequences that
already set the loops every time so I don't think it would be serious,
or even measurable.
Hi Jim,
I'm trying this approach.
Basically, I just set the loops to null now in invalidate and validate
them back in validatePipe:
http://cr.openjdk.java.net/~neugens/100068/webrev.03/
What I do is to get valid Loops when we call getTextPipe, but other than
that, the behavior is basically unchanged. I tried with various
applications, including NetBeans, the Java2D demo and the FontTest app,
playing around with the text configurations (AA, LCD, size, Glyphs etc..
even output to a PNG file) and looks good, but I may miss something of
course.
I moved to a 64 bit machine, I don't think this makes any difference in
this case, but maybe it's a good thing to say.
I would like to see a method in the pipelines that does something like:
boolean mustInitialiseLoopsBecauseWeReallyWantToUseThemGranted()
or so, that we may call at the end of the validatePipe method, but when
I tried this I got a funny StackOverflowException, maybe I did something
wrong, but looks like it's not so easy to change this code and still
make it behave correctly, so I would like to go deeper in this issue
even if you think we can close the bug (of course if the proposed fix is
evaluated as complete).
Cheers,
Mario
