Thanks for the review, Anthony!
Regards,
Anton.
On 04.02.2014 22:55, Anthony Petrov wrote:
Hi Anton,
I skimmed through the code that I'm familiar with, and the changes look good to me. Someone from
Swing and 2D should review their parts, too.
--
best regards,
Anthony
On 2/3/2014 6:36 PM, Anton V. Tarasov wrote:
Hi Jim,
Please look at the updated version:
http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.4
On 01.02.2014 5:35, Jim Graham wrote:
Hi Anton,
On 1/31/14 6:37 AM, Anton V. Tarasov wrote:
My understanding is that, unless the fix is absolutely irrelevant (whic
I hope it isn't), we should integrate it into 9/8u20 to support
SwingNode in its current implementation on Retina displays.
What do you think?
I agree with this sentiment. My suggestion for reducing its footprint
aside (which it looks like you are investigating), the main thing I
will be looking for is whether or not we return an object to a
developer (i.e. it isn't just managed internally to our own code)
where they can do "instanceof BufferedImage" and then see something
odd coming from its width/height.
It looks like if we simply use getLayoutWH() internally then they
would never ever see anything odd from getWidthHeight() anyway. The
only additional "gotcha" would be if they grab the image and render it
directly and it comes out an unexpected size to them (because, for
instance, they didn't check the layout dims like we do internally).
That's a pretty minor issue, though, and I think we could live with it.
I've refactored the fix with this concern in mind. Here is what I've done:
- eliminated the new OffscreenHiDPIImage class (as you suggested) and
put all of its "scale" logic into the OffscreenImage.
- made the scaled OffscreenImage return the physical size (like an
ordinary BufferredImage does) by default .
- renamed the "set/isHiDPIEnabled" method to "set/isReturnLayoutSize".
- made the setReturnLayoutSize(true) be called internally (where we
don't leak the OffscreenImage).
In SG2D, the drawHiDPIImage, for instance, makes a call to
op.filter(img, null); where the img is expected to return its layout
size. That's why I still prefer to use the setReturnLayoutSize
"switcher", in order not to go deep into the 2D rendering code, casting
here and there to OffscreenImage and calling getLayoutWidth/Height.
For 9.0 perhaps we could add the LayoutWH() as new API on
BufferedImage and then most of this would be public? I'd have to mull
over if that makes sense and I'm not entirely sure of the naming yet...
That's a good idea, however we need a 8u working solution as well... As
to the naming, I'm ready for any suggestions.
Thanks,
Anton.
...jim
