Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Krishna Addepalli
Hi Sergey,

As per your recommendation I have done the following changes:
1. Moved the fix to BasicMenuUI.
2. Corrected the test to:
        a. Access the UI elements only in the EDT.
        b. Made sure that the main thread doesn't exit till the test is complete.

Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/

However, I was wondering, why it is good to fix in BasicMenuUI? Also, while writing the testcase, I couldnot find any functionality like "flush" on the event queue, which will execute all the pending events. Is it by design?

Thanks,
Krishna

-----Original Message-----
From: Sergey Bylokhov
Sent: Friday, November 3, 2017 11:24 PM
To: Krishna Addepalli <[hidden email]>; [hidden email]
Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickets when label text shows "..." and is updated

Hi, Krishna.
A few notes:
  - Looks like the bug can be reproduced in JMenu only? Then it will be good to fix it in BasicMenuUI, moreover it is already implemented a
getMaximumSize() in a similar way as in your fix.
  - In the test some of the Swing components are accessed on non-EDT thread. Also note that when you start the Thread you will exit
invokeAndWait() and it is possible that the test will end before the Thread complete(jtreg will kill the test when the main thread is completed)

On 31/10/2017 05:00, Krishna Addepalli wrote:

> Hi Sergey,
>
> Thanks for pointing that out. Fixed the test case and created a new
> webrev here: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>
> Now, the testcase throws an exception and also after a few trials, closes on its own.
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Tuesday, October 31, 2017 1:13 AM
> To: Krishna Addepalli <[hidden email]>;
> [hidden email]
> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
> flickets when label text shows "..." and is updated
>
> Hi, Krishna.
> Can you please clarify in what situation the test will stop working(it does not have any assertions)?
>
> On 27/10/2017 02:45, Krishna Addepalli wrote:
>> Hi All,
>>
>> Please review the fix for bug:
>>
>>
>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>
>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>
>> Summary:
>>
>> The issue is when the label text width is more than the
>> container(JPanel) width, the container tries to render with minimum
>> width for all the components. In such case, the JMenuItem, which is
>> added to the JMenuBar also returns its height dimension as 1 (the
>> default minimum). The test case alternates between the short text and
>> long text on the label, and it gives a flickering effect of the Menu.
>> The fix is to return preferred size from JMenuItem, if its parent is
>> a JMenuBar, since JMenuBar is added to the top level window.
>>
>> Thanks,
>>
>> Krishna
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Krishna Addepalli
Hi Sergey,

Are the changes fine now?

Thanks,
Krishna

-----Original Message-----
From: Krishna Addepalli
Sent: Monday, November 6, 2017 9:02 PM
To: Sergey Bylokhov <[hidden email]>; [hidden email]
Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Hi Sergey,

As per your recommendation I have done the following changes:
1. Moved the fix to BasicMenuUI.
2. Corrected the test to:
        a. Access the UI elements only in the EDT.
        b. Made sure that the main thread doesn't exit till the test is complete.

Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/

However, I was wondering, why it is good to fix in BasicMenuUI? Also, while writing the testcase, I couldnot find any functionality like "flush" on the event queue, which will execute all the pending events. Is it by design?

Thanks,
Krishna

-----Original Message-----
From: Sergey Bylokhov
Sent: Friday, November 3, 2017 11:24 PM
To: Krishna Addepalli <[hidden email]>; [hidden email]
Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickets when label text shows "..." and is updated

Hi, Krishna.
A few notes:
  - Looks like the bug can be reproduced in JMenu only? Then it will be good to fix it in BasicMenuUI, moreover it is already implemented a
getMaximumSize() in a similar way as in your fix.
  - In the test some of the Swing components are accessed on non-EDT thread. Also note that when you start the Thread you will exit
invokeAndWait() and it is possible that the test will end before the Thread complete(jtreg will kill the test when the main thread is completed)

On 31/10/2017 05:00, Krishna Addepalli wrote:

> Hi Sergey,
>
> Thanks for pointing that out. Fixed the test case and created a new
> webrev here: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>
> Now, the testcase throws an exception and also after a few trials, closes on its own.
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Tuesday, October 31, 2017 1:13 AM
> To: Krishna Addepalli <[hidden email]>;
> [hidden email]
> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
> flickets when label text shows "..." and is updated
>
> Hi, Krishna.
> Can you please clarify in what situation the test will stop working(it does not have any assertions)?
>
> On 27/10/2017 02:45, Krishna Addepalli wrote:
>> Hi All,
>>
>> Please review the fix for bug:
>>
>>
>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>
>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>
>> Summary:
>>
>> The issue is when the label text width is more than the
>> container(JPanel) width, the container tries to render with minimum
>> width for all the components. In such case, the JMenuItem, which is
>> added to the JMenuBar also returns its height dimension as 1 (the
>> default minimum). The test case alternates between the short text and
>> long text on the label, and it gives a flickering effect of the Menu.
>> The fix is to return preferred size from JMenuItem, if its parent is
>> a JMenuBar, since JMenuBar is added to the top level window.
>>
>> Thanks,
>>
>> Krishna
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Krishna Addepalli
Hi Sergey,

Gentle reminder! Could you review the fix?

Thanks,
Krishna

-----Original Message-----
From: Krishna Addepalli
Sent: Wednesday, November 8, 2017 8:22 PM
To: Sergey Bylokhov <[hidden email]>; [hidden email]
Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Hi Sergey,

Are the changes fine now?

Thanks,
Krishna

-----Original Message-----
From: Krishna Addepalli
Sent: Monday, November 6, 2017 9:02 PM
To: Sergey Bylokhov <[hidden email]>; [hidden email]
Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Hi Sergey,

As per your recommendation I have done the following changes:
1. Moved the fix to BasicMenuUI.
2. Corrected the test to:
        a. Access the UI elements only in the EDT.
        b. Made sure that the main thread doesn't exit till the test is complete.

Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/

However, I was wondering, why it is good to fix in BasicMenuUI? Also, while writing the testcase, I couldnot find any functionality like "flush" on the event queue, which will execute all the pending events. Is it by design?

Thanks,
Krishna

-----Original Message-----
From: Sergey Bylokhov
Sent: Friday, November 3, 2017 11:24 PM
To: Krishna Addepalli <[hidden email]>; [hidden email]
Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickets when label text shows "..." and is updated

Hi, Krishna.
A few notes:
  - Looks like the bug can be reproduced in JMenu only? Then it will be good to fix it in BasicMenuUI, moreover it is already implemented a
getMaximumSize() in a similar way as in your fix.
  - In the test some of the Swing components are accessed on non-EDT thread. Also note that when you start the Thread you will exit
invokeAndWait() and it is possible that the test will end before the Thread complete(jtreg will kill the test when the main thread is completed)

On 31/10/2017 05:00, Krishna Addepalli wrote:

> Hi Sergey,
>
> Thanks for pointing that out. Fixed the test case and created a new
> webrev here: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>
> Now, the testcase throws an exception and also after a few trials, closes on its own.
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Tuesday, October 31, 2017 1:13 AM
> To: Krishna Addepalli <[hidden email]>;
> [hidden email]
> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
> flickets when label text shows "..." and is updated
>
> Hi, Krishna.
> Can you please clarify in what situation the test will stop working(it does not have any assertions)?
>
> On 27/10/2017 02:45, Krishna Addepalli wrote:
>> Hi All,
>>
>> Please review the fix for bug:
>>
>>
>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>
>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>
>> Summary:
>>
>> The issue is when the label text width is more than the
>> container(JPanel) width, the container tries to render with minimum
>> width for all the components. In such case, the JMenuItem, which is
>> added to the JMenuBar also returns its height dimension as 1 (the
>> default minimum). The test case alternates between the short text and
>> long text on the label, and it gives a flickering effect of the Menu.
>> The fix is to return preferred size from JMenuItem, if its parent is
>> a JMenuBar, since JMenuBar is added to the top level window.
>>
>> Thanks,
>>
>> Krishna
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Sergey Bylokhov
Hi, Krishna.
Small comments: I think that this code
"(JMenu)menuItem).isTopLevelMenu() == true"
can be simplified to
(JMenu)menuItem).isTopLevelMenu().

In the test you create and dispose JFrame and other components outside
of EDT thread. I suggest to use invokeAndWait() which will work
synchronously. Instead of sleep() you van use Robot.waitForIdle();


On 12/11/2017 22:45, Krishna Addepalli wrote:

> Hi Sergey,
>
> Gentle reminder! Could you review the fix?
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Krishna Addepalli
> Sent: Wednesday, November 8, 2017 8:22 PM
> To: Sergey Bylokhov <[hidden email]>; [hidden email]
> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated
>
> Hi Sergey,
>
> Are the changes fine now?
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Krishna Addepalli
> Sent: Monday, November 6, 2017 9:02 PM
> To: Sergey Bylokhov <[hidden email]>; [hidden email]
> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated
>
> Hi Sergey,
>
> As per your recommendation I have done the following changes:
> 1. Moved the fix to BasicMenuUI.
> 2. Corrected the test to:
> a. Access the UI elements only in the EDT.
> b. Made sure that the main thread doesn't exit till the test is complete.
>
> Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>
> However, I was wondering, why it is good to fix in BasicMenuUI? Also, while writing the testcase, I couldnot find any functionality like "flush" on the event queue, which will execute all the pending events. Is it by design?
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Friday, November 3, 2017 11:24 PM
> To: Krishna Addepalli <[hidden email]>; [hidden email]
> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickets when label text shows "..." and is updated
>
> Hi, Krishna.
> A few notes:
>    - Looks like the bug can be reproduced in JMenu only? Then it will be good to fix it in BasicMenuUI, moreover it is already implemented a
> getMaximumSize() in a similar way as in your fix.
>    - In the test some of the Swing components are accessed on non-EDT thread. Also note that when you start the Thread you will exit
> invokeAndWait() and it is possible that the test will end before the Thread complete(jtreg will kill the test when the main thread is completed)
>
> On 31/10/2017 05:00, Krishna Addepalli wrote:
>> Hi Sergey,
>>
>> Thanks for pointing that out. Fixed the test case and created a new
>> webrev here: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>
>> Now, the testcase throws an exception and also after a few trials, closes on its own.
>>
>> Thanks,
>> Krishna
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Tuesday, October 31, 2017 1:13 AM
>> To: Krishna Addepalli <[hidden email]>;
>> [hidden email]
>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>> flickets when label text shows "..." and is updated
>>
>> Hi, Krishna.
>> Can you please clarify in what situation the test will stop working(it does not have any assertions)?
>>
>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>> Hi All,
>>>
>>> Please review the fix for bug:
>>>
>>>
>>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>>
>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>
>>> Summary:
>>>
>>> The issue is when the label text width is more than the
>>> container(JPanel) width, the container tries to render with minimum
>>> width for all the components. In such case, the JMenuItem, which is
>>> added to the JMenuBar also returns its height dimension as 1 (the
>>> default minimum). The test case alternates between the short text and
>>> long text on the label, and it gives a flickering effect of the Menu.
>>> The fix is to return preferred size from JMenuItem, if its parent is
>>> a JMenuBar, since JMenuBar is added to the top level window.
>>>
>>> Thanks,
>>>
>>> Krishna
>>>
>>
>>
>> --
>> Best regards, Sergey.
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Krishna Addepalli
Hi Sergey,

I have modified the code as per your requests, and here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/

However, I have a couple questions regarding the fix:
1. Why was it considered good to fix it in BasicMenuUI rather than in BasicMenuUIItem?
2. While using Robot.waitForIdle(), I came across the function "Suntoolkit.flushPendingEvents" function. But this is a private function and applications cannot use this. Is it expected that the applications should import the robot module to flush the events, considering that mostly it is used for testing purposes? If this is the case, then "Suntoolkit.flushPendingEvents" becomes a valuable tool, given that Swing rendering happens in an EDT that is separate from MainThread.

Thanks,
Krishna

-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, November 15, 2017 2:23 AM
To: Krishna Addepalli <[hidden email]>; [hidden email]
Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Hi, Krishna.
Small comments: I think that this code
"(JMenu)menuItem).isTopLevelMenu() == true"
can be simplified to
(JMenu)menuItem).isTopLevelMenu().

In the test you create and dispose JFrame and other components outside of EDT thread. I suggest to use invokeAndWait() which will work synchronously. Instead of sleep() you van use Robot.waitForIdle();


On 12/11/2017 22:45, Krishna Addepalli wrote:

