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

Tue Jan 28 15:36:50 UTC 2014

On 25.01.2014 5:48, Jim Graham wrote:
> 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...?

Ok, I'll see how it looks like.

Thanks,
Anton.

>
>             ...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.
>>>>
>>