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

[Jim Graham]

Hi Anton,

I think I'm getting a little lost in all of the details that I'm only on 
the periphery of, but it sounds like if that had been the original 
performance we had seen then we might not have gone down the path of 
using/forcing SW/BufferedImage?

To be clear, this is embedding a Swing hierarchy into an FX scene?

I forget, is FX forced into SW mode when this is used?  Or are we 
reading out the pixels in Swing only to put them back into a texture 
when they get to FX?

I think I mentioned the RSL (Resource Sharing Layer) that Dmitri and 
Chris had created which allowed external libraries to get at the texture 
IDs used by Java2D hw pipes, and at one time we were using that to run 
FX hw acceleration, but I think once we went with our own toolkit 
(Glass), we had to own our own contexts and then I think RSL broke.  I 
wonder if we can reintroduce something similar here, or are we using 
completely different (and incompatible) contexts now between FX and J2D?

			...jim

On 1/28/14 7:35 AM, Anton V. Tarasov wrote:
> Hi Jim,
>
> What I have so far. I've implemented flipping of the image on the native
> side. I didn't find any means by wich GL allows me to flip an image once
> it has been rendered to a texture, unless I set up a transform matrix
> prior to the render (just like in J2D). So I did that with an SW image
> right after I fetched it from the texture.
>
> At first, this works just fine in terms of correctness of the picture I
> eventually see on the screen. At second, the perceived performance is
> quite not bad. So, the native flip appeared to work pretty fast.
>
> I tried to get some scores. In average, with volatile images it worked
> from 3 to 20 times slower than with buffered images. The worst result is
> with scrolling and this is perceptibly (when I scroll really fast up and
> down, I can see some delay, but it's subtle). All the other scenarios I
> tried (text rendering, 2D animations) performed visually really similar
> to the buffered case. At least, the results are far from what it was
> before, when it worked close to freeze. So, I would say it now looks
> acceptable from the first glance.
>
> What is interesting is that blitting a GL surface to an SW surface
> spends a good time around glReadPixels, up to 3 times greater. Probably,
> we can do something with it as well.
>
> Additionally, I ran the bouncing balls app. It works 20% faster with
> buffered images, but consumes 30% more CPU. So, in total, the volatile
> version wins here.
>
> And also. The issue with JViewPort (which is forced to a buffered image)
> still exists (however, I have a straightforward solution and looking for
> a better one). As well as the issue with Nimbus which creates buffers
> based on the topmost BufferedImage. (May I use a volatile image as the
> topmost buffer? Didn't try yet).
>
> My conclusion is that, with the improvemtn, at least, the user may
> consider the volatile mode as an alternative choice.
>
> What are your thoughts?
>
> Thanks,
> Anton.
>
> On 25.01.2014 5:50, Jim Graham wrote:
>> Hi Anton,
>>
>> I think the main question is how fast is it compared to forcing a
>> software buffer?  It may be slower than a straight read, but is that
>> slow enough that we need to use a sw buffer instead?
>>
>>             ...jim
>>
>> On 1/24/14 6:46 AM, Anton V. Tarasov wrote:
>>> Hi Jim,
>>>
>>> As I wrote in RT-30035, I tried that on the java side (actually, I tried
>>> to emulate the set of operations needed to turn the image over). This
>>> increased the perf by ~10 times, however this was still ~10 slower than
>>> a simple read. But, I think I can try the following as well:
>>>
>>> 1) to do the turn natively (not sure if it's much faster)
>>> 2) to look if OGL is able do the turn in vram.
>>>
>>> Thanks,
>>> Anton.
>>>
>>> On 24.01.2014 2:08, Jim Graham wrote:
>>>> Hi Anton,
>>>>
>>>> Could the upside-down nature of the pixel readback be solved by
>>>> reading the entire frame in a single operation and then swapping the
>>>> pixels around in our own code?  The memory movement may be faster than
>>>> the overhead of H calls to glReadPixels.  (In the long run, we might
>>>> want to teach the rest of our code how to deal with upside-down image
>>>> data as well and then we don't have to swap it...)
>>>>
>>>>             ...jim
>>>>
>>>> On 1/21/14 5:29 AM, 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.
>>>>>
>>>
>