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

[Anton V. Tarasov]

[cc'ing to j2d]

On 11.12.2013 23:14, Anthony Petrov wrote:

Hi Anton,

On 12/11/2013 02:40 PM, Anton V. Tarasov wrote:

2. I'm not sure if adding the scale field to the BI is a good idea. I
think that the image shouldn't be aware of any scale. After all, it's
just an image, a bitmap. It has its real dimensions corresponding to
the actual size of the image stored in RAM. Whether this image is
going to be represented as a scaled image is something that a code
that uses the image should be concerned with, not the image itself.

There are two options. 1) Follow to your logic, that is to fix every
place in AWT/Swing where BufferedImage is created. In which case in
every such place we will have to get a current scale factor from the
context. 2) Don't touch that code, but solve the task in some generic
way, the way I tried to implement, when a buffered image is created with
an extended size right in the factory methods.

The logic of the first approach is simpler. However, it would require
lots of modifications (in the L&F code, and not only there). And it
would require to take into account the scale factor every time a new
buffered image case is added to the code. Still, this is possible.

With the scond approach I don't need to bother about that code, I just
create scaled images "on the fly".

 > I think that the image shouldn't be aware of any scale.

The "scale" field put into the BufferedImage class means that an image
instance should (or shouldn't) be treated as a HiDPI image by the
Graphics.drawImage(). So, this is a kind of a special "scale" case,
aimed at supporting Retina technology. Probably it deserves a better
name, "hidpiScale" or something. So, it's not that the image has been
scaled by the user to be drawn on a lager area, but that the image
should (or shouldn't) be scaled just to "look smoothly" on a Retina
display. That's what I was thinking about...

Thanks for the clarification. The idea sounds reasonable indeed.

I've also read your reply to Jim and I'm now concerned about the fact that 
scaled images can
report larger dimensions than a Graphics created for them would allow one to 
draw to them. This
may be a problem for some apps that perform rendering based on the dimensions 
of a BI object
passed to them.

Shouldn't the BI.getWidth/Height() methods always report the logical size of 
the image, which is
equal to the physical one for scale == 1?

That's probably worth discussing. Let me move this discussion to another thread.

Thanks,
Anton.

--
best regards,
Anthony

3. src/share/classes/java/awt/peer/FramePeer.java

 139     default void notifyScaleFactorChanged() {}

I think this deserves to be declared in WindowPeer so that we could
use it w/o additional modifications in the future if we add support
for public notifications about the scale factor changes.

Ok.

4. I'm CC'ing swing-dev@ and Alexander Scherbatiy to review changes in
the JViewport class and other Swing classes.

Thanks for the review!

Anton.

--
best regards,
Anthony

On 12/10/2013 06: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.