> Hi Sergey,
>
> Gentle reminder! Could you review the fix?
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Krishna Addepalli
> Sent: Wednesday, November 8, 2017 8:22 PM
> To: Sergey Bylokhov <[hidden email]>;
> [hidden email]
> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
> flickers when label text shows "..." and is updated
>
> Hi Sergey,
>
> Are the changes fine now?
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Krishna Addepalli
> Sent: Monday, November 6, 2017 9:02 PM
> To: Sergey Bylokhov <[hidden email]>;
> [hidden email]
> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
> flickers when label text shows "..." and is updated
>
> Hi Sergey,
>
> As per your recommendation I have done the following changes:
> 1. Moved the fix to BasicMenuUI.
> 2. Corrected the test to:
> a. Access the UI elements only in the EDT.
> b. Made sure that the main thread doesn't exit till the test is complete.
>
> Here is the updated webrev:
> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>
> However, I was wondering, why it is good to fix in BasicMenuUI? Also, while writing the testcase, I couldnot find any functionality like "flush" on the event queue, which will execute all the pending events. Is it by design?
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Friday, November 3, 2017 11:24 PM
> To: Krishna Addepalli <[hidden email]>;
> [hidden email]
> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
> flickets when label text shows "..." and is updated
>
> Hi, Krishna.
> A few notes:
>    - Looks like the bug can be reproduced in JMenu only? Then it will
> be good to fix it in BasicMenuUI, moreover it is already implemented a
> getMaximumSize() in a similar way as in your fix.
>    - In the test some of the Swing components are accessed on non-EDT
> thread. Also note that when you start the Thread you will exit
> invokeAndWait() and it is possible that the test will end before the
> Thread complete(jtreg will kill the test when the main thread is
> completed)
>
> On 31/10/2017 05:00, Krishna Addepalli wrote:
>> Hi Sergey,
>>
>> Thanks for pointing that out. Fixed the test case and created a new
>> webrev here: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>
>> Now, the testcase throws an exception and also after a few trials, closes on its own.
>>
>> Thanks,
>> Krishna
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Tuesday, October 31, 2017 1:13 AM
>> To: Krishna Addepalli <[hidden email]>;
>> [hidden email]
>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>> flickets when label text shows "..." and is updated
>>
>> Hi, Krishna.
>> Can you please clarify in what situation the test will stop working(it does not have any assertions)?
>>
>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>> Hi All,
>>>
>>> Please review the fix for bug:
>>>
>>>
>>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>>
>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>
>>> Summary:
>>>
>>> The issue is when the label text width is more than the
>>> container(JPanel) width, the container tries to render with minimum
>>> width for all the components. In such case, the JMenuItem, which is
>>> added to the JMenuBar also returns its height dimension as 1 (the
>>> default minimum). The test case alternates between the short text
>>> and long text on the label, and it gives a flickering effect of the Menu.
>>> The fix is to return preferred size from JMenuItem, if its parent is
>>> a JMenuBar, since JMenuBar is added to the top level window.
>>>
>>> Thanks,
>>>
>>> Krishna
>>>
>>
>>
>> --
>> Best regards, Sergey.
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Krishna Addepalli
Hi Sergey,

Per our conversation, I have added the volatile keyword to the Boolean flag. Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/

Thanks,
Krishna

-----Original Message-----
From: Krishna Addepalli
Sent: Wednesday, November 15, 2017 6:07 PM
To: Sergey Bylokhov <[hidden email]>; [hidden email]
Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Hi Sergey,

I have modified the code as per your requests, and here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/

However, I have a couple questions regarding the fix:
1. Why was it considered good to fix it in BasicMenuUI rather than in BasicMenuUIItem?
2. While using Robot.waitForIdle(), I came across the function "Suntoolkit.flushPendingEvents" function. But this is a private function and applications cannot use this. Is it expected that the applications should import the robot module to flush the events, considering that mostly it is used for testing purposes? If this is the case, then "Suntoolkit.flushPendingEvents" becomes a valuable tool, given that Swing rendering happens in an EDT that is separate from MainThread.

Thanks,
Krishna

-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, November 15, 2017 2:23 AM
To: Krishna Addepalli <[hidden email]>; [hidden email]
Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Hi, Krishna.
Small comments: I think that this code
"(JMenu)menuItem).isTopLevelMenu() == true"
can be simplified to
(JMenu)menuItem).isTopLevelMenu().

In the test you create and dispose JFrame and other components outside of EDT thread. I suggest to use invokeAndWait() which will work synchronously. Instead of sleep() you van use Robot.waitForIdle();


On 12/11/2017 22:45, Krishna Addepalli wrote:

> Hi Sergey,
>
> Gentle reminder! Could you review the fix?
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Krishna Addepalli
> Sent: Wednesday, November 8, 2017 8:22 PM
> To: Sergey Bylokhov <[hidden email]>;
> [hidden email]
> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
> flickers when label text shows "..." and is updated
>
> Hi Sergey,
>
> Are the changes fine now?
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Krishna Addepalli
> Sent: Monday, November 6, 2017 9:02 PM
> To: Sergey Bylokhov <[hidden email]>;
> [hidden email]
> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
> flickers when label text shows "..." and is updated
>
> Hi Sergey,
>
> As per your recommendation I have done the following changes:
> 1. Moved the fix to BasicMenuUI.
> 2. Corrected the test to:
> a. Access the UI elements only in the EDT.
> b. Made sure that the main thread doesn't exit till the test is complete.
>
> Here is the updated webrev:
> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>
> However, I was wondering, why it is good to fix in BasicMenuUI? Also, while writing the testcase, I couldnot find any functionality like "flush" on the event queue, which will execute all the pending events. Is it by design?
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Friday, November 3, 2017 11:24 PM
> To: Krishna Addepalli <[hidden email]>;
> [hidden email]
> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
> flickets when label text shows "..." and is updated
>
> Hi, Krishna.
> A few notes:
>    - Looks like the bug can be reproduced in JMenu only? Then it will
> be good to fix it in BasicMenuUI, moreover it is already implemented a
> getMaximumSize() in a similar way as in your fix.
>    - In the test some of the Swing components are accessed on non-EDT
> thread. Also note that when you start the Thread you will exit
> invokeAndWait() and it is possible that the test will end before the
> Thread complete(jtreg will kill the test when the main thread is
> completed)
>
> On 31/10/2017 05:00, Krishna Addepalli wrote:
>> Hi Sergey,
>>
>> Thanks for pointing that out. Fixed the test case and created a new
>> webrev here: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>
>> Now, the testcase throws an exception and also after a few trials, closes on its own.
>>
>> Thanks,
>> Krishna
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Tuesday, October 31, 2017 1:13 AM
>> To: Krishna Addepalli <[hidden email]>;
>> [hidden email]
>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>> flickets when label text shows "..." and is updated
>>
>> Hi, Krishna.
>> Can you please clarify in what situation the test will stop working(it does not have any assertions)?
>>
>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>> Hi All,
>>>
>>> Please review the fix for bug:
>>>
>>>
>>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>>
>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>
>>> Summary:
>>>
>>> The issue is when the label text width is more than the
>>> container(JPanel) width, the container tries to render with minimum
>>> width for all the components. In such case, the JMenuItem, which is
>>> added to the JMenuBar also returns its height dimension as 1 (the
>>> default minimum). The test case alternates between the short text
>>> and long text on the label, and it gives a flickering effect of the Menu.
>>> The fix is to return preferred size from JMenuItem, if its parent is
>>> a JMenuBar, since JMenuBar is added to the top level window.
>>>
>>> Thanks,
>>>
>>> Krishna
>>>
>>
>>
>> --
>> Best regards, Sergey.
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Sergey Bylokhov
Looks fine.

On 15/11/2017 07:26, Krishna Addepalli wrote:

> Hi Sergey,
>
> Per our conversation, I have added the volatile keyword to the Boolean flag. Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Krishna Addepalli
> Sent: Wednesday, November 15, 2017 6:07 PM
> To: Sergey Bylokhov <[hidden email]>; [hidden email]
> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated
>
> Hi Sergey,
>
> I have modified the code as per your requests, and here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>
> However, I have a couple questions regarding the fix:
> 1. Why was it considered good to fix it in BasicMenuUI rather than in BasicMenuUIItem?
> 2. While using Robot.waitForIdle(), I came across the function "Suntoolkit.flushPendingEvents" function. But this is a private function and applications cannot use this. Is it expected that the applications should import the robot module to flush the events, considering that mostly it is used for testing purposes? If this is the case, then "Suntoolkit.flushPendingEvents" becomes a valuable tool, given that Swing rendering happens in an EDT that is separate from MainThread.
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Wednesday, November 15, 2017 2:23 AM
> To: Krishna Addepalli <[hidden email]>; [hidden email]
> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated
>
> Hi, Krishna.
> Small comments: I think that this code
> "(JMenu)menuItem).isTopLevelMenu() == true"
> can be simplified to
> (JMenu)menuItem).isTopLevelMenu().
>
> In the test you create and dispose JFrame and other components outside of EDT thread. I suggest to use invokeAndWait() which will work synchronously. Instead of sleep() you van use Robot.waitForIdle();
>
>
> On 12/11/2017 22:45, Krishna Addepalli wrote:
>> Hi Sergey,
>>
>> Gentle reminder! Could you review the fix?
>>
>> Thanks,
>> Krishna
>>
>> -----Original Message-----
>> From: Krishna Addepalli
>> Sent: Wednesday, November 8, 2017 8:22 PM
>> To: Sergey Bylokhov <[hidden email]>;
>> [hidden email]
>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>> flickers when label text shows "..." and is updated
>>
>> Hi Sergey,
>>
>> Are the changes fine now?
>>
>> Thanks,
>> Krishna
>>
>> -----Original Message-----
>> From: Krishna Addepalli
>> Sent: Monday, November 6, 2017 9:02 PM
>> To: Sergey Bylokhov <[hidden email]>;
>> [hidden email]
>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>> flickers when label text shows "..." and is updated
>>
>> Hi Sergey,
>>
>> As per your recommendation I have done the following changes:
>> 1. Moved the fix to BasicMenuUI.
>> 2. Corrected the test to:
>> a. Access the UI elements only in the EDT.
>> b. Made sure that the main thread doesn't exit till the test is complete.
>>
>> Here is the updated webrev:
>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>
>> However, I was wondering, why it is good to fix in BasicMenuUI? Also, while writing the testcase, I couldnot find any functionality like "flush" on the event queue, which will execute all the pending events. Is it by design?
>>
>> Thanks,
>> Krishna
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Friday, November 3, 2017 11:24 PM
>> To: Krishna Addepalli <[hidden email]>;
>> [hidden email]
>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>> flickets when label text shows "..." and is updated
>>
>> Hi, Krishna.
>> A few notes:
>>     - Looks like the bug can be reproduced in JMenu only? Then it will
>> be good to fix it in BasicMenuUI, moreover it is already implemented a
>> getMaximumSize() in a similar way as in your fix.
>>     - In the test some of the Swing components are accessed on non-EDT
>> thread. Also note that when you start the Thread you will exit
>> invokeAndWait() and it is possible that the test will end before the
>> Thread complete(jtreg will kill the test when the main thread is
>> completed)
>>
>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>> Hi Sergey,
>>>
>>> Thanks for pointing that out. Fixed the test case and created a new
>>> webrev here: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>
>>> Now, the testcase throws an exception and also after a few trials, closes on its own.
>>>
>>> Thanks,
>>> Krishna
>>>
>>> -----Original Message-----
>>> From: Sergey Bylokhov
>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>> To: Krishna Addepalli <[hidden email]>;
>>> [hidden email]
>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>> flickets when label text shows "..." and is updated
>>>
>>> Hi, Krishna.
>>> Can you please clarify in what situation the test will stop working(it does not have any assertions)?
>>>
>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>> Hi All,
>>>>
>>>> Please review the fix for bug:
>>>>
>>>>
>>>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>
>>>> Summary:
>>>>
>>>> The issue is when the label text width is more than the
>>>> container(JPanel) width, the container tries to render with minimum
>>>> width for all the components. In such case, the JMenuItem, which is
>>>> added to the JMenuBar also returns its height dimension as 1 (the
>>>> default minimum). The test case alternates between the short text
>>>> and long text on the label, and it gives a flickering effect of the Menu.
>>>> The fix is to return preferred size from JMenuItem, if its parent is
>>>> a JMenuBar, since JMenuBar is added to the top level window.
>>>>
>>>> Thanks,
>>>>
>>>> Krishna
>>>>
>>>
>>>
>>> --
>>> Best regards, Sergey.
>>>
>>
>>
>> --
>> Best regards, Sergey.
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Semyon Sadetsky
Hi Krishna& Sergey,

The provided reg test passes on Ubuntu Linux before the fix.

Yet, I have a question about the assumption I get from the proposed
solution that any component  placed into GridBagLayout must have its
getPrefferredSize() equals to its getMinimumSize() result. Do we have
this assumption documented somewhere?

--Semyon


On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:

