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

[Anton V. Tarasov]

On 12/13/13 2:05 AM, Anthony Petrov wrote:
On 12/12/2013 07:18 PM, Anton V. Tarasov wrote:
[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? IMO, this would simplify the code significantly as Swing is 
already HiDPI-aware. It only needs to be able to determine the scale 
factor of a screen device its top-level is on. Of course, the code in 
the JLF implementation needs to know that too, so that it's able to 
create a BI of appropriate dimensions. So making JLF tracking its 
current GraphicsConfiguration looks like a reasonable idea to me. As 
Sergey suggested, this could be done easily by checking intersections 
between JLF's bounds in screen coordinates and the bounds of all the 
available GraphicsDevices.

I'm not against the idea of using the right device internally (I just 
don't like the idea to add a notifyScreenChanged method). If this really 
can be implemented by means of the comparing the coordinates, then I'll 
do that.

Thanks,
Anton.

--
best regards,
Anthony




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.