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

[Jim Graham]

I'm starting to get the full picture now.  Certainly, if we could share 
bits via the FX/Swing bridge then we could avoid the BufferedImage in 
the middle.  There used to be support in Scenario for the "resource 
sharing" layer that Dmitri and Chris put into Java2D to grab texture IDs 
directly from Java2D.  Could that be re-leveraged more easily than 
fixing this bug?

One question about your non-double-buffered experiment below.  You said 
that it didn't have any noticeable affect on performance.  Is that "the 
same as using a VI and a VI->BI copy"?  Or is it "the same as using a BI 
the whole way"?  I didn't understand which other scenario you were 
comparing its performance to.

If we still find some value in using VI on the Swing side, then how does 
the performance of rendering VI->BI differ from a direct pixel readback 
on the VI?  I would hope that they were the same, but perhaps we are 
being stupid in the VI->BI copy loops.  If readback is faster, could we 
modify the bridge to do a more direct readback rather than a drawImage 
operation?  Have you traced the code to see where the performance is 
going when we use the current VI->BI copies?  Maybe we are just missing 
a more direct copy loop in our list of loops?

			...jim

On 12/19/13 4:52 AM, Anton V. Tarasov wrote:
> Hi Jim and all,
>
> Thinking of getting rid of the need to use BuffedImage at all for JLF,
> I'm facing the following obstacles:
>
> 1. We have to forse developers to use a buffered image as a double
> buffer (currently by means of setting the corresponding property),
> because of the performance issue I'd mentioned
> (https://javafx-jira.kenai.com/browse/RT-30035). I investigated it a
> little. My assumption was that the problem is in copying from a volatile
> image to a buffered image (which Swing performs to flush its double
> buffer to Graphics which is tight to the root BufferedImage, containing
> the pixels of the JLF'c content). And that's the case. Measurement
> showed that copying b/w volatile and buffered works up to 300 times
> slower than copying b/w buffered and buffered. (This is what I see on my
> Mac, where AWT uses OpenGL). It dropps performance drastically.
>
> As an alternative, we can switch off double buffering at all. I checked
> how it affects the performance (theoretically, it should increase it). I
> didn't see any noticable difference... (perhaps, that's because there're
> so much buffers b/w fx & swing, that one buffer less doesn't change the
> picture radically). But switching off double buffering changes the
> default behavior. Theoretically it can surprise the code that relies on it.
>
> 2. JViewport, in the interop mode, is forsedly switched to BACKINGSTORE
> (that is a BufferedImage). (By the way, here we also change the default
> behavior). This probably will be resolved in the future.
>
> 3. nimbus.AbstractRegionPainter takes a GraphicsConfig from the Graphics
> (which is tight to the BufferedImage, the roor buffer for JLF) and
> creates an Image based on it. As the Graphics is derived from BI, the
> result is BI as well. So far, I don't know if we can change this
> behavior for the AbstractRegionPainter.
>
> We could originally create a VolatileImage (the root image for JLF),
> however with a volatile buffer we will face the same performance issue
> when we extract pixels from it. The "unified rendering" (exchanging bits
> on GPU) is cool idea, but no one started it yet.
>
> On 19.12.2013 2:02, Jim Graham wrote:
>> Hi Anton,
>>
>> I don't know enough about the overall architecture yet to be too
>> specific about possible solutions at this point.  Here are some
>> questions that I still don't know the answer to...
>>
>> - I'm assuming that Swing gets its back buffer from the
>> getOffscreenBuffer call because that was what you modified to return a
>> HiDPI image.  When Swing calls it internally, does it ever leak that
>> instance?  Could it use a different API to get that back buffer so
>> that the public API doesn't change?
>
> I'm afraid it can't. It calls the public method to create a double
> buffer, so that a custom RepaintManager could override it for instance.
> However, here's what comments in the code says:
>
>      // Support for both the standard and volatile offscreen buffers
> exists to
>      // provide backwards compatibility for the [rare] programs which
> may be
>      // calling getOffScreenBuffer() and not expecting to get a
> VolatileImage.
>      // Swing internally is migrating to use *only* the volatile image
> buffer.
>
>      // Support for standard offscreen buffer
>      //
>      DoubleBufferInfo standardDoubleBuffer;
>
> "Rare programs" may be calling the method. Swing long ago migrated to
> volatile buffers, for which there's another public method:
> getVolatileOffscreenBuffer().  Also, the comments seem outdated saying
> that it supports both the standard and volatile buffers. Currently, it
> only supports standard offscreen buffer.
>
>>
>>
>> - The method returns Image.  If worse comes to worst then we could try
>> to hide the identity of the BI that gets returned, but that would be
>> an awkward wrapper object, so hopefully a different solution works.
>>
>> - Do we need to provide "automatically scaled image buffers" to
>> developers that request one?  I'm guessing that the standard practice
>> for DB in Swing is that the Swing code manages the image used for the
>> back buffer, but does the developer ever get involved in that process
>> such that we need to have magically scaled images in their hands?
>
> A custom RepaintManager can theoretically interfere in the process of
> double buffer management. It can draw some additional pixels to it or
> something of the like (I'll ask Alexander Potochkin, a former Swing
> owner, who should know much about it).
>
>>
>>
>> - My impression was that there is some code in Swing that allocates
>> the backbuffer, tells the Swing tree of JComponents to render into it,
>> then draws that image to the screen.  Logically it might look
>> something like this:
>>
>> backbuffer = createBackBuffer(w, h);
>> // ...
>> comp.paint(backbuffer.getGraphics());
>> // ...
>> screengraphics.drawImage(backbuffer, x, y, null);
>>
>> (though those lines of code may be spread across a number of methods
>> for all I know)
>>
>> Instead if it did this:
>>
>> backbuffer = createBackBuffer(w*scale, h*scale);
>> // backbuffer.getWidth,Height() return w*scale, h*scale, but we don't
>> care
>> // ...
>> Graphics g = backbuffer.getGraphics();
>> g.scale(scale, scale);
>> comp.paint(g);
>> // ...
>> // this call forces the logical size of the image without any special
>> // processing or instance recognition in SG2D
>> screengraphics.drawImage(backbuffer, x, y, w, h, null);
>>
>> then we don't need any fancy wrappers or anything.  This doesn't solve
>> any "manual double buffering" that a developer would do, though, but
>> it evades return values from public methods that have "mismatched
>> BufferedImage objects"...
>
> This is possible for RepaintManager and JViewport, but looks not direct
> for nimbus.AbstractRegionPainter where the moment of image creation is
> quite distant from its actual painting (the logic is spread across
> multiple calls)... But I can look at it deeper.
>
> Thanks,
> Anton.
>
>>
>>             ...jim
>>
>> On 12/18/13 1:25 AM, Anton V. Tarasov wrote:
>>> Hi Jim,
>>>
>>> Thanks for noticing (sorry, but I simply forgot to check we don't export
>>> the buffer...) What can we do about that? I have the following thoughts:
>>>
>>> 1) We can warn developers to be ready for a HiDPI raster when they call
>>> that method under the following conditions: 1) the interop mode, 2)
>>> MacOSX 3) a Retina display.
>>> 2) In case the method is called, we can create a "shadow buffer" and
>>> start to sync it with the main buffer. The main buffer will be scaled to
>>> the shadow buffer on every repaint cycle.
>>> 3) We can introduce an internal property which will switch on/off the
>>> 2nd scenario. For instance, a developer may ask for the buffer and don't
>>> bother about its hidpi raster.
>>>
>>> Yes, I understand the solutions are far from perfect, but please take
>>> into account, the interop is a special mode where we (and developers)
>>> should be ready for compromises...
>>>
>>> What do you think? Do you have any better solutions in mind?
>>>
>>> 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.
>>>
>