> Looks fine.
>
> On 15/11/2017 07:26, Krishna Addepalli wrote:
>> Hi Sergey,
>>
>> Per our conversation, I have added the volatile keyword to the
>> Boolean flag. Here is the updated webrev:
>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>
>> Thanks,
>> Krishna
>>
>> -----Original Message-----
>> From: Krishna Addepalli
>> Sent: Wednesday, November 15, 2017 6:07 PM
>> To: Sergey Bylokhov <[hidden email]>;
>> [hidden email]
>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>> flickers when label text shows "..." and is updated
>>
>> Hi Sergey,
>>
>> I have modified the code as per your requests, and here is the
>> updated webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>
>> However, I have a couple questions regarding the fix:
>> 1. Why was it considered good to fix it in BasicMenuUI rather than in
>> BasicMenuUIItem?
>> 2. While using Robot.waitForIdle(), I came across the function
>> "Suntoolkit.flushPendingEvents" function. But this is a private
>> function and applications cannot use this. Is it expected that the
>> applications should import the robot module to flush the events,
>> considering that mostly it is used for testing purposes? If this is
>> the case, then "Suntoolkit.flushPendingEvents" becomes a valuable
>> tool, given that Swing rendering happens in an EDT that is separate
>> from MainThread.
>>
>> Thanks,
>> Krishna
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Wednesday, November 15, 2017 2:23 AM
>> To: Krishna Addepalli <[hidden email]>;
>> [hidden email]
>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>> flickers when label text shows "..." and is updated
>>
>> Hi, Krishna.
>> Small comments: I think that this code
>> "(JMenu)menuItem).isTopLevelMenu() == true"
>> can be simplified to
>> (JMenu)menuItem).isTopLevelMenu().
>>
>> In the test you create and dispose JFrame and other components
>> outside of EDT thread. I suggest to use invokeAndWait() which will
>> work synchronously. Instead of sleep() you van use Robot.waitForIdle();
>>
>>
>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>> Hi Sergey,
>>>
>>> Gentle reminder! Could you review the fix?
>>>
>>> Thanks,
>>> Krishna
>>>
>>> -----Original Message-----
>>> From: Krishna Addepalli
>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>> To: Sergey Bylokhov <[hidden email]>;
>>> [hidden email]
>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>> flickers when label text shows "..." and is updated
>>>
>>> Hi Sergey,
>>>
>>> Are the changes fine now?
>>>
>>> Thanks,
>>> Krishna
>>>
>>> -----Original Message-----
>>> From: Krishna Addepalli
>>> Sent: Monday, November 6, 2017 9:02 PM
>>> To: Sergey Bylokhov <[hidden email]>;
>>> [hidden email]
>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>> flickers when label text shows "..." and is updated
>>>
>>> Hi Sergey,
>>>
>>> As per your recommendation I have done the following changes:
>>> 1. Moved the fix to BasicMenuUI.
>>> 2. Corrected the test to:
>>>     a. Access the UI elements only in the EDT.
>>>     b. Made sure that the main thread doesn't exit till the test is
>>> complete.
>>>
>>> Here is the updated webrev:
>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>
>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>> Also, while writing the testcase, I couldnot find any functionality
>>> like "flush" on the event queue, which will execute all the pending
>>> events. Is it by design?
>>>
>>> Thanks,
>>> Krishna
>>>
>>> -----Original Message-----
>>> From: Sergey Bylokhov
>>> Sent: Friday, November 3, 2017 11:24 PM
>>> To: Krishna Addepalli <[hidden email]>;
>>> [hidden email]
>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>> flickets when label text shows "..." and is updated
>>>
>>> Hi, Krishna.
>>> A few notes:
>>>     - Looks like the bug can be reproduced in JMenu only? Then it will
>>> be good to fix it in BasicMenuUI, moreover it is already implemented a
>>> getMaximumSize() in a similar way as in your fix.
>>>     - In the test some of the Swing components are accessed on non-EDT
>>> thread. Also note that when you start the Thread you will exit
>>> invokeAndWait() and it is possible that the test will end before the
>>> Thread complete(jtreg will kill the test when the main thread is
>>> completed)
>>>
>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>> Hi Sergey,
>>>>
>>>> Thanks for pointing that out. Fixed the test case and created a new
>>>> webrev here: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>
>>>> Now, the testcase throws an exception and also after a few trials,
>>>> closes on its own.
>>>>
>>>> Thanks,
>>>> Krishna
>>>>
>>>> -----Original Message-----
>>>> From: Sergey Bylokhov
>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>> To: Krishna Addepalli <[hidden email]>;
>>>> [hidden email]
>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>> flickets when label text shows "..." and is updated
>>>>
>>>> Hi, Krishna.
>>>> Can you please clarify in what situation the test will stop
>>>> working(it does not have any assertions)?
>>>>
>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>> Hi All,
>>>>>
>>>>> Please review the fix for bug:
>>>>>
>>>>>
>>>>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>
>>>>> Summary:
>>>>>
>>>>> The issue is when the label text width is more than the
>>>>> container(JPanel) width, the container tries to render with minimum
>>>>> width for all the components. In such case, the JMenuItem, which is
>>>>> added to the JMenuBar also returns its height dimension as 1 (the
>>>>> default minimum). The test case alternates between the short text
>>>>> and long text on the label, and it gives a flickering effect of
>>>>> the Menu.
>>>>> The fix is to return preferred size from JMenuItem, if its parent is
>>>>> a JMenuBar, since JMenuBar is added to the top level window.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Krishna
>>>>>
>>>>
>>>>
>>>> --
>>>> Best regards, Sergey.
>>>>
>>>
>>>
>>> --
>>> Best regards, Sergey.
>>>
>>
>>
>> --
>> Best regards, Sergey.
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Sergey Bylokhov
On 15/11/2017 08:58, Semyon Sadetsky wrote:
> Hi Krishna& Sergey,
>
> The provided reg test passes on Ubuntu Linux before the fix.

I have checked it on my Ubuntu and it always does not pass. If it pass
in your case then you should cooperate with Krishna and check what happened.

>
> Yet, I have a question about the assumption I get from the proposed
> solution that any component  placed into GridBagLayout must have its
> getPrefferredSize() equals to its getMinimumSize() result. Do we have
> this assumption documented somewhere?

There are no such restrictions that getMinimumSize() must return exact
value of getPrefferredSize(). It should return some reasonable value
instead of null, which can be considered as 1x1 by some layout managers.
In most cases the PrefferredSize is used for this purpose.

>
> --Semyon
>
>
> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>> Looks fine.
>>
>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>> Hi Sergey,
>>>
>>> Per our conversation, I have added the volatile keyword to the
>>> Boolean flag. Here is the updated webrev:
>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>
>>> Thanks,
>>> Krishna
>>>
>>> -----Original Message-----
>>> From: Krishna Addepalli
>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>> To: Sergey Bylokhov <[hidden email]>;
>>> [hidden email]
>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>> flickers when label text shows "..." and is updated
>>>
>>> Hi Sergey,
>>>
>>> I have modified the code as per your requests, and here is the
>>> updated webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>
>>> However, I have a couple questions regarding the fix:
>>> 1. Why was it considered good to fix it in BasicMenuUI rather than in
>>> BasicMenuUIItem?
>>> 2. While using Robot.waitForIdle(), I came across the function
>>> "Suntoolkit.flushPendingEvents" function. But this is a private
>>> function and applications cannot use this. Is it expected that the
>>> applications should import the robot module to flush the events,
>>> considering that mostly it is used for testing purposes? If this is
>>> the case, then "Suntoolkit.flushPendingEvents" becomes a valuable
>>> tool, given that Swing rendering happens in an EDT that is separate
>>> from MainThread.
>>>
>>> Thanks,
>>> Krishna
>>>
>>> -----Original Message-----
>>> From: Sergey Bylokhov
>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>> To: Krishna Addepalli <[hidden email]>;
>>> [hidden email]
>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>> flickers when label text shows "..." and is updated
>>>
>>> Hi, Krishna.
>>> Small comments: I think that this code
>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>> can be simplified to
>>> (JMenu)menuItem).isTopLevelMenu().
>>>
>>> In the test you create and dispose JFrame and other components
>>> outside of EDT thread. I suggest to use invokeAndWait() which will
>>> work synchronously. Instead of sleep() you van use Robot.waitForIdle();
>>>
>>>
>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>> Hi Sergey,
>>>>
>>>> Gentle reminder! Could you review the fix?
>>>>
>>>> Thanks,
>>>> Krishna
>>>>
>>>> -----Original Message-----
>>>> From: Krishna Addepalli
>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>> To: Sergey Bylokhov <[hidden email]>;
>>>> [hidden email]
>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>> flickers when label text shows "..." and is updated
>>>>
>>>> Hi Sergey,
>>>>
>>>> Are the changes fine now?
>>>>
>>>> Thanks,
>>>> Krishna
>>>>
>>>> -----Original Message-----
>>>> From: Krishna Addepalli
>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>> To: Sergey Bylokhov <[hidden email]>;
>>>> [hidden email]
>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>> flickers when label text shows "..." and is updated
>>>>
>>>> Hi Sergey,
>>>>
>>>> As per your recommendation I have done the following changes:
>>>> 1. Moved the fix to BasicMenuUI.
>>>> 2. Corrected the test to:
>>>>     a. Access the UI elements only in the EDT.
>>>>     b. Made sure that the main thread doesn't exit till the test is
>>>> complete.
>>>>
>>>> Here is the updated webrev:
>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>
>>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>>> Also, while writing the testcase, I couldnot find any functionality
>>>> like "flush" on the event queue, which will execute all the pending
>>>> events. Is it by design?
>>>>
>>>> Thanks,
>>>> Krishna
>>>>
>>>> -----Original Message-----
>>>> From: Sergey Bylokhov
>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>> To: Krishna Addepalli <[hidden email]>;
>>>> [hidden email]
>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>> flickets when label text shows "..." and is updated
>>>>
>>>> Hi, Krishna.
>>>> A few notes:
>>>>     - Looks like the bug can be reproduced in JMenu only? Then it will
>>>> be good to fix it in BasicMenuUI, moreover it is already implemented a
>>>> getMaximumSize() in a similar way as in your fix.
>>>>     - In the test some of the Swing components are accessed on non-EDT
>>>> thread. Also note that when you start the Thread you will exit
>>>> invokeAndWait() and it is possible that the test will end before the
>>>> Thread complete(jtreg will kill the test when the main thread is
>>>> completed)
>>>>
>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>> Hi Sergey,
>>>>>
>>>>> Thanks for pointing that out. Fixed the test case and created a new
>>>>> webrev here: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>
>>>>> Now, the testcase throws an exception and also after a few trials,
>>>>> closes on its own.
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>> -----Original Message-----
>>>>> From: Sergey Bylokhov
>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>> [hidden email]
>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>> flickets when label text shows "..." and is updated
>>>>>
>>>>> Hi, Krishna.
>>>>> Can you please clarify in what situation the test will stop
>>>>> working(it does not have any assertions)?
>>>>>
>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> Please review the fix for bug:
>>>>>>
>>>>>>
>>>>>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>
>>>>>> Summary:
>>>>>>
>>>>>> The issue is when the label text width is more than the
>>>>>> container(JPanel) width, the container tries to render with minimum
>>>>>> width for all the components. In such case, the JMenuItem, which is
>>>>>> added to the JMenuBar also returns its height dimension as 1 (the
>>>>>> default minimum). The test case alternates between the short text
>>>>>> and long text on the label, and it gives a flickering effect of
>>>>>> the Menu.
>>>>>> The fix is to return preferred size from JMenuItem, if its parent is
>>>>>> a JMenuBar, since JMenuBar is added to the top level window.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Krishna
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Best regards, Sergey.
>>>>>
>>>>
>>>>
>>>> --
>>>> Best regards, Sergey.
>>>>
>>>
>>>
>>> --
>>> Best regards, Sergey.
>>>
>>
>>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Semyon Sadetsky
On 11/15/2017 09:29 AM, Sergey Bylokhov wrote:

> On 15/11/2017 08:58, Semyon Sadetsky wrote:
>> Hi Krishna& Sergey,
>>
>> The provided reg test passes on Ubuntu Linux before the fix.
>
> I have checked it on my Ubuntu and it always does not pass. If it pass
> in your case then you should cooperate with Krishna and check what
> happened.
>
>>
>> Yet, I have a question about the assumption I get from the proposed
>> solution that any component  placed into GridBagLayout must have its
>> getPrefferredSize() equals to its getMinimumSize() result. Do we have
>> this assumption documented somewhere?
>
> There are no such restrictions that getMinimumSize() must return exact
> value of getPrefferredSize(). It should return some reasonable value
> instead of null, which can be considered as 1x1 by some layout
> managers. In most cases the PrefferredSize is used for this purpose.
This doesn't correspond to what I see if add

menu.setMinimumSize(new Dimension(2,2));

After that the problem described in the bug comes back.

