Hi Sergey,
I checked after reverting all the fixes starting from 8001470 and I saw
that
although the tests like bug8001470.java, ScrollFlickerTest pass in
windows and mac, present test pass in windows but fails on mac,
parent bug bug8001470.java testcase fails on linux without the d.width
<= 0 || d.height <= 0 check
as I saw that Dimension d returns negative width(-10) and height (-15)
in JEditorPane.getPreferredSize() when bug8001470.java is run on linux.
The "rootViewNeedsLayout" flag causes this check to be not used and
still pass and I think this is needed.
Also, as told, the hidpi issue of the present test is caused by 8178025
<https://bugs.openjdk.java.net/browse/JDK-8178025>and should need some
SwingUtilities2.isScaleChanged check somewhere, which I intend to work
on in 8229222 <https://bugs.openjdk.java.net/browse/JDK-8229222>, if you
do not have any objection.
Regards
Prasanta
On 10-Aug-19 2:12 AM, Sergey Bylokhov wrote:
Hi, Prasanta.
Since this is the second regression of JDK-8130892, my suggestion was
to try to revert all of these fixes and check that such changes are
really necessary. Do we really need to change the check from
"d.width <= 0 || d.height <= 0" to "d.width <= 0 || d.height <= 0",
and do wee need a flag?
I do not suggest to re-implement the fix, but recheck that we will not
integrate on more regression.
----- [email protected] wrote:
>
Hi Sergey,
>
It seems the hidpi issue of the test is because of JDK-8178025
<https://bugs.openjdk.java.net/browse/JDK-8178025> where we calculate
the fontmetrics because of scaleChange
>
/jdk10-b33/bin/java JEditorPaneLayoutTest
> Exception in thread "main" java.lang.RuntimeException: Wrong size
java.awt.Dimension[width=180,height=10] expected
java.awt.Dimension[width=180,height=44]
> at JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:87)
>
>
jdk10-b34/bin/java JEditorPaneLayoutTest
> Exception in thread "main" java.lang.RuntimeException: Wrong size
java.awt.Dimension[width=181,height=10] expected
java.awt.Dimension[width=180,height=44]
> at JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:87)
>
Since all the regression tests for this issue like
>
javax/swing/plaf/basic/BasicTextUI/8001470/bug8001470.java,
javax/swing/JTextArea/ScrollbarFlicker/ScrollFlickerTest.java and the
present test
is passing with the fix and I have already created a JBS
issueJDK-8229222 <https://bugs.openjdk.java.net/browse/JDK-8229222>to
work on the test issue, which is not directly related to this fix, can
we check this in and I work on the JBS issue subsequently?
>
Regards
Prasanta
>
On 09-Aug-19 2:43 AM, Sergey Bylokhov wrote:
>
>
Hi, Prasanta.
>
I have one additional suggestion. I think we need to check the
whole related sequence of bugs
>
>
0) JDK-8001470 - The bug which is unrela6ed to the current bug,
but it is introduce the test from which we start the step (1)
>
1) Initial bug was reproduced on Solaris 11 only:
https://bugs.openjdk.java.net/browse/JDK-8130892
As a fix the check for "uninitialized" state was changed, but it
was caused next regression
>
>
2) https://bugs.openjdk.java.net/browse/JDK-8160246 , it also
reproduced only on Solaris and this is the fix where we added the
"rootViewInitialised" flag, but it cause next regression
>
2) Current bug https://bugs.openjdk.java.net/browse/JDK-8226513
>
>
And you already found by the test that it dose not take into
account the fact that the GConfig may be different when you
calculate the size for invisible component, and the size after it
is became visible, probably ithe fix itself does not take into
account this?
>
It is also quite interesting to check the root cause of the
JDK-8130892 :
https://bugs.openjdk.java.net/browse/JDK-8130892?focusedCommentId=13822701&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13822701
>
I doubt that negative size is a correct value for the component.
>
>
----- [email protected] wrote:
> >
OK +1 from me !
> >
> > -phil.
> >
> >
> > On 8/8/19 2:23 AM, Prasanta Sadhukhan wrote:
> >
> >
> > On 07-Aug-19 9:42 PM, Phil Race wrote:
> >
I have a minor quibble that "rootViewInitialized" is no
longer a very appropriate variable name,
> > since it is not just about initialization.
> > Can we rename it to "rootViewNeedsLayout" ?
> >
> >
Renamed.
> >
> > Also because the windows 10 issue has a different
cause, potentially this test can be adjusted
> > to allow some tolerance to tell the difference between
> > "not re-layed out at all", and "I'm a few pixels off
in my expectations".
> > ie the test should be about "did I relayout?", not "is
my preferred size exactly the actual size?".
> >
> > Could this be a hi-dpi issue ? Are you running at 96
dpi or something else when this fails ?
> >
> >
Yes, it is. I have modified the jtreg command line to test in
uiScale 1. It now pass in windows and others and fails without
the fix.
http://cr.openjdk.java.net/~psadhukhan/8226513/webrev.02/
Regards
> > Prasanta
> >
> > -phil.
> >
> >
> >
> > On 8/7/19 2:04 AM, Prasanta Sadhukhan wrote:
> >
I confirm that the test pass on linux and mac with the
fix and fail without the fix. Only on windows, it fails.
> >
But I also see that the failure is not because of the
fix. The "problem" was there even before the fix, for
example with jdk13 (fails with jdk11, jdk12 too )
> >
jdk13-b24/bin/java JEditorPaneLayoutTest
> > Exception in thread "main"
java.lang.RuntimeException: Wrong size
java.awt.Dimension[width=183,height=10] expected
java.awt.Dimension[width=177,height=44]
> > at
JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:84)
> >
which got somewhat improved by JDK-8217731
<https://bugs.openjdk.java.net/browse/JDK-8217731>
jdk13-b25/bin/java JEditorPaneLayoutTest
> > Exception in thread "main"
java.lang.RuntimeException: Wrong size
java.awt.Dimension[width=181,height=10] expected
java.awt.Dimension[width=180,height=44]
> > at
JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:84)
So, the 1 pixel difference in width was there even
before this fix and also this fix fixes the height of
the basic text component and the unit tests of JBS
works ok, so to me this fix looks ok to me.
+1 for the fix from me and
> >
I have added the test to problem list only for windows
to figure out the 1 pixel difference in width which
seems to be because of layouting in windows.
> >
http://cr.openjdk.java.net/~psadhukhan/8226513/webrev.01/
Regards
> > Prasanta
> >
> > On 22-Jul-19 4:01 PM, Prasanta Sadhukhan wrote:
> >
Hi Semyon,
> >
> > Although the JBS testcase is passing with your
change, your testcase is failing even with the fix
for me in windows10
> >
> > java.lang.RuntimeException: Wrong size
java.awt.Dimension[width=181,height=44] expected
java.awt.Dimension[width=180,height=44]
> > at
JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:84)
> >
> > Regards
> >
> > Prasanta
> >
> > On 17-Jul-19 1:33 AM,
[email protected] wrote:
> >
bug:
https://bugs.openjdk.java.net/browse/JDK-8226513
> >
> > webrev:
http://cr.openjdk.java.net/~ssadetsky/8226513/webrev.00/
> >
> > The fix adds resetting the root view
initialization flag when the text component
underling document is changed and also removes
the check for the zero component size for the
root view initialization to correct the
resulting preferred component size in some
scenarios when the root view need to be
initially layouted.
> >
> > --Semyon
> >
> >
> >
> >
> >
