On 12/12/13 7:16 PM, Anton V. Tarasov wrote:
[cc'ing to j2d]
On 11.12.2013 14:38, Sergey Bylokhov wrote:
On 11.12.2013 13:18, Anton V. Tarasov wrote:
Hi Sergey,
On 11.12.2013 3:26, Sergey Bylokhov wrote:
Hi, Anton.
My expectation was that everything should work automatically, if
you get correct CGraphicsDevice for the embedded swing window +
some tweaks in the code related to the peer and CGLSurfaceData/
Why we need all this change in
CGraphicsDevice,CGLGraphicsConfig,OffScreenImage,CPlatformLWView,JViewport,RepaintManager.
With Nimus, at some moment, when the
nimbus.AbstractRegionPainter.paint(Graphics2D g, ...) method is
called it is passed the graphics instance created by
JLF.createGraphics() which derives it from the JLF's root buffered
image. Then, somewhere up the stack the method calls for
getImage(g.getDeviceConfiguration(),..),
Yes, correct. But you can created double sized surface for you
buffered image(in the same way as it was done for volatile) and
provide correct DeviceConfiguration for it.
Sergey,
It seems I didn't yet understand you. Could you please clarify? What
"double sized" surface do you mean for a BufferedImage, and what do
you mean by the "correct" DeviceConfiguration for it? (I can't put a
CGLSurfaceData into a BufferedImage).
When retina support was added to the volatile images, there were the
same problems with the mixing of "logical" and "real" sizes. And
volatile image(logical view) was not changed, but its surface was. When
the volatile image is created we check the native scale and use it to
create the surface, then in SG2D we take the difference between an image
and its surface into account.
Why we cannot do the same thing for OffscreenImage? We control when it
is created, we control when its used, offscrenimage is created for the
component-> we know what GraphicConfigs should be used for the scale
checking. Probably we can reuse/extract some code which was added to the
cglsurface?
Thanks,
Anton.
where the graphics conf is of BufferdeImageGraphicsConfig type. This
flows into GraphicsConfiguration.createCompatibleVolatileImage(int
w, int h, ...) and then into BufImgVolatileSurfaceManager which
doesn't support acceleration, and eventually into
BufferedImageGraphicsConfig.createCompatibleImage(int w, int h,
...). The [w, h] values passed here represent a logical size of a
Nimbus ui element. Unless I scale the size of the requested image
here, the ui element is shown stretched. That's why the changes in
BufferedImageGraphicsConfig.
With Aqua, I don't observe calls into BufImgVolatileSurfaceManager.
I suppose the reason is that a GraphicsConfiguration instance is
always taken from a component (from the JLF's hierarchy) which is
tight to a CGraphicsDevice and has a CGLGraphicsConfig, not
BufferdeImageGraphicsConfig. CGLVolatileSurfaceManager enables
acceleration, and so the process of a volatile image creation never
falls back to CLGGraphicsConfig.createCompatibleImage(...). By
"never" I mean that I didn't catch it with the tests I ran. However,
"find usages" shows that these methods are directly called from
CTrayIcon, CDragSourceContextPeer, CCustomCursor, at least. I rather
should cover those areas with some tests as well. Anyway, I've made
these methods (CLGGraphicsConfig.createCompatibleImage(...))
consistent with the
BufferedImageGraphicsConfig.createCompatibleImage(...) methods and
with the general idea I've described before.
Then, the other files you're referring to:
- CPlatformLWView
As I explained before, AWT in the lw embedding mode can't match a
window to the display it is showing on, simply because it doesn't
have a platform window underneath.
CPlatformView.nativeGetNSViewDisplayID(getAWTView()) returns zero,
and so CPlatformView.getGraphicsDevice() returns default device, not
necessarily matching the scale factor of the current device.
Why? You can try to check it youseft via
CGLGraphicsConfig.getBounds()+Peer.getBounds(); You need to override
CPlatformWindow.getGraphicsDevice() and return correct device. I see
that the similar bug exists in applets where
CPlatformEmbeddedFrame.getGraphicsDevice always return default device.
Same question about Component.getLocationOnScreen() is working in
SwingNode?
- 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.
- 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.
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.
One unrelated question. Did you try to use CALayer's embedding
mechanics? Probably it is possible to add CAlayer which is used by
the swing and awt to the FX CAlayer? In this case all problems
related to the painting goes away and it will be much faster, only
events should be generated(The same way our plugin works see
CPlatformEmbeddedFrame).
This is in plans (interop "unified rendering" for d3d, ogl). At
least, there are plans to investigate it....
I guess it could be simpler than the current solution, because
different layers will have different context which will be used by
the swing and fx independently.
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.
--
Best regards, Sergey.