>
>>
>> --Semyon
>>
>>
>> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>>> Looks fine.
>>>
>>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>>> Hi Sergey,
>>>>
>>>> Per our conversation, I have added the volatile keyword to the
>>>> Boolean flag. Here is the updated webrev:
>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>>
>>>> Thanks,
>>>> Krishna
>>>>
>>>> -----Original Message-----
>>>> From: Krishna Addepalli
>>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>>> To: Sergey Bylokhov <[hidden email]>;
>>>> [hidden email]
>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>> flickers when label text shows "..." and is updated
>>>>
>>>> Hi Sergey,
>>>>
>>>> I have modified the code as per your requests, and here is the
>>>> updated webrev:
>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>>
>>>> However, I have a couple questions regarding the fix:
>>>> 1. Why was it considered good to fix it in BasicMenuUI rather than
>>>> in BasicMenuUIItem?
>>>> 2. While using Robot.waitForIdle(), I came across the function
>>>> "Suntoolkit.flushPendingEvents" function. But this is a private
>>>> function and applications cannot use this. Is it expected that the
>>>> applications should import the robot module to flush the events,
>>>> considering that mostly it is used for testing purposes? If this is
>>>> the case, then "Suntoolkit.flushPendingEvents" becomes a valuable
>>>> tool, given that Swing rendering happens in an EDT that is separate
>>>> from MainThread.
>>>>
>>>> Thanks,
>>>> Krishna
>>>>
>>>> -----Original Message-----
>>>> From: Sergey Bylokhov
>>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>>> To: Krishna Addepalli <[hidden email]>;
>>>> [hidden email]
>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>> flickers when label text shows "..." and is updated
>>>>
>>>> Hi, Krishna.
>>>> Small comments: I think that this code
>>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>>> can be simplified to
>>>> (JMenu)menuItem).isTopLevelMenu().
>>>>
>>>> In the test you create and dispose JFrame and other components
>>>> outside of EDT thread. I suggest to use invokeAndWait() which will
>>>> work synchronously. Instead of sleep() you van use
>>>> Robot.waitForIdle();
>>>>
>>>>
>>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>>> Hi Sergey,
>>>>>
>>>>> Gentle reminder! Could you review the fix?
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>> -----Original Message-----
>>>>> From: Krishna Addepalli
>>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>> [hidden email]
>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>> flickers when label text shows "..." and is updated
>>>>>
>>>>> Hi Sergey,
>>>>>
>>>>> Are the changes fine now?
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>> -----Original Message-----
>>>>> From: Krishna Addepalli
>>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>> [hidden email]
>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>> flickers when label text shows "..." and is updated
>>>>>
>>>>> Hi Sergey,
>>>>>
>>>>> As per your recommendation I have done the following changes:
>>>>> 1. Moved the fix to BasicMenuUI.
>>>>> 2. Corrected the test to:
>>>>>     a. Access the UI elements only in the EDT.
>>>>>     b. Made sure that the main thread doesn't exit till the test
>>>>> is complete.
>>>>>
>>>>> Here is the updated webrev:
>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>>
>>>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>>>> Also, while writing the testcase, I couldnot find any
>>>>> functionality like "flush" on the event queue, which will execute
>>>>> all the pending events. Is it by design?
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>> -----Original Message-----
>>>>> From: Sergey Bylokhov
>>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>> [hidden email]
>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>> flickets when label text shows "..." and is updated
>>>>>
>>>>> Hi, Krishna.
>>>>> A few notes:
>>>>>     - Looks like the bug can be reproduced in JMenu only? Then it
>>>>> will
>>>>> be good to fix it in BasicMenuUI, moreover it is already
>>>>> implemented a
>>>>> getMaximumSize() in a similar way as in your fix.
>>>>>     - In the test some of the Swing components are accessed on
>>>>> non-EDT
>>>>> thread. Also note that when you start the Thread you will exit
>>>>> invokeAndWait() and it is possible that the test will end before the
>>>>> Thread complete(jtreg will kill the test when the main thread is
>>>>> completed)
>>>>>
>>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>>> Hi Sergey,
>>>>>>
>>>>>> Thanks for pointing that out. Fixed the test case and created a new
>>>>>> webrev here:
>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>>
>>>>>> Now, the testcase throws an exception and also after a few
>>>>>> trials, closes on its own.
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Sergey Bylokhov
>>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickets when label text shows "..." and is updated
>>>>>>
>>>>>> Hi, Krishna.
>>>>>> Can you please clarify in what situation the test will stop
>>>>>> working(it does not have any assertions)?
>>>>>>
>>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> Please review the fix for bug:
>>>>>>>
>>>>>>>
>>>>>>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>>
>>>>>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>>
>>>>>>> Summary:
>>>>>>>
>>>>>>> The issue is when the label text width is more than the
>>>>>>> container(JPanel) width, the container tries to render with minimum
>>>>>>> width for all the components. In such case, the JMenuItem, which is
>>>>>>> added to the JMenuBar also returns its height dimension as 1 (the
>>>>>>> default minimum). The test case alternates between the short text
>>>>>>> and long text on the label, and it gives a flickering effect of
>>>>>>> the Menu.
>>>>>>> The fix is to return preferred size from JMenuItem, if its
>>>>>>> parent is
>>>>>>> a JMenuBar, since JMenuBar is added to the top level window.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Krishna
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards, Sergey.
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Best regards, Sergey.
>>>>>
>>>>
>>>>
>>>> --
>>>> Best regards, Sergey.
>>>>
>>>
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Sergey Bylokhov
On 15/11/2017 10:53, Semyon Sadetsky wrote:
>> There are no such restrictions that getMinimumSize() must return exact
>> value of getPrefferredSize(). It should return some reasonable value
>> instead of null, which can be considered as 1x1 by some layout
>> managers. In most cases the PrefferredSize is used for this purpose.
> This doesn't correspond to what I see if add
>
> menu.setMinimumSize(new Dimension(2,2));
>
> After that the problem described in the bug comes back.

This will not be a bug, if the application thinks that 2x2 or 200x200
are reasonable values for maximum/minimum/preferred sizes and will set
it, then these values will be used.

>>
>>>
>>> --Semyon
>>>
>>>
>>> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>>>> Looks fine.
>>>>
>>>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>>>> Hi Sergey,
>>>>>
>>>>> Per our conversation, I have added the volatile keyword to the
>>>>> Boolean flag. Here is the updated webrev:
>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>> -----Original Message-----
>>>>> From: Krishna Addepalli
>>>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>> [hidden email]
>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>> flickers when label text shows "..." and is updated
>>>>>
>>>>> Hi Sergey,
>>>>>
>>>>> I have modified the code as per your requests, and here is the
>>>>> updated webrev:
>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>>>
>>>>> However, I have a couple questions regarding the fix:
>>>>> 1. Why was it considered good to fix it in BasicMenuUI rather than
>>>>> in BasicMenuUIItem?
>>>>> 2. While using Robot.waitForIdle(), I came across the function
>>>>> "Suntoolkit.flushPendingEvents" function. But this is a private
>>>>> function and applications cannot use this. Is it expected that the
>>>>> applications should import the robot module to flush the events,
>>>>> considering that mostly it is used for testing purposes? If this is
>>>>> the case, then "Suntoolkit.flushPendingEvents" becomes a valuable
>>>>> tool, given that Swing rendering happens in an EDT that is separate
>>>>> from MainThread.
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>> -----Original Message-----
>>>>> From: Sergey Bylokhov
>>>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>> [hidden email]
>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>> flickers when label text shows "..." and is updated
>>>>>
>>>>> Hi, Krishna.
>>>>> Small comments: I think that this code
>>>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>>>> can be simplified to
>>>>> (JMenu)menuItem).isTopLevelMenu().
>>>>>
>>>>> In the test you create and dispose JFrame and other components
>>>>> outside of EDT thread. I suggest to use invokeAndWait() which will
>>>>> work synchronously. Instead of sleep() you van use
>>>>> Robot.waitForIdle();
>>>>>
>>>>>
>>>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>>>> Hi Sergey,
>>>>>>
>>>>>> Gentle reminder! Could you review the fix?
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Krishna Addepalli
>>>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickers when label text shows "..." and is updated
>>>>>>
>>>>>> Hi Sergey,
>>>>>>
>>>>>> Are the changes fine now?
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Krishna Addepalli
>>>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickers when label text shows "..." and is updated
>>>>>>
>>>>>> Hi Sergey,
>>>>>>
>>>>>> As per your recommendation I have done the following changes:
>>>>>> 1. Moved the fix to BasicMenuUI.
>>>>>> 2. Corrected the test to:
>>>>>>     a. Access the UI elements only in the EDT.
>>>>>>     b. Made sure that the main thread doesn't exit till the test
>>>>>> is complete.
>>>>>>
>>>>>> Here is the updated webrev:
>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>>>
>>>>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>>>>> Also, while writing the testcase, I couldnot find any
>>>>>> functionality like "flush" on the event queue, which will execute
>>>>>> all the pending events. Is it by design?
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Sergey Bylokhov
>>>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickets when label text shows "..." and is updated
>>>>>>
>>>>>> Hi, Krishna.
>>>>>> A few notes:
>>>>>>     - Looks like the bug can be reproduced in JMenu only? Then it
>>>>>> will
>>>>>> be good to fix it in BasicMenuUI, moreover it is already
>>>>>> implemented a
>>>>>> getMaximumSize() in a similar way as in your fix.
>>>>>>     - In the test some of the Swing components are accessed on
>>>>>> non-EDT
>>>>>> thread. Also note that when you start the Thread you will exit
>>>>>> invokeAndWait() and it is possible that the test will end before the
>>>>>> Thread complete(jtreg will kill the test when the main thread is
>>>>>> completed)
>>>>>>
>>>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Thanks for pointing that out. Fixed the test case and created a new
>>>>>>> webrev here:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>>>
>>>>>>> Now, the testcase throws an exception and also after a few
>>>>>>> trials, closes on its own.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Sergey Bylokhov
>>>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>> flickets when label text shows "..." and is updated
>>>>>>>
>>>>>>> Hi, Krishna.
>>>>>>> Can you please clarify in what situation the test will stop
>>>>>>> working(it does not have any assertions)?
>>>>>>>
>>>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>>>> Hi All,
>>>>>>>>
>>>>>>>> Please review the fix for bug:
>>>>>>>>
>>>>>>>>
>>>>>>>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>>>
>>>>>>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>>>
>>>>>>>> Summary:
>>>>>>>>
>>>>>>>> The issue is when the label text width is more than the
>>>>>>>> container(JPanel) width, the container tries to render with minimum
>>>>>>>> width for all the components. In such case, the JMenuItem, which is
>>>>>>>> added to the JMenuBar also returns its height dimension as 1 (the
>>>>>>>> default minimum). The test case alternates between the short text
>>>>>>>> and long text on the label, and it gives a flickering effect of
>>>>>>>> the Menu.
>>>>>>>> The fix is to return preferred size from JMenuItem, if its
>>>>>>>> parent is
>>>>>>>> a JMenuBar, since JMenuBar is added to the top level window.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards, Sergey.
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Best regards, Sergey.
>>>>>
>>>>
>>>>
>>>
>>
>>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Krishna Addepalli
Hi Semyon,

1. I have tried in my Ubuntu 17.04 VM and was able to see the test case fail before fix, and passing after the fix.
2. Regarding the minimum size, it is not related to any layout - the test fails across any Layout - I have verified this in debugging process. The only point is, whenever the label text / any other component rendering width is larger the window size, it requests all the components to report the minimum size. In the case of JMenu (which is a top level menu), it is better to return a preferred size rather than minimum size (which by the way is 1x1), so that the item is rendered correctly. Returning null from the fixed function will cause the layout class to return the default minimum value as per the layout class used. So, I didn’t see any reason to come up with arbitrary minimum values.

Thanks,
Krishna

-----Original Message-----
From: Sergey Bylokhov
Sent: Thursday, November 16, 2017 6:51 AM
To: Semyon Sadetsky <[hidden email]>; Krishna Addepalli <[hidden email]>; [hidden email]
Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

On 15/11/2017 10:53, Semyon Sadetsky wrote:
>> There are no such restrictions that getMinimumSize() must return
>> exact value of getPrefferredSize(). It should return some reasonable
>> value instead of null, which can be considered as 1x1 by some layout
>> managers. In most cases the PrefferredSize is used for this purpose.
> This doesn't correspond to what I see if add
>
> menu.setMinimumSize(new Dimension(2,2));
>
> After that the problem described in the bug comes back.

This will not be a bug, if the application thinks that 2x2 or 200x200 are reasonable values for maximum/minimum/preferred sizes and will set it, then these values will be used.

>>
>>>
>>> --Semyon
>>>
>>>
>>> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>>>> Looks fine.
>>>>
>>>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>>>> Hi Sergey,
>>>>>
>>>>> Per our conversation, I have added the volatile keyword to the
>>>>> Boolean flag. Here is the updated webrev:
>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>> -----Original Message-----
>>>>> From: Krishna Addepalli
>>>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>> [hidden email]
>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>> flickers when label text shows "..." and is updated
>>>>>
>>>>> Hi Sergey,
>>>>>
>>>>> I have modified the code as per your requests, and here is the
>>>>> updated webrev:
>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>>>
>>>>> However, I have a couple questions regarding the fix:
>>>>> 1. Why was it considered good to fix it in BasicMenuUI rather than
>>>>> in BasicMenuUIItem?
>>>>> 2. While using Robot.waitForIdle(), I came across the function
>>>>> "Suntoolkit.flushPendingEvents" function. But this is a private
>>>>> function and applications cannot use this. Is it expected that the
>>>>> applications should import the robot module to flush the events,
>>>>> considering that mostly it is used for testing purposes? If this
>>>>> is the case, then "Suntoolkit.flushPendingEvents" becomes a
>>>>> valuable tool, given that Swing rendering happens in an EDT that
>>>>> is separate from MainThread.
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>> -----Original Message-----
>>>>> From: Sergey Bylokhov
>>>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>> [hidden email]
>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>> flickers when label text shows "..." and is updated
>>>>>
>>>>> Hi, Krishna.
>>>>> Small comments: I think that this code
>>>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>>>> can be simplified to
>>>>> (JMenu)menuItem).isTopLevelMenu().
>>>>>
>>>>> In the test you create and dispose JFrame and other components
>>>>> outside of EDT thread. I suggest to use invokeAndWait() which will
>>>>> work synchronously. Instead of sleep() you van use
>>>>> Robot.waitForIdle();
>>>>>
>>>>>
>>>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>>>> Hi Sergey,
>>>>>>
>>>>>> Gentle reminder! Could you review the fix?
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Krishna Addepalli
>>>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickers when label text shows "..." and is updated
>>>>>>
>>>>>> Hi Sergey,
>>>>>>
>>>>>> Are the changes fine now?
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Krishna Addepalli
>>>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickers when label text shows "..." and is updated
>>>>>>
>>>>>> Hi Sergey,
>>>>>>
>>>>>> As per your recommendation I have done the following changes:
>>>>>> 1. Moved the fix to BasicMenuUI.
>>>>>> 2. Corrected the test to:
>>>>>>     a. Access the UI elements only in the EDT.
>>>>>>     b. Made sure that the main thread doesn't exit till the test
>>>>>> is complete.
>>>>>>
>>>>>> Here is the updated webrev:
>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>>>
>>>>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>>>>> Also, while writing the testcase, I couldnot find any
>>>>>> functionality like "flush" on the event queue, which will execute
>>>>>> all the pending events. Is it by design?
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Sergey Bylokhov
>>>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickets when label text shows "..." and is updated
>>>>>>
>>>>>> Hi, Krishna.
>>>>>> A few notes:
>>>>>>     - Looks like the bug can be reproduced in JMenu only? Then it
>>>>>> will be good to fix it in BasicMenuUI, moreover it is already
>>>>>> implemented a
>>>>>> getMaximumSize() in a similar way as in your fix.
>>>>>>     - In the test some of the Swing components are accessed on
>>>>>> non-EDT thread. Also note that when you start the Thread you will
>>>>>> exit
>>>>>> invokeAndWait() and it is possible that the test will end before
>>>>>> the Thread complete(jtreg will kill the test when the main thread
>>>>>> is
>>>>>> completed)
>>>>>>
>>>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Thanks for pointing that out. Fixed the test case and created a
>>>>>>> new webrev here:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>>>
>>>>>>> Now, the testcase throws an exception and also after a few
>>>>>>> trials, closes on its own.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Sergey Bylokhov
>>>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>> GridBagLayout flickets when label text shows "..." and is
>>>>>>> updated
>>>>>>>
>>>>>>> Hi, Krishna.
>>>>>>> Can you please clarify in what situation the test will stop
>>>>>>> working(it does not have any assertions)?
>>>>>>>
>>>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>>>> Hi All,
>>>>>>>>
>>>>>>>> Please review the fix for bug:
>>>>>>>>
>>>>>>>>
>>>>>>>> Bug: JDK-8178430:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>>>
>>>>>>>> Webrev:
>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>>>
>>>>>>>> Summary:
>>>>>>>>
>>>>>>>> The issue is when the label text width is more than the
>>>>>>>> container(JPanel) width, the container tries to render with
>>>>>>>> minimum width for all the components. In such case, the
>>>>>>>> JMenuItem, which is added to the JMenuBar also returns its
>>>>>>>> height dimension as 1 (the default minimum). The test case
>>>>>>>> alternates between the short text and long text on the label,
>>>>>>>> and it gives a flickering effect of the Menu.
>>>>>>>> The fix is to return preferred size from JMenuItem, if its
>>>>>>>> parent is a JMenuBar, since JMenuBar is added to the top level
>>>>>>>> window.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards, Sergey.
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Best regards, Sergey.
>>>>>
>>>>
>>>>
>>>
>>
>>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Semyon Sadetsky
Hi Krishna,

On 11/16/2017 03:38 AM, Krishna Addepalli wrote:
> Hi Semyon,
>
> 1. I have tried in my Ubuntu 17.04 VM and was able to see the test case fail before fix, and passing after the fix.
> 2. Regarding the minimum size, it is not related to any layout - the test fails across any Layout - I have verified this in debugging process. The only point is, whenever the label text / any other component rendering width is larger the window size, it requests all the components to report the minimum size. In the case of JMenu (which is a top level menu), it is better to return a preferred size rather than minimum size (which by the way is 1x1), so that the item is rendered correctly. Returning null from the fixed function will cause the layout class to return the default minimum value as per the layout class used. So, I didn’t see any reason to come up with arbitrary minimum values.
I don't see this bug as a Menu issue because any other component that
has minimumSize != prefferredSize placed instead of the menu causes the
same unexpected layout behavior.
The heights of all components should be preserved with the layout width
change but the the upper component jumps to its minimum size after the
width of the window reaches some value.

--Semyon

>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Thursday, November 16, 2017 6:51 AM
> To: Semyon Sadetsky <[hidden email]>; Krishna Addepalli <[hidden email]>; [hidden email]
> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated
>
> On 15/11/2017 10:53, Semyon Sadetsky wrote:
>>> There are no such restrictions that getMinimumSize() must return
>>> exact value of getPrefferredSize(). It should return some reasonable
>>> value instead of null, which can be considered as 1x1 by some layout
>>> managers. In most cases the PrefferredSize is used for this purpose.
>> This doesn't correspond to what I see if add
>>
>> menu.setMinimumSize(new Dimension(2,2));
>>
>> After that the problem described in the bug comes back.
> This will not be a bug, if the application thinks that 2x2 or 200x200 are reasonable values for maximum/minimum/preferred sizes and will set it, then these values will be used.
>
>>>> --Semyon
>>>>
>>>>
>>>> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>>>>> Looks fine.
>>>>>
>>>>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>>>>> Hi Sergey,
>>>>>>
>>>>>> Per our conversation, I have added the volatile keyword to the
>>>>>> Boolean flag. Here is the updated webrev:
>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Krishna Addepalli
>>>>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickers when label text shows "..." and is updated
>>>>>>
>>>>>> Hi Sergey,
>>>>>>
>>>>>> I have modified the code as per your requests, and here is the
>>>>>> updated webrev:
>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>>>>
>>>>>> However, I have a couple questions regarding the fix:
>>>>>> 1. Why was it considered good to fix it in BasicMenuUI rather than
>>>>>> in BasicMenuUIItem?
>>>>>> 2. While using Robot.waitForIdle(), I came across the function
>>>>>> "Suntoolkit.flushPendingEvents" function. But this is a private
>>>>>> function and applications cannot use this. Is it expected that the
>>>>>> applications should import the robot module to flush the events,
>>>>>> considering that mostly it is used for testing purposes? If this
>>>>>> is the case, then "Suntoolkit.flushPendingEvents" becomes a
>>>>>> valuable tool, given that Swing rendering happens in an EDT that
>>>>>> is separate from MainThread.
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Sergey Bylokhov
>>>>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickers when label text shows "..." and is updated
>>>>>>
>>>>>> Hi, Krishna.
>>>>>> Small comments: I think that this code
>>>>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>>>>> can be simplified to
>>>>>> (JMenu)menuItem).isTopLevelMenu().
>>>>>>
>>>>>> In the test you create and dispose JFrame and other components
>>>>>> outside of EDT thread. I suggest to use invokeAndWait() which will
>>>>>> work synchronously. Instead of sleep() you van use
>>>>>> Robot.waitForIdle();
>>>>>>
>>>>>>
>>>>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Gentle reminder! Could you review the fix?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Krishna Addepalli
>>>>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>> flickers when label text shows "..." and is updated
>>>>>>>
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Are the changes fine now?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Krishna Addepalli
>>>>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>> flickers when label text shows "..." and is updated
>>>>>>>
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> As per your recommendation I have done the following changes:
>>>>>>> 1. Moved the fix to BasicMenuUI.
>>>>>>> 2. Corrected the test to:
>>>>>>>      a. Access the UI elements only in the EDT.
>>>>>>>      b. Made sure that the main thread doesn't exit till the test
>>>>>>> is complete.
>>>>>>>
>>>>>>> Here is the updated webrev:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>>>>
>>>>>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>>>>>> Also, while writing the testcase, I couldnot find any
>>>>>>> functionality like "flush" on the event queue, which will execute
>>>>>>> all the pending events. Is it by design?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Sergey Bylokhov
>>>>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>> flickets when label text shows "..." and is updated
>>>>>>>
>>>>>>> Hi, Krishna.
>>>>>>> A few notes:
>>>>>>>      - Looks like the bug can be reproduced in JMenu only? Then it
>>>>>>> will be good to fix it in BasicMenuUI, moreover it is already
>>>>>>> implemented a
>>>>>>> getMaximumSize() in a similar way as in your fix.
>>>>>>>      - In the test some of the Swing components are accessed on
>>>>>>> non-EDT thread. Also note that when you start the Thread you will
>>>>>>> exit
>>>>>>> invokeAndWait() and it is possible that the test will end before
>>>>>>> the Thread complete(jtreg will kill the test when the main thread
>>>>>>> is
>>>>>>> completed)
>>>>>>>
>>>>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Thanks for pointing that out. Fixed the test case and created a
>>>>>>>> new webrev here:
>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>>>>
>>>>>>>> Now, the testcase throws an exception and also after a few
>>>>>>>> trials, closes on its own.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Sergey Bylokhov
>>>>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>> GridBagLayout flickets when label text shows "..." and is
>>>>>>>> updated
>>>>>>>>
>>>>>>>> Hi, Krishna.
>>>>>>>> Can you please clarify in what situation the test will stop
>>>>>>>> working(it does not have any assertions)?
>>>>>>>>
>>>>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>>>>> Hi All,
>>>>>>>>>
>>>>>>>>> Please review the fix for bug:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Bug: JDK-8178430:
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>>>>
>>>>>>>>> Webrev:
>>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>>>>
>>>>>>>>> Summary:
>>>>>>>>>
>>>>>>>>> The issue is when the label text width is more than the
>>>>>>>>> container(JPanel) width, the container tries to render with
>>>>>>>>> minimum width for all the components. In such case, the
>>>>>>>>> JMenuItem, which is added to the JMenuBar also returns its
>>>>>>>>> height dimension as 1 (the default minimum). The test case
>>>>>>>>> alternates between the short text and long text on the label,
>>>>>>>>> and it gives a flickering effect of the Menu.
>>>>>>>>> The fix is to return preferred size from JMenuItem, if its
>>>>>>>>> parent is a JMenuBar, since JMenuBar is added to the top level
>>>>>>>>> window.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Krishna
>>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Best regards, Sergey.
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards, Sergey.
>>>>>>
>>>>>
>>>
>
> --
> Best regards, Sergey.

Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Semyon Sadetsky
In reply to this post by Sergey Bylokhov
Hi Sergey,

On 11/15/2017 05:21 PM, Sergey Bylokhov wrote:

> On 15/11/2017 10:53, Semyon Sadetsky wrote:
>>> There are no such restrictions that getMinimumSize() must return
>>> exact value of getPrefferredSize(). It should return some reasonable
>>> value instead of null, which can be considered as 1x1 by some layout
>>> managers. In most cases the PrefferredSize is used for this purpose.
>> This doesn't correspond to what I see if add
>>
>> menu.setMinimumSize(new Dimension(2,2));
>>
>> After that the problem described in the bug comes back.
>
> This will not be a bug, if the application thinks that 2x2 or 200x200
> are reasonable values for maximum/minimum/preferred sizes and will set
> it, then these values will be used.
But according to the layout constraints there enough space for preferred
size. Why it shrinks to the minimum size?

