[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] <AWT Dev> [9] Review Request: JDK-8029455 JLightweightFrame: support scaled pai
From:       "Anton V. Tarasov" <anton.tarasov () oracle ! com>
Date:       2014-01-31 14:37:10
Message-ID: 52EBB516.70906 () oracle ! com
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread]