[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 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.