>
>>>
>>>>
>>>> --Semyon
>>>>
>>>>
>>>> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>>>>> Looks fine.
>>>>>
>>>>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>>>>> Hi Sergey,
>>>>>>
>>>>>> Per our conversation, I have added the volatile keyword to the
>>>>>> Boolean flag. Here is the updated webrev:
>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Krishna Addepalli
>>>>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickers when label text shows "..." and is updated
>>>>>>
>>>>>> Hi Sergey,
>>>>>>
>>>>>> I have modified the code as per your requests, and here is the
>>>>>> updated webrev:
>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>>>>
>>>>>> However, I have a couple questions regarding the fix:
>>>>>> 1. Why was it considered good to fix it in BasicMenuUI rather
>>>>>> than in BasicMenuUIItem?
>>>>>> 2. While using Robot.waitForIdle(), I came across the function
>>>>>> "Suntoolkit.flushPendingEvents" function. But this is a private
>>>>>> function and applications cannot use this. Is it expected that
>>>>>> the applications should import the robot module to flush the
>>>>>> events, considering that mostly it is used for testing purposes?
>>>>>> If this is the case, then "Suntoolkit.flushPendingEvents" becomes
>>>>>> a valuable tool, given that Swing rendering happens in an EDT
>>>>>> that is separate from MainThread.
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Sergey Bylokhov
>>>>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickers when label text shows "..." and is updated
>>>>>>
>>>>>> Hi, Krishna.
>>>>>> Small comments: I think that this code
>>>>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>>>>> can be simplified to
>>>>>> (JMenu)menuItem).isTopLevelMenu().
>>>>>>
>>>>>> In the test you create and dispose JFrame and other components
>>>>>> outside of EDT thread. I suggest to use invokeAndWait() which
>>>>>> will work synchronously. Instead of sleep() you van use
>>>>>> Robot.waitForIdle();
>>>>>>
>>>>>>
>>>>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Gentle reminder! Could you review the fix?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Krishna Addepalli
>>>>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>> flickers when label text shows "..." and is updated
>>>>>>>
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Are the changes fine now?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Krishna Addepalli
>>>>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>> flickers when label text shows "..." and is updated
>>>>>>>
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> As per your recommendation I have done the following changes:
>>>>>>> 1. Moved the fix to BasicMenuUI.
>>>>>>> 2. Corrected the test to:
>>>>>>>     a. Access the UI elements only in the EDT.
>>>>>>>     b. Made sure that the main thread doesn't exit till the test
>>>>>>> is complete.
>>>>>>>
>>>>>>> Here is the updated webrev:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>>>>
>>>>>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>>>>>> Also, while writing the testcase, I couldnot find any
>>>>>>> functionality like "flush" on the event queue, which will
>>>>>>> execute all the pending events. Is it by design?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Sergey Bylokhov
>>>>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>> flickets when label text shows "..." and is updated
>>>>>>>
>>>>>>> Hi, Krishna.
>>>>>>> A few notes:
>>>>>>>     - Looks like the bug can be reproduced in JMenu only? Then
>>>>>>> it will
>>>>>>> be good to fix it in BasicMenuUI, moreover it is already
>>>>>>> implemented a
>>>>>>> getMaximumSize() in a similar way as in your fix.
>>>>>>>     - In the test some of the Swing components are accessed on
>>>>>>> non-EDT
>>>>>>> thread. Also note that when you start the Thread you will exit
>>>>>>> invokeAndWait() and it is possible that the test will end before
>>>>>>> the
>>>>>>> Thread complete(jtreg will kill the test when the main thread is
>>>>>>> completed)
>>>>>>>
>>>>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Thanks for pointing that out. Fixed the test case and created a
>>>>>>>> new
>>>>>>>> webrev here:
>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>>>>
>>>>>>>> Now, the testcase throws an exception and also after a few
>>>>>>>> trials, closes on its own.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Sergey Bylokhov
>>>>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>>> flickets when label text shows "..." and is updated
>>>>>>>>
>>>>>>>> Hi, Krishna.
>>>>>>>> Can you please clarify in what situation the test will stop
>>>>>>>> working(it does not have any assertions)?
>>>>>>>>
>>>>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>>>>> Hi All,
>>>>>>>>>
>>>>>>>>> Please review the fix for bug:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Bug: JDK-8178430:
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>>>>
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>>>>
>>>>>>>>> Summary:
>>>>>>>>>
>>>>>>>>> The issue is when the label text width is more than the
>>>>>>>>> container(JPanel) width, the container tries to render with
>>>>>>>>> minimum
>>>>>>>>> width for all the components. In such case, the JMenuItem,
>>>>>>>>> which is
>>>>>>>>> added to the JMenuBar also returns its height dimension as 1 (the
>>>>>>>>> default minimum). The test case alternates between the short text
>>>>>>>>> and long text on the label, and it gives a flickering effect
>>>>>>>>> of the Menu.
>>>>>>>>> The fix is to return preferred size from JMenuItem, if its
>>>>>>>>> parent is
>>>>>>>>> a JMenuBar, since JMenuBar is added to the top level window.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Krishna
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Best regards, Sergey.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards, Sergey.
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Sergey Bylokhov
On 16/11/2017 09:17, Semyon Sadetsky wrote:
>> This will not be a bug, if the application thinks that 2x2 or 200x200
>> are reasonable values for maximum/minimum/preferred sizes and will set
>> it, then these values will be used.
> But according to the layout constraints there enough space for preferred
> size. Why it shrinks to the minimum size?

Why minimum size cannot be used if the component reports this small
size? Its the problem of the component if it reports the size which
broke the appearance of the component.

>>
>>>>
>>>>>
>>>>> --Semyon
>>>>>
>>>>>
>>>>> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>>>>>> Looks fine.
>>>>>>
>>>>>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Per our conversation, I have added the volatile keyword to the
>>>>>>> Boolean flag. Here is the updated webrev:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Krishna Addepalli
>>>>>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>> flickers when label text shows "..." and is updated
>>>>>>>
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> I have modified the code as per your requests, and here is the
>>>>>>> updated webrev:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>>>>>
>>>>>>> However, I have a couple questions regarding the fix:
>>>>>>> 1. Why was it considered good to fix it in BasicMenuUI rather
>>>>>>> than in BasicMenuUIItem?
>>>>>>> 2. While using Robot.waitForIdle(), I came across the function
>>>>>>> "Suntoolkit.flushPendingEvents" function. But this is a private
>>>>>>> function and applications cannot use this. Is it expected that
>>>>>>> the applications should import the robot module to flush the
>>>>>>> events, considering that mostly it is used for testing purposes?
>>>>>>> If this is the case, then "Suntoolkit.flushPendingEvents" becomes
>>>>>>> a valuable tool, given that Swing rendering happens in an EDT
>>>>>>> that is separate from MainThread.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Sergey Bylokhov
>>>>>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>> flickers when label text shows "..." and is updated
>>>>>>>
>>>>>>> Hi, Krishna.
>>>>>>> Small comments: I think that this code
>>>>>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>>>>>> can be simplified to
>>>>>>> (JMenu)menuItem).isTopLevelMenu().
>>>>>>>
>>>>>>> In the test you create and dispose JFrame and other components
>>>>>>> outside of EDT thread. I suggest to use invokeAndWait() which
>>>>>>> will work synchronously. Instead of sleep() you van use
>>>>>>> Robot.waitForIdle();
>>>>>>>
>>>>>>>
>>>>>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Gentle reminder! Could you review the fix?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Krishna Addepalli
>>>>>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>>> flickers when label text shows "..." and is updated
>>>>>>>>
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Are the changes fine now?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Krishna Addepalli
>>>>>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>>> flickers when label text shows "..." and is updated
>>>>>>>>
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> As per your recommendation I have done the following changes:
>>>>>>>> 1. Moved the fix to BasicMenuUI.
>>>>>>>> 2. Corrected the test to:
>>>>>>>>     a. Access the UI elements only in the EDT.
>>>>>>>>     b. Made sure that the main thread doesn't exit till the test
>>>>>>>> is complete.
>>>>>>>>
>>>>>>>> Here is the updated webrev:
>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>>>>>
>>>>>>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>>>>>>> Also, while writing the testcase, I couldnot find any
>>>>>>>> functionality like "flush" on the event queue, which will
>>>>>>>> execute all the pending events. Is it by design?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Sergey Bylokhov
>>>>>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>>> flickets when label text shows "..." and is updated
>>>>>>>>
>>>>>>>> Hi, Krishna.
>>>>>>>> A few notes:
>>>>>>>>     - Looks like the bug can be reproduced in JMenu only? Then
>>>>>>>> it will
>>>>>>>> be good to fix it in BasicMenuUI, moreover it is already
>>>>>>>> implemented a
>>>>>>>> getMaximumSize() in a similar way as in your fix.
>>>>>>>>     - In the test some of the Swing components are accessed on
>>>>>>>> non-EDT
>>>>>>>> thread. Also note that when you start the Thread you will exit
>>>>>>>> invokeAndWait() and it is possible that the test will end before
>>>>>>>> the
>>>>>>>> Thread complete(jtreg will kill the test when the main thread is
>>>>>>>> completed)
>>>>>>>>
>>>>>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>>>>>> Hi Sergey,
>>>>>>>>>
>>>>>>>>> Thanks for pointing that out. Fixed the test case and created a
>>>>>>>>> new
>>>>>>>>> webrev here:
>>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>>>>>
>>>>>>>>> Now, the testcase throws an exception and also after a few
>>>>>>>>> trials, closes on its own.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Krishna
>>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Sergey Bylokhov
>>>>>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>>>> [hidden email]
>>>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>>>> flickets when label text shows "..." and is updated
>>>>>>>>>
>>>>>>>>> Hi, Krishna.
>>>>>>>>> Can you please clarify in what situation the test will stop
>>>>>>>>> working(it does not have any assertions)?
>>>>>>>>>
>>>>>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>>>>>> Hi All,
>>>>>>>>>>
>>>>>>>>>> Please review the fix for bug:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Bug: JDK-8178430:
>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>>>>>
>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>>>>>
>>>>>>>>>> Summary:
>>>>>>>>>>
>>>>>>>>>> The issue is when the label text width is more than the
>>>>>>>>>> container(JPanel) width, the container tries to render with
>>>>>>>>>> minimum
>>>>>>>>>> width for all the components. In such case, the JMenuItem,
>>>>>>>>>> which is
>>>>>>>>>> added to the JMenuBar also returns its height dimension as 1 (the
>>>>>>>>>> default minimum). The test case alternates between the short text
>>>>>>>>>> and long text on the label, and it gives a flickering effect
>>>>>>>>>> of the Menu.
>>>>>>>>>> The fix is to return preferred size from JMenuItem, if its
>>>>>>>>>> parent is
>>>>>>>>>> a JMenuBar, since JMenuBar is added to the top level window.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Krishna
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Best regards, Sergey.
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Best regards, Sergey.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Krishna Addepalli
In reply to this post by Semyon Sadetsky
Hi Semyon,

Yes, you are right about all the components exhibiting the same behavior. But, that would require more extensive analysis, and I feel should be addressed in its own bug - which can be linked to this.

Thanks,
Krishna

-----Original Message-----
From: Semyon Sadetsky
Sent: Thursday, November 16, 2017 10:42 PM
To: Krishna Addepalli <[hidden email]>; Sergey Bylokhov <[hidden email]>; [hidden email]
Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Hi Krishna,

On 11/16/2017 03:38 AM, Krishna Addepalli wrote:
> Hi Semyon,
>
> 1. I have tried in my Ubuntu 17.04 VM and was able to see the test case fail before fix, and passing after the fix.
> 2. Regarding the minimum size, it is not related to any layout - the test fails across any Layout - I have verified this in debugging process. The only point is, whenever the label text / any other component rendering width is larger the window size, it requests all the components to report the minimum size. In the case of JMenu (which is a top level menu), it is better to return a preferred size rather than minimum size (which by the way is 1x1), so that the item is rendered correctly. Returning null from the fixed function will cause the layout class to return the default minimum value as per the layout class used. So, I didn’t see any reason to come up with arbitrary minimum values.
I don't see this bug as a Menu issue because any other component that has minimumSize != prefferredSize placed instead of the menu causes the same unexpected layout behavior.
The heights of all components should be preserved with the layout width change but the the upper component jumps to its minimum size after the width of the window reaches some value.

--Semyon

