<AWT Dev> [9] Review Request: 8176448 [macos] Popups in JCombobox and Choice have incorrect location in multiscreen systems

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

<AWT Dev> [9] Review Request: 8176448 [macos] Popups in JCombobox and Choice have incorrect location in multiscreen systems

Sergey Bylokhov
Hello,
Please review the fix for jdk9.

This bug was caused by the change in JDK8137571 where we started to use graphic configuration for each component to calculate the preferred size on the screen.
The problem is that in the code we had a few assumptions that the left-top corner of the screen is 0.0. Moreover for the Menu we do not update the GC when the window is moved to another screen(I’ll file a separate bug for this).

Fix description:
 - Menu components now tries to take the graphics configuration from the frame.
 - All calculations are updated to take into account that the left-top corner is screenBounds.x/y.
 - The choice component now updates the GC of its popup when its own gc is updated.
 - I intentionally do not reformat the change to simplify review.

I have started the jck/jtreg tests, they are still in progress I'll notify if some new issues will be found.

Bug: https://bugs.openjdk.java.net/browse/JDK-8160270
Webrev can be found at: http://cr.openjdk.java.net/~serb/8160270/webrev.00
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> [9] Review Request: 8160270 dual-screen issue with java.awt.Choice

Sergey Bylokhov
Hello,
Please review the fix for jdk9.

This bug was caused by the change in JDK8137571 where we started to use graphic configuration for each component to calculate the preferred size on the screen.
The problem is that in the code we had a few assumptions that the left-top corner of the screen is 0.0. Moreover for the Menu we do not update the GC when the window is moved to another screen(I’ll file a separate bug for this).

Fix description:
- Menu components now tries to take the graphics configuration from the frame.
- All calculations are updated to take into account that the left-top corner is screenBounds.x/y.
- The choice component now updates the GC of its popup when its own gc is updated.
- I intentionally do not reformat the change to simplify review.

I have started the jck/jtreg tests, they are still in progress I'll notify if some new issues will be found.

Bug: https://bugs.openjdk.java.net/browse/JDK-8160270
Webrev can be found at: http://cr.openjdk.java.net/~serb/8160270/webrev.00

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> [9] Review Request: 8160270 dual-screen issue with java.awt.Choice

Alexandr Scherbatiy

The fix looks good to me.

Thanks,
Alexandr.

On 3/16/2017 7:44 PM, Sergey Bylokhov wrote:

> Hello,
> Please review the fix for jdk9.
>
> This bug was caused by the change in JDK8137571 where we started to use graphic configuration for each component to calculate the preferred size on the screen.
> The problem is that in the code we had a few assumptions that the left-top corner of the screen is 0.0. Moreover for the Menu we do not update the GC when the window is moved to another screen(I’ll file a separate bug for this).
>
> Fix description:
> - Menu components now tries to take the graphics configuration from the frame.
> - All calculations are updated to take into account that the left-top corner is screenBounds.x/y.
> - The choice component now updates the GC of its popup when its own gc is updated.
> - I intentionally do not reformat the change to simplify review.
>
> I have started the jck/jtreg tests, they are still in progress I'll notify if some new issues will be found.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8160270
> Webrev can be found at: http://cr.openjdk.java.net/~serb/8160270/webrev.00
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> [9] Review Request: 8160270 dual-screen issue with java.awt.Choice

Phil Race
It all looks plausible .. so +1 assuming no problems with the tests.

-phil.

On 03/16/2017 12:20 PM, Alexandr Scherbatiy wrote:

>
> The fix looks good to me.
>
> Thanks,
> Alexandr.
>
> On 3/16/2017 7:44 PM, Sergey Bylokhov wrote:
>> Hello,
>> Please review the fix for jdk9.
>>
>> This bug was caused by the change in JDK8137571 where we started to
>> use graphic configuration for each component to calculate the
>> preferred size on the screen.
>> The problem is that in the code we had a few assumptions that the
>> left-top corner of the screen is 0.0. Moreover for the Menu we do not
>> update the GC when the window is moved to another screen(I’ll file a
>> separate bug for this).
>>
>> Fix description:
>> - Menu components now tries to take the graphics configuration from
>> the frame.
>> - All calculations are updated to take into account that the left-top
>> corner is screenBounds.x/y.
>> - The choice component now updates the GC of its popup when its own
>> gc is updated.
>> - I intentionally do not reformat the change to simplify review.
>>
>> I have started the jck/jtreg tests, they are still in progress I'll
>> notify if some new issues will be found.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8160270
>> Webrev can be found at:
>> http://cr.openjdk.java.net/~serb/8160270/webrev.00
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> [9] Review Request: 8160270 dual-screen issue with java.awt.Choice

