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

[Anton V. Tarasov]

Unfortunately, I have to suspend the request, as I'm taking a vacation 
for the rest of this year...

Thanks,
Anton.

On 12/10/13 6:22 PM, 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.
>
> 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. 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.
>