[OpenJDK 2D-Dev] <AWT Dev> [9] Review Request: JDK-8029455 JLightweightFrame: support scaled painting

[Jim Graham]

Another idea is to simply have a "layout size" parameter (and scale 
maybe) on the OSI and by default it is the same as the Raster size, but 
then the cases where we know internally that we are going to be using 
the object for HiDPI we then set the layout size appropriately.  I think 
there may be a lot less boolean-testing code overall with that scheme, 
and I'm not sure you need a new subclass with the appropriate defaults - 
you can just use the base OSI with new attributes...?

			...jim

On 1/24/14 6:47 AM, Anton V. Tarasov wrote:
> Hi Anthony,
>
> I see your concern. I'll think of a better name.
>
> Thanks,
> Anton.
>
> On 24.01.2014 15:47, Anthony Petrov wrote:
>> Hi Anton,
>>
>> I suggest to rename the OffscreenHiDPIImage.hidpiEnabled and its
>> corresponding setter/getter to something like "reportLayoutSize" and
>> add a good javadoc for it so that it's clear what this boolean flag
>> does just by looking at its name.
>>
>> --
>> best regards,
>> Anthony
>>
>> On 1/21/2014 5:29 PM, Anton V. Tarasov wrote:
>>> Hi all,
>>>
>>> Let me please resume the review process.
>>>
>>> With the new webrev:
>>>
>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.3
>>>
>>> I'm addressing the last concern which was about "leaking" the internal
>>> OffscreenHiDPIImage to the public via RepaintManager.getOffscreenBuffer.
>>>
>>> The explanation will follow, but before that I'd like to share the info
>>> related to the volatile buffer performance issue (which I was talking
>>> about before). I did some investigations of the native side and figured
>>> out the real source of the performance drop. It's the code where
>>> glReadPixels is called to read _every_ scanline of the image. This is so
>>> because of the nature of the OGL coordinate space which is upside down
>>> comparing to the j2d space. Please find more details here:
>>> https://javafx-jira.kenai.com/browse/RT-30035?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=380146.
>>>
>>> If I'm not mistaken, we can do nothing about it.
>>>
>>> So, Swing/Interop can't use a volatile image as a back buffer, and it
>>> should use a buffered image or no back buffer at all. (Otherwise,
>>> performance is not acceptable).
>>>
>>> Now, to the fix. What I did is I added a "hidpiEnabled" property to the
>>> OffscreenHiDPIImage class. When it's "true" (by default) the image
>>> returns its size in layout space (just like in the previous version),
>>> when it's "false" the image returns its size in physical space (just
>>> like an ordinary BufferedImage). In RepaintManager I set the image
>>> "hidpi disabled", thus hiding its layout size from the developer . The
>>> property is taken into account in SunGraphics2D.isHiDPIImage(). Because
>>> an OffscreenHiDPIImage with hidpiEnabled==false is drawn as an ordinary
>>> image, I draw it via drawImage(img, x, y, width, height) in
>>> RepaintManager.
>>>
>>> Why I still use the OffscreenHiDPIImage class instead of a BufferedImage
>>> is because otherwise I'd have to do pretty the same coding, but it would
>>> be scattered. The class basically incapsulates the following logic:
>>>
>>> - Keeps layout size of the image. (Used in drawImage.)
>>> - Keeps the scale factor. (Used by
>>> SurfaceData/VolatileImage/GraphicsConfig.)
>>> - Overrides SurfaceData.getDefaultScale. The point is that I can't
>>> simply call Graphics2D.scale(s, s) as this won't work when Swing draws
>>> into the image. (SunGraphics2D asks SurfaceData for the scale).
>>> - Overrides VolatileImage to make it return a scaled BI as its backup.
>>> (Used by Nimbus.)
>>> - Overrides GraphicsConfiguration to let it access the BI and its scale
>>> factor. (Used by Nimbus. This could be implemented otherwise, but not
>>> much better).
>>>
>>> No more changes in the current version. I know there're some concerns
>>> related to detecting a device change, but let me address it after we
>>> come to agreement about the approach discussed above.
>>>
>>> Thanks,
>>> Anton.
>>>
>>>
>>> On 18.12.2013 5:03, Jim Graham wrote:
>>>> Hi Anton,
>>>>
>>>> javax.swing.RepaintManager.getOffscreenBuffer is a public method that
>>>> can now return one of the new HiDPI offscreen images which is a
>>>> subclass of BufferedImage.  This was what I was worried about in terms
>>>> of one of these internal double buffers leaking to developer code.  If
>>>> they test the image using instanceof they will see that it is a
>>>> BufferdImage and they may try to dig out the raster and get confused...
>>>>
>>>>             ...jim
>>>>
>>>> On 12/17/13 10:21 AM, Anton V. Tarasov wrote:
>>>>> Hi all,
>>>>>
>>>>> Please look at the new version:
>>>>>
>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.2
>>>>>
>>>>> It contains the following changes:
>>>>>
>>>>> - All the scale related stuff is moved to the new class:
>>>>> OffScreenHiDPIImage.java
>>>>>
>>>>> - JViewport and RepaintManager no longer cache buffers.
>>>>>
>>>>> - JLightweightFrame has new method: createHiDPIImage(w, h).
>>>>>
>>>>> - JViewport, RepaintManager and AbstractRegionPainter goes the new
>>>>> path
>>>>> to create a HiDPI buffered image.
>>>>>
>>>>> - A new internal property is added: "swing.jlf.hidpiImageEnabled".
>>>>> False
>>>>> by default. It makes JLF.createImage(w, h) forward the call to
>>>>> JLF.createHiDPIImage(w, h). This can be used by a third party code in
>>>>> case it creates a buffered image via Component.createImage(w, h) and
>>>>> uses the image so that it can benefit from being a HiDPI image on a
>>>>> Retina display.
>>>>>
>>>>> For instance, SwingSet2 has an animating Bezier curve demo. Switching
>>>>> the property on makes the curve auto scale smoothly. Please, look
>>>>> at the
>>>>> screenshots:
>>>>>
>>>>> -- http://cr.openjdk.java.net/~ant/JDK-8029455/RoughtCurve.png
>>>>> -- http://cr.openjdk.java.net/~ant/JDK-8029455/SmoothCurve.png
>>>>>
>>>>> - SunGraphics2D now draws a HiDPI buffered image the same way it
>>>>> draws a
>>>>> VolatileImage.
>>>>>
>>>>> - I've removed the copyArea() method from the BufImgSurfaceData, and
>>>>> modified the original version. The only question I have is: do I
>>>>> need to
>>>>> check for "instanceof OffScreenHiDPIImage.SurfaceData" in case when
>>>>> "transformState == TRANSFORM_TRANSLATESCALE"? If this method is
>>>>> invoked
>>>>> with some other SD, and the transform is SCALE, will it do the job
>>>>> with
>>>>> the coordinates conversion done?
>>>>>
>>>>> - I've left the new methods in FramePeer default... May yet we
>>>>> implement
>>>>> them in other peers when we really need it?
>>>>>
>>>>> - CPlatformLWWindow.getGraphicsDevice() checks for an intersection +
>>>>> scale. This heuristic actually may fail when a Window is moved b/w
>>>>> three
>>>>> or four displays so that it intersects them all at some time. JFX will
>>>>> set a new scale factor in between and AWT may pick up a wrong
>>>>> device. I
>>>>> don't know any simple solution for that. For two monitors this will
>>>>> work.
>>>>>
>>>>> Thanks,
>>>>> Anton.
>>>
>