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

[Anton V. Tarasov]

Hi Jim,

On 31.01.2014 2:32, Jim Graham wrote:
> 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?
>

This is a good point. However Sergey mentioned an issue in Netbeans, where switching (for some 
reason) a JViewport to a backingstore (a buffered image back buffer) mode leads to a drastic 
performance drop on OSX. I didn't yet have chances to get into the details, but this looks just like 
the issue in question.

Anyway, in the 2D code there's an obvious bottleneck - reading a texture into SW surface with 
glReadPixels by scanlines.

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

Right.

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

It isn't forced. Yes, we put back SW pixels to a texture on the fx side. This is the original, 
probably straightforward & simple, design of the swing/fx interop.

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

Unfortunately, I can't answer this question now. As I already wrote, we have in mind the "unified 
rendering" project, which is aimed at exactly this - exchanging pixels on the gl/d3d level.

But so far we have the sw-based interop. And I'd like to undestand what I'm doing with the sw-based 
Retina support. The fix is ready. It has a couple of concerns, 1) your last suggestion to migrate 
the "scale" logic from OffscreenHiDPIImage to OffscreenImage (I'm working on it) 2) a correction to 
the device detection logic is required.

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?

Regards,
Anton.

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