Re: [14] RFR 8226513:, JEditorPane is shown with incorrect size

[Prasanta Sadhukhan]

Hi Sergey,

When frame.setVisible() is called we do
Frame.setVisible() -> BorderLayout.layoutContainer() where we do
if ((c=getChild(CENTER,ltr)) != null) {
            c.setBounds(left, top, right - left, bottom - top);
        }

Now, in linux we get
bottom = target.height - insets.bottom; 0-5=-5
and right = target.width - insets.right; 0-5=-5

right=-5, left=5, bottom=-5,top=25

so component's or JRootPane's bounds width=-10,height=-30

Now, when we call frame.pack(), we do GridLayout.preferredLayoutSize() 
where Component comp = parent.getComponent(i) returns JTextField as 
component with -10x-15 as width,height

Regards
Prasanta
On 12-Aug-19 2:25 PM, Sergey Bylokhov wrote:
Hi, Prasanta.

On 8/12/19 1:41 am, Prasanta Sadhukhan wrote:
as I saw that Dimension d returns negative width(-10) and height 
(-15) in JEditorPane.getPreferredSize() when bug8001470.java is run 
on linux.

But why this size is negative?


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.

----- prasanta.sadhuk...@oracle.com 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.
    >

    >
    ----- philip.r...@oracle.com 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, 
semyon.sadet...@oracle.com 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
                        > >
                        > >
                        > >

            > >

    > >