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

[Jim Graham]

The "inCreateCI" field is not MT-safe.  BufferedImage objects don't have 
any threading restrictions so a solution that is MT-safe is required 
there in the BufferedImageGraphicsConfig object.  I also think that any 
device-associated GraphicsConfig objects should be MT-safe in those 
methods as well - I'm not sure if we've ever documented otherwise, but 
those methods have tended to be harmless to call on any thread and 
fairly far removed from the more platform/screen intensive parts of the 
AWT that might assume thread safety.

In BufferedImage, the private fields and methods will require an 
accessor method for the inner class to access them.  Is there a reason 
they can't be left package-accessible?

In JViewport, is there a reason why a map of various scaled backing 
stores are kept rather than just validating the existing backing store 
against the new desired scale and replacing it when it changes?  Does 
the scale on the backing store ping-pong back and forth between scales much?

BufImgSurfaceData - you have to validate the transform as not flipping 
or rotating (even quadrant rotation will violate your conditions.  I 
believe that testing the transform type with a mask consisting of the 
TRANSLATE and SCALE_MASK flags should be fine.  Also, do we wan to set a 
precedent of allowing copyArea under an arbitrary scale, or should we 
test it to make sure it is specifically the same scale as the device 
scale factor?

SG2D - I'm worried about the performance impliciations of adding a new 
method in that creates and returns an array on every drawImage call. Did 
you do a performance test with J2DBench to verify the impact of these 
operations?

With the new support for scaled BufferedImages we now have a situation 
where some images return their logical dimensions from 
getWidth/Height(null) and some return physical dimensions.  That's a 
little dicey as the method now has two meanings depending on how the 
images were created.  We're sort of between a rock and a hard place in 
that BufferedImage objects have a historical context for how their 
values relate to the pixels you can access, but we also have a 
relationship between how the return values of getWidth/Height(observer) 
relate to the size the image is drawn on the screen.  Is this only ever 
done for the backing store objects?  Do we ever expose them to 
developers?  Could this be implemented by having the code that renders 
the backing store state its explicit on-screen size so that we don't 
need to add any computations to the use of getWidth/Height(null) to SG2D?

I'm going to take a break here and hit send to get these concepts out 
into the discussion before I look too much further...

			...jim

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