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

[Anton V. Tarasov]

[cc'ing to j2d]

On 11.12.2013 14:38, Sergey Bylokhov wrote:
> On 11.12.2013 13:18, Anton V. Tarasov wrote:
>> Hi Sergey,
>>
>> On 11.12.2013 3:26, Sergey Bylokhov wrote:
>>> Hi, Anton.
>>> My expectation was that everything should work automatically, if you get correct CGraphicsDevice 
>>> for the embedded swing window + some tweaks in the code related to the peer and CGLSurfaceData/
>>>
>>> Why we need all this change in 
>>> CGraphicsDevice,CGLGraphicsConfig,OffScreenImage,CPlatformLWView,JViewport,RepaintManager.
>>
>> With Nimus, at some moment, when the nimbus.AbstractRegionPainter.paint(Graphics2D g, ...) method 
>> is called it is passed the graphics instance created by JLF.createGraphics() which derives it 
>> from the JLF's root buffered image. Then, somewhere up the stack the method calls for 
>> getImage(g.getDeviceConfiguration(),..),
> Yes, correct. But you can created double sized surface for you buffered image(in the same way as 
> it was done for volatile) and provide correct DeviceConfiguration for it.

Sergey,

It seems I didn't yet understand you. Could you please clarify? What "double sized" surface do you 
mean for a BufferedImage, and what do you mean by the "correct" DeviceConfiguration for it? (I can't 
put a CGLSurfaceData into a BufferedImage).

Thanks,
Anton.

>> where the graphics conf is of BufferdeImageGraphicsConfig type. This flows into 
>> GraphicsConfiguration.createCompatibleVolatileImage(int w, int h, ...) and then into 
>> BufImgVolatileSurfaceManager which doesn't support acceleration, and eventually into 
>> BufferedImageGraphicsConfig.createCompatibleImage(int w, int h, ...). The [w, h] values passed 
>> here represent a logical size of a Nimbus ui element. Unless I scale the size of the requested 
>> image here, the ui element is shown stretched. That's why the changes in 
>> BufferedImageGraphicsConfig.
>>
>> With Aqua, I don't observe calls into BufImgVolatileSurfaceManager. I suppose the reason is that 
>> a GraphicsConfiguration instance is always taken from a component (from the JLF's hierarchy) 
>> which is tight to a CGraphicsDevice and has a CGLGraphicsConfig, not BufferdeImageGraphicsConfig. 
>> CGLVolatileSurfaceManager enables acceleration, and so the process of a volatile image creation 
>> never falls back to CLGGraphicsConfig.createCompatibleImage(...). By "never" I mean that I didn't 
>> catch it with the tests I ran. However, "find usages" shows that these methods are directly 
>> called from CTrayIcon, CDragSourceContextPeer, CCustomCursor, at least. I rather should cover 
>> those areas with some tests as well. Anyway, I've made these methods 
>> (CLGGraphicsConfig.createCompatibleImage(...)) consistent with the 
>> BufferedImageGraphicsConfig.createCompatibleImage(...) methods and with the general idea I've 
>> described before.
>>
>> Then, the other files you're referring to:
>>
>> - CPlatformLWView
>>
>> As I explained before, AWT in the lw embedding mode can't match a window to the display it is 
>> showing on, simply because it doesn't have a platform window underneath. 
>> CPlatformView.nativeGetNSViewDisplayID(getAWTView()) returns zero, and so 
>> CPlatformView.getGraphicsDevice() returns default device, not necessarily matching the scale 
>> factor of the current device.
> Why? You can try to check it youseft via CGLGraphicsConfig.getBounds()+Peer.getBounds(); You need 
> to override CPlatformWindow.getGraphicsDevice() and return correct device.  I see that the similar 
> bug exists in applets where CPlatformEmbeddedFrame.getGraphicsDevice always return  default device.
> Same question about Component.getLocationOnScreen() is working in SwingNode?
>>
>> - 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.
>>
>> - 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.
>>
>>> 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.
>>
>>>
>>> One unrelated question. Did you try to use CALayer's embedding mechanics? Probably it is 
>>> possible to add CAlayer which is used by the swing and awt to the FX CAlayer? In this case all 
>>> problems related to the painting goes away and it will be much faster, only events should be 
>>> generated(The same way our plugin works see CPlatformEmbeddedFrame).
>>
>> This is in plans (interop "unified rendering" for d3d, ogl). At least, there are plans to 
>> investigate it....
> I guess it could be simpler than the current solution, because different layers will have 
> different context which will be used by the swing and fx independently.
>>
>> 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.
>>>>
>>>
>>>
>>
>
>