Hi, Phil.
The new webrev:
http://cr.openjdk.java.net/~serb/8201552/webrev.03/
I have checked why validation of the component is necessary, when the
"ancestor" is changed (I removed it, but the new validation on GC change
null->non-null is equivalent).
Such validation is used as a last chance to update the state of the
component before show it to the user. In theory it is not necessary but
some components skips validation on font change(like jtree) and maybe
other properties. I have files a separate bug for that:
https://bugs.openjdk.java.net/browse/JDK-8206024
- In the fix I have moved these checks to SwingU2.
- TODO block for JSpinner: is removed because this bug was fixed
https://bugs.openjdk.java.net/browse/JDK-8205144
On 22/06/2018 16:54, Philip Race wrote:
It is not really the GC .. there is no GC .. but because of that the
code uses a default FontRenderContext.
I think Swing is conflating GC + FRC but given that, then the logic
to recalculate based on graphicsconfiguration should be fine, although
maybe you can check if we really need to trigger it ..
if the screen we are shown on has no scale (ie uiScale == 1) then
we may be triggering a pointless invalidation with consequent overhead.
Can you check if we can skip it in some cases ?
-phil.
On 6/22/18, 4:16 PM, Sergey Bylokhov wrote:
The place where the current graphicsConfiguration of the component is
matter:
- JComponent.getFontMetrics(Font)
-- SwingUtilities2.getFontMetrics(this, font)
--- SwingUtilities2.getFRCProperty(JComponent c)
if (c != null) {
GraphicsConfiguration gc = c.getGraphicsConfiguration();
AffineTransform tx = (gc == null) ? null :
gc.getDefaultTransform();
Object aaHint = c.getClientProperty(KEY_TEXT_ANTIALIASING);
return getFRCFromCache(tx, aaHint);
}
When the bug is reproduced we calculated the size of the tree based on
one GC and draw it using another GC. The first GC which is used for
size calculation may be null or it maybe a non-null value pointed to
another screen, for example if the jtree was shown on one screen and
then dragged to another.
On 22/06/2018 14:40, Sergey Bylokhov wrote:
In the SwingSet2 the bug is visible only if the user switches the L&F
immediately before switching to the tree demo. And is not reproduced
if the user switches to the tree demo and then change the L&F.
- If the tree is not visible then it will calculate the size of the
text based on some default GC, and will not update this size when it
will be added to the frame(which has another actual GC). So the tree
will use the size calculated using one GC, and will draw the text
using another GC. If the user will switch L&F the size will be
recalculated using correct GraphicsConfiguration.
-phil.
On 06/22/2018 01:48 PM, Sergey Bylokhov wrote:
Any volunteers for review?
=)
On 17/06/2018 15:37, Sergey Bylokhov wrote:
Unfortunately after additional testing I found a bug in our text
related components. In the JTextPane the text looks broken if we
request some change in the component after it is became visible.
For example if we change the font then the text will be
overlapping. So if I will be applied this fix, which will force
text component to relayout(because of the change in graphics
config), then the text will be broken from the beginning.
But before the fix it will be broken only if the application will
change the pane after it became visible(BTW text rendering during
editing is broken in both cases).
So I temporary reverted the changes in the text related components:
http://cr.openjdk.java.net/~serb/8201552/webrev.02
Two follow bugs were created:
- Text components: https://bugs.openjdk.java.net/browse/JDK-8205143
- JSpinner: https://bugs.openjdk.java.net/browse/JDK-8205144
On 15/06/2018 23:31, Sergey Bylokhov wrote:
Hello.
Please review the fix for jdk11.
Bug: https://bugs.openjdk.java.net/browse/JDK-8201552
Webrev: http://cr.openjdk.java.net/~serb/8201552/webrev.01/
Short description:
This fix enhance implementation of JDK-8178025[1] for most of our
Swing components. The main rule which I tried to follow:
"If layout of the component depends from the font then it should
depend on the current graphics configuration as well, because
FontMetrics depends on graphics configuration".
Long description:
The fix for JDK-8178025 added a special property
"graphicsConfiguration" which: fired when the
graphicsConfiguration is changed from one non-null value to
another non-null value.
Those fix also updated some of the components(to
refresh/re-validate its states when the "graphicsConfiguration"
or "ancestor" were changed).
The usage of "ancestor" was not obvious, so I modify the code to
fire "graphicsConfiguration" every time, this cover a situation
when the "GC=null" is changed to "GC=non-null"(previously it was
covered by "ancestor" property). So after this fix our components
will listen only "font" and "graphicsConfiguration".
In implementation of JDK-8178025 the "graphicsConfiguration" is
fired immediately after GC is changed, it caused the issues in
some containers like JTree. When the container get such
notification it usually tries to get some information from
children, but in this moment children had previous graphic
config, so the result calculated(and usually cached) in the
container was wrong. In this fix I changed implementation of this
property. Now it will fired only when the container and all its
children are updated.
===
Note that the new test StalePreferredSize.java has a TODO block.
Because JSpinner does not work properly even after the current
fix. The reason is that during validation it is unexpectedly
change the font from normal to bold(I'll fix this in a separate bug)
[1] https://bugs.openjdk.java.net/browse/JDK-8178025
--
Best regards, Sergey.
