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

[Anton V. Tarasov]

[cc'ing to j2d alias]

On 11.12.2013 21:29, Sergey Bylokhov wrote:
> On 11.12.2013 20:23, Anton V. Tarasov wrote:
>>
>>>> - CGraphicsDevice
>>>>
>>>> This setter is only called from CPlatformLWView.getGraphicsDevice(). I've explained it in my 
>>>> previous message. It's needed to change the scale factor of the default device when no device 
>>>> in the list fits. The case is impossible with the current implementation of SwingNode (which 
>>>> only passes JLF a scale factor matching one of a real display), however, as JLF provides a 
>>>> generic lw embedding API, I should cover that case as well.
> Not sure that matching fx to awt devices via scale is not a good idea. Note that it is expected 
> that fields in the CGraphicsDevice chenges only if the screen is changed/added/removed.
> Probably Instead of notifyScaleFactorChanged you can notify about screens changes?

JLightweightFrame is a toplevel that paints its content to an off-screen buffer, so it is 
conceptually not associated with any screen. The host (SwingNode) application communicates with JLF 
on an API level. Introducing a notion of a "screen" to the API doesn't correlate with the JLF's 
concept, imho.

Why I'm still picking the device is because this seemed to me an acceptable approach that integrates 
with LWAWT smoothly. But I agree with you that matching the device via a scale factor is not a good 
idea. Theoretically I can pickup wrong device, but even then it won't change anything for me. I just 
need a device with the requested scale.

What do you think then if we always use default device, for which we will change the scale?

>
>>>>
>>>> - OffScreenImage
>>>>
>>>> I've put a BufferedImage accessor there, nothing else. I didn't find a better place... (I'd 
>>>> appreciate showing it).
>>>>
>>>> - JViewport, RepaintManager
>>>>
>>>> These classes create a double buffer. In case the buffer is backed by a BufferedImage, it will 
>>>> be created with the current scale factor set. The buffer won't be changed when a user moves the 
>>>> host window across multiple screens with different scales. I see two options. 1) Drop the 
>>>> double buffer reference every time the scale changes (in that case, the buffer will be 
>>>> recreated every time, I cross a screen) 2) Create a map which will cache the buffers (say, for 
>>>> 1 and 2 scale factors for double screen env). I think the second approach is better.
>>>>
>>>> > Probably it will be better to disable doublebuffering and SwingPaintEventDispatcher 
>>>> completely(see swing.showFromDoubleBuffer)?
>>>>
>>>> Why? If we can manage it for JLF/SwingNode, why should we downgrade performance?
>>> You have 1 buffere on fx side, buffer in SwingNode, buffer in jviewport, and swing itself use 
>>> double buffering.
>>
>> Ok, this is a good point. But still I can't simply switch off double buffering w/o doing any 
>> benchmarking. SwingNode perf analisys & improvement is in plans...
> It would be good to know results of the benchmarks.

Ok, but as this is a separate task I'd like to know what we're fighting for. Is the goal to avoid 
creating BufferedImage's at all?