Sergey Bylokhov
No issues in the awt tests, but I decide to update the fix in place.
One line was added to check that we access the native peers only for HW components.
 657     GraphicsConfiguration getCurrentGraphicsConfiguration() {
+658         Component hw = SunToolkit.getHeavyweightComponent(target);
 659         XWindow peer = AWTAccessor.getComponentAccessor().getPeer(hw);
 660         if (peer != null && peer.graphicsConfig != null) {
 661             return peer.graphicsConfig;
 662         }
 663         return graphicsConfig;
 664     }
If there are no objections I’ll push this version.

:

It all looks plausible .. so +1 assuming no problems with the tests.

-phil.

On 03/16/2017 12:20 PM, Alexandr Scherbatiy wrote:

The fix looks good to me.

Thanks,
Alexandr.

On 3/16/2017 7:44 PM, Sergey Bylokhov wrote:
Hello,
Please review the fix for jdk9.

This bug was caused by the change in JDK8137571 where we started to use graphic configuration for each component to calculate the preferred size on the screen.
The problem is that in the code we had a few assumptions that the left-top corner of the screen is 0.0. Moreover for the Menu we do not update the GC when the window is moved to another screen(I’ll file a separate bug for this).

Fix description:
- Menu components now tries to take the graphics configuration from the frame.
- All calculations are updated to take into account that the left-top corner is screenBounds.x/y.
- The choice component now updates the GC of its popup when its own gc is updated.
- I intentionally do not reformat the change to simplify review.

I have started the jck/jtreg tests, they are still in progress I'll notify if some new issues will be found.

Bug: https://bugs.openjdk.java.net/browse/JDK-8160270
Webrev can be found at: http://cr.openjdk.java.net/~serb/8160270/webrev.00




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> [9] Review Request: 8160270 dual-screen issue with java.awt.Choice

Alexandr Scherbatiy

The fix looks good to me.

Thanks,
Alexandr.

On 3/17/2017 8:19 AM, Sergey Bylokhov wrote:
No issues in the awt tests, but I decide to update the fix in place.
One line was added to check that we access the native peers only for HW components.
 657     GraphicsConfiguration getCurrentGraphicsConfiguration() {
+658         Component hw = SunToolkit.getHeavyweightComponent(target);
 659         XWindow peer = AWTAccessor.getComponentAccessor().getPeer(hw);
 660         if (peer != null && peer.graphicsConfig != null) {
 661             return peer.graphicsConfig;
 662         }
 663         return graphicsConfig;
 664     }
If there are no objections I’ll push this version.

:

It all looks plausible .. so +1 assuming no problems with the tests.

-phil.

On 03/16/2017 12:20 PM, Alexandr Scherbatiy wrote:

The fix looks good to me.

Thanks,
Alexandr.

On 3/16/2017 7:44 PM, Sergey Bylokhov wrote:
Hello,
Please review the fix for jdk9.

This bug was caused by the change in JDK8137571 where we started to use graphic configuration for each component to calculate the preferred size on the screen.
The problem is that in the code we had a few assumptions that the left-top corner of the screen is 0.0. Moreover for the Menu we do not update the GC when the window is moved to another screen(I’ll file a separate bug for this).

Fix description:
- Menu components now tries to take the graphics configuration from the frame.
- All calculations are updated to take into account that the left-top corner is screenBounds.x/y.
- The choice component now updates the GC of its popup when its own gc is updated.
- I intentionally do not reformat the change to simplify review.

I have started the jck/jtreg tests, they are still in progress I'll notify if some new issues will be found.

Bug: https://bugs.openjdk.java.net/browse/JDK-8160270
Webrev can be found at: http://cr.openjdk.java.net/~serb/8160270/webrev.00





Loading...