>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Thursday, November 16, 2017 6:51 AM
> To: Semyon Sadetsky <[hidden email]>; Krishna Addepalli
> <[hidden email]>; [hidden email]
> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
> flickers when label text shows "..." and is updated
>
> On 15/11/2017 10:53, Semyon Sadetsky wrote:
>>> There are no such restrictions that getMinimumSize() must return
>>> exact value of getPrefferredSize(). It should return some reasonable
>>> value instead of null, which can be considered as 1x1 by some layout
>>> managers. In most cases the PrefferredSize is used for this purpose.
>> This doesn't correspond to what I see if add
>>
>> menu.setMinimumSize(new Dimension(2,2));
>>
>> After that the problem described in the bug comes back.
> This will not be a bug, if the application thinks that 2x2 or 200x200 are reasonable values for maximum/minimum/preferred sizes and will set it, then these values will be used.
>
>>>> --Semyon
>>>>
>>>>
>>>> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>>>>> Looks fine.
>>>>>
>>>>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>>>>> Hi Sergey,
>>>>>>
>>>>>> Per our conversation, I have added the volatile keyword to the
>>>>>> Boolean flag. Here is the updated webrev:
>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Krishna Addepalli
>>>>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickers when label text shows "..." and is updated
>>>>>>
>>>>>> Hi Sergey,
>>>>>>
>>>>>> I have modified the code as per your requests, and here is the
>>>>>> updated webrev:
>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>>>>
>>>>>> However, I have a couple questions regarding the fix:
>>>>>> 1. Why was it considered good to fix it in BasicMenuUI rather
>>>>>> than in BasicMenuUIItem?
>>>>>> 2. While using Robot.waitForIdle(), I came across the function
>>>>>> "Suntoolkit.flushPendingEvents" function. But this is a private
>>>>>> function and applications cannot use this. Is it expected that
>>>>>> the applications should import the robot module to flush the
>>>>>> events, considering that mostly it is used for testing purposes?
>>>>>> If this is the case, then "Suntoolkit.flushPendingEvents" becomes
>>>>>> a valuable tool, given that Swing rendering happens in an EDT
>>>>>> that is separate from MainThread.
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Sergey Bylokhov
>>>>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>> [hidden email]
>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickers when label text shows "..." and is updated
>>>>>>
>>>>>> Hi, Krishna.
>>>>>> Small comments: I think that this code
>>>>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>>>>> can be simplified to
>>>>>> (JMenu)menuItem).isTopLevelMenu().
>>>>>>
>>>>>> In the test you create and dispose JFrame and other components
>>>>>> outside of EDT thread. I suggest to use invokeAndWait() which
>>>>>> will work synchronously. Instead of sleep() you van use
>>>>>> Robot.waitForIdle();
>>>>>>
>>>>>>
>>>>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Gentle reminder! Could you review the fix?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Krishna Addepalli
>>>>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>> updated
>>>>>>>
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Are the changes fine now?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Krishna Addepalli
>>>>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>> updated
>>>>>>>
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> As per your recommendation I have done the following changes:
>>>>>>> 1. Moved the fix to BasicMenuUI.
>>>>>>> 2. Corrected the test to:
>>>>>>>      a. Access the UI elements only in the EDT.
>>>>>>>      b. Made sure that the main thread doesn't exit till the
>>>>>>> test is complete.
>>>>>>>
>>>>>>> Here is the updated webrev:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>>>>
>>>>>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>>>>>> Also, while writing the testcase, I couldnot find any
>>>>>>> functionality like "flush" on the event queue, which will
>>>>>>> execute all the pending events. Is it by design?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Sergey Bylokhov
>>>>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>> GridBagLayout flickets when label text shows "..." and is
>>>>>>> updated
>>>>>>>
>>>>>>> Hi, Krishna.
>>>>>>> A few notes:
>>>>>>>      - Looks like the bug can be reproduced in JMenu only? Then
>>>>>>> it will be good to fix it in BasicMenuUI, moreover it is already
>>>>>>> implemented a
>>>>>>> getMaximumSize() in a similar way as in your fix.
>>>>>>>      - In the test some of the Swing components are accessed on
>>>>>>> non-EDT thread. Also note that when you start the Thread you
>>>>>>> will exit
>>>>>>> invokeAndWait() and it is possible that the test will end before
>>>>>>> the Thread complete(jtreg will kill the test when the main
>>>>>>> thread is
>>>>>>> completed)
>>>>>>>
>>>>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Thanks for pointing that out. Fixed the test case and created a
>>>>>>>> new webrev here:
>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>>>>
>>>>>>>> Now, the testcase throws an exception and also after a few
>>>>>>>> trials, closes on its own.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Sergey Bylokhov
>>>>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>> GridBagLayout flickets when label text shows "..." and is
>>>>>>>> updated
>>>>>>>>
>>>>>>>> Hi, Krishna.
>>>>>>>> Can you please clarify in what situation the test will stop
>>>>>>>> working(it does not have any assertions)?
>>>>>>>>
>>>>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>>>>> Hi All,
>>>>>>>>>
>>>>>>>>> Please review the fix for bug:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Bug: JDK-8178430:
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>>>>
>>>>>>>>> Webrev:
>>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>>>>
>>>>>>>>> Summary:
>>>>>>>>>
>>>>>>>>> The issue is when the label text width is more than the
>>>>>>>>> container(JPanel) width, the container tries to render with
>>>>>>>>> minimum width for all the components. In such case, the
>>>>>>>>> JMenuItem, which is added to the JMenuBar also returns its
>>>>>>>>> height dimension as 1 (the default minimum). The test case
>>>>>>>>> alternates between the short text and long text on the label,
>>>>>>>>> and it gives a flickering effect of the Menu.
>>>>>>>>> The fix is to return preferred size from JMenuItem, if its
>>>>>>>>> parent is a JMenuBar, since JMenuBar is added to the top level
>>>>>>>>> window.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Krishna
>>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Best regards, Sergey.
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards, Sergey.
>>>>>>
>>>>>
>>>
>
> --
> Best regards, Sergey.

Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Krishna Addepalli
In reply to this post by Sergey Bylokhov
The problem is because the label text is too long to be shown in the current window size. Hence, the renderer asks for the minimum size for all the components.
The default minimum size for JMenu (which is from BasicMenuItemUI) is 1x1, which is what has been rendered as. The proposed fix is if the JMenu is a toplevel menu, then instead of returning the default minimum size, return the preferred size, so that the component can be rendered correctly at reasonable minimum size of the window.

-----Original Message-----
From: Sergey Bylokhov
Sent: Friday, November 17, 2017 6:20 AM
To: Semyon Sadetsky <[hidden email]>; Krishna Addepalli <[hidden email]>; [hidden email]
Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

On 16/11/2017 09:17, Semyon Sadetsky wrote:
>> This will not be a bug, if the application thinks that 2x2 or 200x200
>> are reasonable values for maximum/minimum/preferred sizes and will
>> set it, then these values will be used.
> But according to the layout constraints there enough space for
> preferred size. Why it shrinks to the minimum size?

Why minimum size cannot be used if the component reports this small size? Its the problem of the component if it reports the size which broke the appearance of the component.

>>
>>>>
>>>>>
>>>>> --Semyon
>>>>>
>>>>>
>>>>> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>>>>>> Looks fine.
>>>>>>
>>>>>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Per our conversation, I have added the volatile keyword to the
>>>>>>> Boolean flag. Here is the updated webrev:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Krishna Addepalli
>>>>>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>> updated
>>>>>>>
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> I have modified the code as per your requests, and here is the
>>>>>>> updated webrev:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>>>>>
>>>>>>> However, I have a couple questions regarding the fix:
>>>>>>> 1. Why was it considered good to fix it in BasicMenuUI rather
>>>>>>> than in BasicMenuUIItem?
>>>>>>> 2. While using Robot.waitForIdle(), I came across the function
>>>>>>> "Suntoolkit.flushPendingEvents" function. But this is a private
>>>>>>> function and applications cannot use this. Is it expected that
>>>>>>> the applications should import the robot module to flush the
>>>>>>> events, considering that mostly it is used for testing purposes?
>>>>>>> If this is the case, then "Suntoolkit.flushPendingEvents"
>>>>>>> becomes a valuable tool, given that Swing rendering happens in
>>>>>>> an EDT that is separate from MainThread.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Sergey Bylokhov
>>>>>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>> updated
>>>>>>>
>>>>>>> Hi, Krishna.
>>>>>>> Small comments: I think that this code
>>>>>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>>>>>> can be simplified to
>>>>>>> (JMenu)menuItem).isTopLevelMenu().
>>>>>>>
>>>>>>> In the test you create and dispose JFrame and other components
>>>>>>> outside of EDT thread. I suggest to use invokeAndWait() which
>>>>>>> will work synchronously. Instead of sleep() you van use
>>>>>>> Robot.waitForIdle();
>>>>>>>
>>>>>>>
>>>>>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Gentle reminder! Could you review the fix?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Krishna Addepalli
>>>>>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>>> updated
>>>>>>>>
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Are the changes fine now?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Krishna Addepalli
>>>>>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>>> updated
>>>>>>>>
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> As per your recommendation I have done the following changes:
>>>>>>>> 1. Moved the fix to BasicMenuUI.
>>>>>>>> 2. Corrected the test to:
>>>>>>>>     a. Access the UI elements only in the EDT.
>>>>>>>>     b. Made sure that the main thread doesn't exit till the
>>>>>>>> test is complete.
>>>>>>>>
>>>>>>>> Here is the updated webrev:
>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>>>>>
>>>>>>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>>>>>>> Also, while writing the testcase, I couldnot find any
>>>>>>>> functionality like "flush" on the event queue, which will
>>>>>>>> execute all the pending events. Is it by design?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Sergey Bylokhov
>>>>>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>> GridBagLayout flickets when label text shows "..." and is
>>>>>>>> updated
>>>>>>>>
>>>>>>>> Hi, Krishna.
>>>>>>>> A few notes:
>>>>>>>>     - Looks like the bug can be reproduced in JMenu only? Then
>>>>>>>> it will be good to fix it in BasicMenuUI, moreover it is
>>>>>>>> already implemented a
>>>>>>>> getMaximumSize() in a similar way as in your fix.
>>>>>>>>     - In the test some of the Swing components are accessed on
>>>>>>>> non-EDT thread. Also note that when you start the Thread you
>>>>>>>> will exit
>>>>>>>> invokeAndWait() and it is possible that the test will end
>>>>>>>> before the Thread complete(jtreg will kill the test when the
>>>>>>>> main thread is
>>>>>>>> completed)
>>>>>>>>
>>>>>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>>>>>> Hi Sergey,
>>>>>>>>>
>>>>>>>>> Thanks for pointing that out. Fixed the test case and created
>>>>>>>>> a new webrev here:
>>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>>>>>
>>>>>>>>> Now, the testcase throws an exception and also after a few
>>>>>>>>> trials, closes on its own.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Krishna
>>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Sergey Bylokhov
>>>>>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>>>> [hidden email]
>>>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>>> GridBagLayout flickets when label text shows "..." and is
>>>>>>>>> updated
>>>>>>>>>
>>>>>>>>> Hi, Krishna.
>>>>>>>>> Can you please clarify in what situation the test will stop
>>>>>>>>> working(it does not have any assertions)?
>>>>>>>>>
>>>>>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>>>>>> Hi All,
>>>>>>>>>>
>>>>>>>>>> Please review the fix for bug:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Bug: JDK-8178430:
>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>>>>>
>>>>>>>>>> Webrev:
>>>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>>>>>
>>>>>>>>>> Summary:
>>>>>>>>>>
>>>>>>>>>> The issue is when the label text width is more than the
>>>>>>>>>> container(JPanel) width, the container tries to render with
>>>>>>>>>> minimum width for all the components. In such case, the
>>>>>>>>>> JMenuItem, which is added to the JMenuBar also returns its
>>>>>>>>>> height dimension as 1 (the default minimum). The test case
>>>>>>>>>> alternates between the short text and long text on the label,
>>>>>>>>>> and it gives a flickering effect of the Menu.
>>>>>>>>>> The fix is to return preferred size from JMenuItem, if its
>>>>>>>>>> parent is a JMenuBar, since JMenuBar is added to the top
>>>>>>>>>> level window.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Krishna
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Best regards, Sergey.
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Best regards, Sergey.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Semyon Sadetsky
In reply to this post by Krishna Addepalli
Can you create this bug send link to it to this thread?

--Semyon

On 11/17/17 1:45 AM, Krishna Addepalli wrote:

> Hi Semyon,
>
> Yes, you are right about all the components exhibiting the same behavior. But, that would require more extensive analysis, and I feel should be addressed in its own bug - which can be linked to this.
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Semyon Sadetsky
> Sent: Thursday, November 16, 2017 10:42 PM
> To: Krishna Addepalli <[hidden email]>; Sergey Bylokhov <[hidden email]>; [hidden email]
> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated
>
> Hi Krishna,
>
> On 11/16/2017 03:38 AM, Krishna Addepalli wrote:
>> Hi Semyon,
>>
>> 1. I have tried in my Ubuntu 17.04 VM and was able to see the test case fail before fix, and passing after the fix.
>> 2. Regarding the minimum size, it is not related to any layout - the test fails across any Layout - I have verified this in debugging process. The only point is, whenever the label text / any other component rendering width is larger the window size, it requests all the components to report the minimum size. In the case of JMenu (which is a top level menu), it is better to return a preferred size rather than minimum size (which by the way is 1x1), so that the item is rendered correctly. Returning null from the fixed function will cause the layout class to return the default minimum value as per the layout class used. So, I didn’t see any reason to come up with arbitrary minimum values.
> I don't see this bug as a Menu issue because any other component that has minimumSize != prefferredSize placed instead of the menu causes the same unexpected layout behavior.
> The heights of all components should be preserved with the layout width change but the the upper component jumps to its minimum size after the width of the window reaches some value.
>
> --Semyon
>> Thanks,
>> Krishna
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Thursday, November 16, 2017 6:51 AM
>> To: Semyon Sadetsky <[hidden email]>; Krishna Addepalli
>> <[hidden email]>; [hidden email]
>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>> flickers when label text shows "..." and is updated
>>
>> On 15/11/2017 10:53, Semyon Sadetsky wrote:
>>>> There are no such restrictions that getMinimumSize() must return
>>>> exact value of getPrefferredSize(). It should return some reasonable
>>>> value instead of null, which can be considered as 1x1 by some layout
>>>> managers. In most cases the PrefferredSize is used for this purpose.
>>> This doesn't correspond to what I see if add
>>>
>>> menu.setMinimumSize(new Dimension(2,2));
>>>
>>> After that the problem described in the bug comes back.
>> This will not be a bug, if the application thinks that 2x2 or 200x200 are reasonable values for maximum/minimum/preferred sizes and will set it, then these values will be used.
>>
>>>>> --Semyon
>>>>>
>>>>>
>>>>> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>>>>>> Looks fine.
>>>>>>
>>>>>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Per our conversation, I have added the volatile keyword to the
>>>>>>> Boolean flag. Here is the updated webrev:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Krishna Addepalli
>>>>>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>> flickers when label text shows "..." and is updated
>>>>>>>
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> I have modified the code as per your requests, and here is the
>>>>>>> updated webrev:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>>>>>
>>>>>>> However, I have a couple questions regarding the fix:
>>>>>>> 1. Why was it considered good to fix it in BasicMenuUI rather
>>>>>>> than in BasicMenuUIItem?
>>>>>>> 2. While using Robot.waitForIdle(), I came across the function
>>>>>>> "Suntoolkit.flushPendingEvents" function. But this is a private
>>>>>>> function and applications cannot use this. Is it expected that
>>>>>>> the applications should import the robot module to flush the
>>>>>>> events, considering that mostly it is used for testing purposes?
>>>>>>> If this is the case, then "Suntoolkit.flushPendingEvents" becomes
>>>>>>> a valuable tool, given that Swing rendering happens in an EDT
>>>>>>> that is separate from MainThread.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Sergey Bylokhov
>>>>>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>>> flickers when label text shows "..." and is updated
>>>>>>>
>>>>>>> Hi, Krishna.
>>>>>>> Small comments: I think that this code
>>>>>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>>>>>> can be simplified to
>>>>>>> (JMenu)menuItem).isTopLevelMenu().
>>>>>>>
>>>>>>> In the test you create and dispose JFrame and other components
>>>>>>> outside of EDT thread. I suggest to use invokeAndWait() which
>>>>>>> will work synchronously. Instead of sleep() you van use
>>>>>>> Robot.waitForIdle();
>>>>>>>
>>>>>>>
>>>>>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Gentle reminder! Could you review the fix?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Krishna Addepalli
>>>>>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>>> updated
>>>>>>>>
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Are the changes fine now?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Krishna Addepalli
>>>>>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>>> updated
>>>>>>>>
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> As per your recommendation I have done the following changes:
>>>>>>>> 1. Moved the fix to BasicMenuUI.
>>>>>>>> 2. Corrected the test to:
>>>>>>>>       a. Access the UI elements only in the EDT.
>>>>>>>>       b. Made sure that the main thread doesn't exit till the
>>>>>>>> test is complete.
>>>>>>>>
>>>>>>>> Here is the updated webrev:
>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>>>>>
>>>>>>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>>>>>>> Also, while writing the testcase, I couldnot find any
>>>>>>>> functionality like "flush" on the event queue, which will
>>>>>>>> execute all the pending events. Is it by design?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Sergey Bylokhov
>>>>>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>> GridBagLayout flickets when label text shows "..." and is
>>>>>>>> updated
>>>>>>>>
>>>>>>>> Hi, Krishna.
>>>>>>>> A few notes:
>>>>>>>>       - Looks like the bug can be reproduced in JMenu only? Then
>>>>>>>> it will be good to fix it in BasicMenuUI, moreover it is already
>>>>>>>> implemented a
>>>>>>>> getMaximumSize() in a similar way as in your fix.
>>>>>>>>       - In the test some of the Swing components are accessed on
>>>>>>>> non-EDT thread. Also note that when you start the Thread you
>>>>>>>> will exit
>>>>>>>> invokeAndWait() and it is possible that the test will end before
>>>>>>>> the Thread complete(jtreg will kill the test when the main
>>>>>>>> thread is
>>>>>>>> completed)
>>>>>>>>
>>>>>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>>>>>> Hi Sergey,
>>>>>>>>>
>>>>>>>>> Thanks for pointing that out. Fixed the test case and created a
>>>>>>>>> new webrev here:
>>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>>>>>
>>>>>>>>> Now, the testcase throws an exception and also after a few
>>>>>>>>> trials, closes on its own.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Krishna
>>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Sergey Bylokhov
>>>>>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>>>> [hidden email]
>>>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>>> GridBagLayout flickets when label text shows "..." and is
>>>>>>>>> updated
>>>>>>>>>
>>>>>>>>> Hi, Krishna.
>>>>>>>>> Can you please clarify in what situation the test will stop
>>>>>>>>> working(it does not have any assertions)?
>>>>>>>>>
>>>>>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>>>>>> Hi All,
>>>>>>>>>>
>>>>>>>>>> Please review the fix for bug:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Bug: JDK-8178430:
>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>>>>>
>>>>>>>>>> Webrev:
>>>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>>>>>
>>>>>>>>>> Summary:
>>>>>>>>>>
>>>>>>>>>> The issue is when the label text width is more than the
>>>>>>>>>> container(JPanel) width, the container tries to render with
>>>>>>>>>> minimum width for all the components. In such case, the
>>>>>>>>>> JMenuItem, which is added to the JMenuBar also returns its
>>>>>>>>>> height dimension as 1 (the default minimum). The test case
>>>>>>>>>> alternates between the short text and long text on the label,
>>>>>>>>>> and it gives a flickering effect of the Menu.
>>>>>>>>>> The fix is to return preferred size from JMenuItem, if its
>>>>>>>>>> parent is a JMenuBar, since JMenuBar is added to the top level
>>>>>>>>>> window.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Krishna
>>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Best regards, Sergey.
>>>>>>>>>
>>>>>>>> --
>>>>>>>> Best regards, Sergey.
>>>>>>>>
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>>>>
>> --
>> Best regards, Sergey.

Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Krishna Addepalli
Hi Semyon,
Created a bug : https://bugs.openjdk.java.net/browse/JDK-8191552

Please add any more information that you feel is necessary.

Thanks,
Krishna

-----Original Message-----
From: Semyon Sadetsky
Sent: Friday, November 17, 2017 9:39 PM
To: Krishna Addepalli <[hidden email]>; Sergey Bylokhov <[hidden email]>; [hidden email]
Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Can you create this bug send link to it to this thread?

--Semyon

On 11/17/17 1:45 AM, Krishna Addepalli wrote:

> Hi Semyon,
>
> Yes, you are right about all the components exhibiting the same behavior. But, that would require more extensive analysis, and I feel should be addressed in its own bug - which can be linked to this.
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Semyon Sadetsky
> Sent: Thursday, November 16, 2017 10:42 PM
> To: Krishna Addepalli <[hidden email]>; Sergey Bylokhov
> <[hidden email]>; [hidden email]
> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
> flickers when label text shows "..." and is updated
>
> Hi Krishna,
>
> On 11/16/2017 03:38 AM, Krishna Addepalli wrote:
>> Hi Semyon,
>>
>> 1. I have tried in my Ubuntu 17.04 VM and was able to see the test case fail before fix, and passing after the fix.
>> 2. Regarding the minimum size, it is not related to any layout - the test fails across any Layout - I have verified this in debugging process. The only point is, whenever the label text / any other component rendering width is larger the window size, it requests all the components to report the minimum size. In the case of JMenu (which is a top level menu), it is better to return a preferred size rather than minimum size (which by the way is 1x1), so that the item is rendered correctly. Returning null from the fixed function will cause the layout class to return the default minimum value as per the layout class used. So, I didn’t see any reason to come up with arbitrary minimum values.
> I don't see this bug as a Menu issue because any other component that has minimumSize != prefferredSize placed instead of the menu causes the same unexpected layout behavior.
> The heights of all components should be preserved with the layout width change but the the upper component jumps to its minimum size after the width of the window reaches some value.
>
> --Semyon
>> Thanks,
>> Krishna
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Thursday, November 16, 2017 6:51 AM
>> To: Semyon Sadetsky <[hidden email]>; Krishna Addepalli
>> <[hidden email]>; [hidden email]
>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>> flickers when label text shows "..." and is updated
>>
>> On 15/11/2017 10:53, Semyon Sadetsky wrote:
>>>> There are no such restrictions that getMinimumSize() must return
>>>> exact value of getPrefferredSize(). It should return some
>>>> reasonable value instead of null, which can be considered as 1x1 by
>>>> some layout managers. In most cases the PrefferredSize is used for this purpose.
>>> This doesn't correspond to what I see if add
>>>
>>> menu.setMinimumSize(new Dimension(2,2));
>>>
>>> After that the problem described in the bug comes back.
>> This will not be a bug, if the application thinks that 2x2 or 200x200 are reasonable values for maximum/minimum/preferred sizes and will set it, then these values will be used.
>>
>>>>> --Semyon
>>>>>
>>>>>
>>>>> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>>>>>> Looks fine.
>>>>>>
>>>>>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Per our conversation, I have added the volatile keyword to the
>>>>>>> Boolean flag. Here is the updated webrev:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Krishna Addepalli
>>>>>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>> updated
>>>>>>>
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> I have modified the code as per your requests, and here is the
>>>>>>> updated webrev:
>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>>>>>
>>>>>>> However, I have a couple questions regarding the fix:
>>>>>>> 1. Why was it considered good to fix it in BasicMenuUI rather
>>>>>>> than in BasicMenuUIItem?
>>>>>>> 2. While using Robot.waitForIdle(), I came across the function
>>>>>>> "Suntoolkit.flushPendingEvents" function. But this is a private
>>>>>>> function and applications cannot use this. Is it expected that
>>>>>>> the applications should import the robot module to flush the
>>>>>>> events, considering that mostly it is used for testing purposes?
>>>>>>> If this is the case, then "Suntoolkit.flushPendingEvents"
>>>>>>> becomes a valuable tool, given that Swing rendering happens in
>>>>>>> an EDT that is separate from MainThread.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishna
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Sergey Bylokhov
>>>>>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>> [hidden email]
>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>> updated
>>>>>>>
>>>>>>> Hi, Krishna.
>>>>>>> Small comments: I think that this code
>>>>>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>>>>>> can be simplified to
>>>>>>> (JMenu)menuItem).isTopLevelMenu().
>>>>>>>
>>>>>>> In the test you create and dispose JFrame and other components
>>>>>>> outside of EDT thread. I suggest to use invokeAndWait() which
>>>>>>> will work synchronously. Instead of sleep() you van use
>>>>>>> Robot.waitForIdle();
>>>>>>>
>>>>>>>
>>>>>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Gentle reminder! Could you review the fix?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Krishna Addepalli
>>>>>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>>> updated
>>>>>>>>
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Are the changes fine now?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Krishna Addepalli
>>>>>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>>>>>> To: Sergey Bylokhov <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>> GridBagLayout flickers when label text shows "..." and is
>>>>>>>> updated
>>>>>>>>
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> As per your recommendation I have done the following changes:
>>>>>>>> 1. Moved the fix to BasicMenuUI.
>>>>>>>> 2. Corrected the test to:
>>>>>>>>       a. Access the UI elements only in the EDT.
>>>>>>>>       b. Made sure that the main thread doesn't exit till the
>>>>>>>> test is complete.
>>>>>>>>
>>>>>>>> Here is the updated webrev:
>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>>>>>
>>>>>>>> However, I was wondering, why it is good to fix in BasicMenuUI?
>>>>>>>> Also, while writing the testcase, I couldnot find any
>>>>>>>> functionality like "flush" on the event queue, which will
>>>>>>>> execute all the pending events. Is it by design?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Krishna
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Sergey Bylokhov
>>>>>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>>> [hidden email]
>>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>> GridBagLayout flickets when label text shows "..." and is
>>>>>>>> updated
>>>>>>>>
>>>>>>>> Hi, Krishna.
>>>>>>>> A few notes:
>>>>>>>>       - Looks like the bug can be reproduced in JMenu only?
>>>>>>>> Then it will be good to fix it in BasicMenuUI, moreover it is
>>>>>>>> already implemented a
>>>>>>>> getMaximumSize() in a similar way as in your fix.
>>>>>>>>       - In the test some of the Swing components are accessed
>>>>>>>> on non-EDT thread. Also note that when you start the Thread you
>>>>>>>> will exit
>>>>>>>> invokeAndWait() and it is possible that the test will end
>>>>>>>> before the Thread complete(jtreg will kill the test when the
>>>>>>>> main thread is
>>>>>>>> completed)
>>>>>>>>
>>>>>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>>>>>> Hi Sergey,
>>>>>>>>>
>>>>>>>>> Thanks for pointing that out. Fixed the test case and created
>>>>>>>>> a new webrev here:
>>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>>>>>
>>>>>>>>> Now, the testcase throws an exception and also after a few
>>>>>>>>> trials, closes on its own.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Krishna
>>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Sergey Bylokhov
>>>>>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>>>>>> To: Krishna Addepalli <[hidden email]>;
>>>>>>>>> [hidden email]
>>>>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in
>>>>>>>>> GridBagLayout flickets when label text shows "..." and is
>>>>>>>>> updated
>>>>>>>>>
>>>>>>>>> Hi, Krishna.
>>>>>>>>> Can you please clarify in what situation the test will stop
>>>>>>>>> working(it does not have any assertions)?
>>>>>>>>>
>>>>>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>>>>>> Hi All,
>>>>>>>>>>
>>>>>>>>>> Please review the fix for bug:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Bug: JDK-8178430:
>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>>>>>
>>>>>>>>>> Webrev:
>>>>>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>>>>>
>>>>>>>>>> Summary:
>>>>>>>>>>
>>>>>>>>>> The issue is when the label text width is more than the
>>>>>>>>>> container(JPanel) width, the container tries to render with
>>>>>>>>>> minimum width for all the components. In such case, the
>>>>>>>>>> JMenuItem, which is added to the JMenuBar also returns its
>>>>>>>>>> height dimension as 1 (the default minimum). The test case
>>>>>>>>>> alternates between the short text and long text on the label,
>>>>>>>>>> and it gives a flickering effect of the Menu.
>>>>>>>>>> The fix is to return preferred size from JMenuItem, if its
>>>>>>>>>> parent is a JMenuBar, since JMenuBar is added to the top
>>>>>>>>>> level window.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Krishna
>>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Best regards, Sergey.
>>>>>>>>>
>>>>>>>> --
>>>>>>>> Best regards, Sergey.
>>>>>>>>
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>>>>
>> --
>> Best regards, Sergey.