>> So far, unless it requires lots of coding (but it doesn't) I'd prefer to keep that option working.
>>
>>>>
>>>>> Actually I still do not understand why JViewport works in the standalone application.
>>>>
>>>> Could you please clarify, I don't understand this question...
>>> I see that JViewport use Offscreen image as a double buffer, is that true that it use it in the 
>>> standalone swing application? If yes why it works.
>>
>> JViewport.paint() is not called with its default blit mode, and so it doesn't actually use an 
>> OffscreenBuffer. For JLF, the mode is set to backing store. If I set the backing store mode in 
>> standalone swing, JViewport stops scaling on Retina. So, it didn't work before.
> Is this related to the JDK-8023966?

Right.

Thanks,
Anton.

>>
>>
>> Thanks,
>> Anton.
>>
>>>
>>>>
>>>> Thanks for the review!
>>>>
>>>> Anton.
>>>>
>>>>>
>>>>> On 10.12.2013 18:22, Anton V. Tarasov wrote:
>>>>>> Hi Jim, Sergey and All,
>>>>>>
>>>>>> Please review the fix that adds support of Retina displays to JLightweightFrame (which javafx 
>>>>>> SwingNode is based on).
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1
>>>>>> jira: https://bugs.openjdk.java.net/browse/JDK-8029455
>>>>>>
>>>>>> (After the fix goes into jdk9 it should be ported to 8u20 as well, because the functionality 
>>>>>> is essential for SwingNode.)
>>>>>>
>>>>>> The general idea of the fix is as follows.
>>>>>>
>>>>>> A BufferedImage instance, being created in the context in which the scale factor is 
>>>>>> determined and is different from one, is automatically created with appropriately extended 
>>>>>> size. The image itself becomes a scaled image (a "scale" private field is set on it). By the 
>>>>>> "context" I mean the circumstances where the BufferedImage is related to a JLightweightFrame, 
>>>>>> a GraphicsConfiguration, a SurfaceData, or a GraphicsDevice which determine the scale factor.
>>>>>>
>>>>>> Here are the related changes:
>>>>>>
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/share/classes/java/awt/image/BufferedImage.java.udiff.html
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/share/classes/sun/awt/image/OffScreenImage.java.udiff.html
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/share/classes/sun/swing/JLightweightFrame.java.udiff.html 
>>>>>> (the resizeBuffer method)
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/macosx/classes/sun/lwawt/LWLightweightFramePeer.java.udiff.html
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/share/classes/sun/awt/image/BufferedImageGraphicsConfig.java.udiff.html
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/macosx/classes/sun/java2d/opengl/CGLGraphicsConfig.java.udiff.html
>>>>>>
>>>>>> The "scale" value of a BufferedImage is used when 1) BufferedImageGraphicsConfig is created 
>>>>>> 2) BufImgSurfaceData.getDefaultScale() is called:
>>>>>>
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/share/classes/sun/awt/image/BufferedImageGraphicsConfig.java.udiff.html
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/share/classes/sun/awt/image/BufImgSurfaceData.java.udiff.html
>>>>>>
>>>>>> The former is used in the GraphicsConfiguration.createCompatibleImage() calls, and the latter 
>>>>>> is used in SurfaceManager.getImageScale(Image):
>>>>>>
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/share/classes/sun/awt/image/SurfaceManager.java.udiff.html
>>>>>>
>>>>>> A scaled BufferedImage is supported by the SunGraphics2D.drawImage() primitives. Here's the 
>>>>>> pattern of how the image may be created and drawn:
>>>>>>
>>>>>> int scale = <get the scale factor from the context>;
>>>>>> BufferedImage img = new BufferedImage(width * scale, height * scale, ...);
>>>>>> img.setScale(scale); // an accessor is currently used instead
>>>>>> <...>
>>>>>> g2d.drawImage(img, x, y, ...); // 1) draw the image with auto-scale
>>>>>> g2d.drawImage(img, x, y, dw, dh, ...) // 2) draw the image into a specified rect
>>>>>>
>>>>>> In the first case, if the BufferedImage is created with an extended size, the "scale" value 
>>>>>> of the image matters, it should be drawn as a HiDPI image.
>>>>>> In the second case, if the BufferedImage is created with an extended size, the "scale" value 
>>>>>> of the image doesn't matter (it may not be evidently set) as the image will anyway be scaled 
>>>>>> from its physical bounds into provided logical bounds. This all should (as I suppose) provide 
>>>>>> backward compatibility for buffered images that were created in their logical bounds or 
>>>>>> without setting the "scale" field. For instance, the 
>>>>>> AquaPainter.paintFromSingleCachedImage(...) method creates & draws an image as follows:
>>>>>>
>>>>>> int scale = ((SunGraphics2D) g).surfaceData.getDefaultScale();
>>>>>> int imgW = bounds.width * scale;
>>>>>> int imgH = bounds.height * scale;
>>>>>> BufferedImage img = new BufferedImage(imgW, imgH, ...);
>>>>>> <paint into the img>
>>>>>> g.drawImage(img, bounds.x, bounds.y, bounds.width, bounds.height, null);
>>>>>>
>>>>>> Here, the img.scale value is not set (I didn't modify this code), and SunGraphics2D doesn't 
>>>>>> treat the image as a HiDPI image, however it is drawn as expected. An alternative way to draw 
>>>>>> the image would be:
>>>>>>
>>>>>> int scale = ((SunGraphics2D) g).surfaceData.getDefaultScale();
>>>>>> int imgW = bounds.width * scale;
>>>>>> int imgH = bounds.height * scale;
>>>>>> BufferedImage img = new BufferedImage(imgW, imgH, ...);
>>>>>> img.setScale(scale);
>>>>>> <paint into the img>
>>>>>> g.drawImage(img, bounds.x, bounds.y, ...);
>>>>>>
>>>>>> The result would be the same.
>>>>>>
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/share/classes/sun/java2d/SunGraphics2D.java.sdiff.html
>>>>>>
>>>>>> The following changes:
>>>>>>
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/macosx/classes/sun/lwawt/macosx/CPlatformLWView.java.udiff.html
>>>>>>
>>>>>> are defined by this logic. Running Swing via JLightweightFrame (JLF) makes it "display 
>>>>>> agnostic". Swing is painted to an off-screen buffer and it's the host (e.g. SwingNode) that 
>>>>>> renders the buffer on a particular device. So, the host should detect the scale of the 
>>>>>> current display and set it on JLF.
>>>>> Does it mean that all methods related to the Component.getLocationOnScreen() does not work?
>>>>>>
>>>>>> However, AWT in order to paint to a volatile image requires CGraphicsDevice and 
>>>>>> CGLSurfaceData to be created. By default AWT creates CGraphicsDevice instances matching all 
>>>>>> the detected display devices (CGraphicsEnvironment.initDevices()). But, as JLF doesn't have 
>>>>>> any platform window behind it, AWT can't match JLF to the exact device it's currently 
>>>>>> displayed on.
>>>>> Why? You can try to check it youseft via CGLGraphicsConfig.getBounds()+Peer.getBounds();
>>>>>> So, on the one hand, AWT doesn't know which device is current and what is the current scale 
>>>>>> (the host passes this value), but from the other hand, AWT has a list of all the 
>>>>>> CGraphicsDevice instances.
>>>>>>
>>>>>> I tried to leverage from that fact. The CPlatformLWView.getGraphicsDevice() method takes the 
>>>>>> current scale from the JLF instance, and then tries to match it to an existent device from 
>>>>>> the list. In case it can't find a device with the specified scale (which should not actually 
>>>>>> happen, unless the host passes an arbitrary scale value, which is not the case for SwingNode) 
>>>>>> it takes a default device and changes its scale forcedly. I'm not sure if I should create a 
>>>>>> new dummy device instance instead. The scale factor of the device (which is then propagated 
>>>>>> to CGLSurfaceData on its creation) is the only info that JLF will take from the device to 
>>>>>> create a scaled volatile image.
>>>>>>
>>>>>> The following changes:
>>>>>>
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/share/classes/javax/swing/JViewport.java.udiff.html
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/share/classes/javax/swing/RepaintManager.java.udiff.html
>>>>>>
>>>>>> were made to map a backing store image to a scale factor.
>>>>>>
>>>>>> The JViewPort.paint(...) method calls SunGraphics2D.copyArea(...) on scrolling. The method 
>>>>>> was not implemented for a graphics with a scale transform and a BufImgSurfaceData (it threw 
>>>>>> exceptions). I took that code, copied it to the BufImgSurfaceData.copyArea(...) and added a 
>>>>>> general translation for the coords:
>>>>>>
>>>>>> - 
>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.1/src/share/classes/sun/awt/image/BufImgSurfaceData.java.udiff.html
>>>>>>
>>>>>> It works, but I'm not sure the implementation is eligible (I don't know the details of the 
>>>>>> Blit class, at least it warns not to use the same source and dest).
>>>>>>
>>>>>> The rest of the changes (not covered here) should be clear.
>>>>>>
>>>>>> Testing:
>>>>>>
>>>>>> - Using jfc/SwingSet2 and jfc/Java2D demos (in a standalone mode & embedded into SwingNode [1]).
>>>>>> - Testing both Nimbus and Aqua L&F.
>>>>>> - Setting swing.volatileImageBufferEnabled=false/true for all combinations.
>>>>>>
>>>>>> Currently, I see no regressions and no visual issues comparing a standalone mode and a 
>>>>>> SwingSet mode.
>>>>>>
>>>>>> At the end, I suspect there may be some intersection b/w this fix and the fix which 
>>>>>> introduced MultiResolutionToolkitImage. Unfortunately, I didn't yet read that review saga... 
>>>>>> Please tell me if I should incorporate anything from that fix.
>>>>>>
>>>>>> Thanks,
>>>>>> Anton.
>>>>>>
>>>>>> [1] There's a SwingSet part of the fix which I'm going to post to the jfx alias separately.
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>