[cc'ing to j2d]
Jim, let me continue this thread later when a general agreement about a scaled
BufferedImage is made.
Thanks,
Anton.
On 12.12.2013 1:44, Jim Graham wrote:
Hi Anton,
On 12/11/13 5:13 AM, Anton V. Tarasov wrote:
Hi Jim,
On 11.12.2013 13:14, Jim Graham wrote:
In BufferedImage, the private fields and methods will require an
accessor method for the inner class to access them. Is there a reason
they can't be left package-accessible?
Sorry, I don't clearly understand what you mean... The "scale" private
field and the new methods are accessed from other packages, so I can't
make them package-private. I've added the accessor which I put into
OffscreenImage (an internal class).
This is entirely an internal detail related to how inner classes are implemented (like the class
that you use to allow OffscreenImage to access the BI methods).
The inner "accessor helper" class is technically outside the class scope of the BufferedImage
class - it is a separate class and it is restricted in the byte code interpreter to the same
access privileges as any other "separate class", but the methods it calls on BI are private. The
runtime does not allow this, but the compiler wants you to think this is possible for convenience
of writing inner classes. As a result, it injects fake accessor calls into the BI class that
allow the inner classes to call private methods on BI. Dump the byte codes of the relevant
classes with javap and you will see the shadow calls that break the privacy of those methods.
Those methods are not allowed from java code, only the inner class magic can use them. It is
byte-code magic to enable the syntactic sugar of inner classes.
I believe if you just make the new methods on BI package-private instead of fully class-private,
then those helper inner-class-only accessor methods aren't needed.
In JViewport, is there a reason why a map of various scaled backing
stores are kept rather than just validating the existing backing store
against the new desired scale and replacing it when it changes? Does
the scale on the backing store ping-pong back and forth between scales
much?
This doesn't ping-pong much, indeed. So you want to say using a cache
here is not justified? Ok, why I did that is probably to mimic the logic
used by RepaintManager which does use a map b/w graphic configs and
volatile images (the volatileMap field), whereas changes of the
GraphicsConfig shouldn't occur often.
Anyway, this indeed doesn't increase performance but increases a
footprint, so I'm ok not to use cache here.
I just wanted an analysis of whether the new complexity had any value. It sounds like it mirrors
what was done in other places and that is fine, but perhaps this particular variant of that
similar mechanism is premature? In general, I'd wait to add complexity until a problem is
demonstrated that makes it worthwhile.
BufImgSurfaceData - you have to validate the transform as not flipping
or rotating (even quadrant rotation will violate your conditions. I
believe that testing the transform type with a mask consisting of the
TRANSLATE and SCALE_MASK flags should be fine. Also, do we wan to set
a precedent of allowing copyArea under an arbitrary scale, or should
we test it to make sure it is specifically the same scale as the
device scale factor?
Ok, I'll check the transform for TRANSLATE/SCALE only. The same scale as
the device works for me, so I'll limit it to that case.
Though, I do think we should eventually have copyarea work with scales.
BTW, this same code appears in SG2D, but it is rejected by a test on the transform type. Have you
thought about simply inserting your transform code into the test for the transform type in SG2D
rather than shadowing the entire method?
SG2D - I'm worried about the performance impliciations of adding a new
method in that creates and returns an array on every drawImage call.
Did you do a performance test with J2DBench to verify the impact of
these operations?
Well, this was done just to make the code more clear. I can remove that
method (and do the instanceof check in every place I called it), or as
an alternative I can cache the array so that not to create it (out of
the stack) every time. Does this make sense to you? (I didn't run
J2DBench yet).
A single SG2D should be single threaded (calling from multiple threads is not supported and
results are not guaranteed) so caching the array in the instance would be fine. Also, we are
discussing the issue of getWidth(null) not matching the logical size of the image in another
thread. Depending on what we decide there, it may make sense to move the scaling logic inside the
getWidth() methods of the scaled buffered images and then we need far fewer mods to SG2D. But, I
think that would only work reasonably if such "scaled buffered images" are never leaked to the
developer until we come up with a reasonable external API that makes sense for them.
I'm going to save the rest of the reply for another message since it gets
involved...
...jim