Skip to content
This repository has been archived by the owner. It is now read-only.

8247332: Checkbox reaches inconsistent state on tap to click #146

Closed
wants to merge 7 commits into from

Conversation

@jayathirthrao
Copy link
Member

@jayathirthrao jayathirthrao commented Dec 18, 2020

This fix implements usage of CVDisplayLink for blitting content into CAMetalLayer and solves many event based state issues. Detailed analysis in JBS bug.


Progress

  • Change must not contain extraneous whitespace

Issues

  • JDK-8247332: Checkbox reaches inconsistent state on tap to click
  • JDK-8251477: java/awt/Frame/MiscUndecorated/RepaintTest.java fails with metal pipeline
  • JDK-8247739: Lanai : SwingSet2Demo -More than one radio button can be selected the same time
  • JDK-8252501: Sometimes more than one menu items are shown as highlighted on hover
  • JDK-8242187: Lanai - JCheckBox and JRadioButton can get into inconsitent visual state -- but recover on window refresh
  • JDK-8252907: Motif L&F - SwingSet2 - Two Toolbar buttons are shown as selected

Download

$ git fetch https://git.openjdk.java.net/lanai pull/146/head:pull/146
$ git checkout pull/146

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Dec 18, 2020

👋 Welcome back jdv! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

@openjdk openjdk bot commented Dec 18, 2020

@jayathirthrao This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8247332: Checkbox reaches inconsistent state on tap to click
8251477: java/awt/Frame/MiscUndecorated/RepaintTest.java fails with metal pipeline
8247739: Lanai : SwingSet2Demo -More than one radio button can be selected the same time
8252501: Sometimes more than one menu items are shown as highlighted on hover
8242187: Lanai - JCheckBox and JRadioButton can get into inconsitent visual state -- but recover on window refresh
8252907: Motif L&F - SwingSet2 - Two Toolbar buttons are shown as selected

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 57 new commits pushed to the master branch:

  • 5c53e62: Automatic merge of jdk:master into master
  • 3f77a60: 8258582: HttpClient: the HttpClient doesn't explicitly shutdown its default executor when stopping.
  • f15528e: Merge
  • 30ca0a5: 8247994: Localize javadoc search
  • 47c180d: 8258515: javac should issue an error if an annotation is nested in a local class or interface
  • cb5a6b1: 8258225: compiler/c2/cr6340864/TestIntVect.java runs faster in interpreter
  • 61cbf0f: 8258293: tools/jpackage/share/RuntimePackageTest.java#id0 with RuntimePackageTest.testUsrInstallDir2
  • 7aac4dc: 8257621: JFR StringPool misses cached items across consecutive recordings
  • 61390d8: 8257999: Parallel GC crash in gc/parallel/TestDynShrinkHeap.java: new region is not in covered_region
  • 952dc70: 8257636: Update usage of "type" terminology in java.lang.Class and java.lang.reflect
  • ... and 47 more: https://git.openjdk.java.net/lanai/compare/a47c1df13d9ffeb64fad0b1209093a885842d7c6...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@mlbridge
Copy link

@mlbridge mlbridge bot commented Dec 18, 2020

Webrevs

@jayathirthrao
Copy link
Member Author

@jayathirthrao jayathirthrao commented Dec 18, 2020

/issue add 8251477,8247739,8252501,8242187,8252907

@openjdk
Copy link

@openjdk openjdk bot commented Dec 18, 2020

@jayathirthrao
Adding additional issue to issue list: 8251477: java/awt/Frame/MiscUndecorated/RepaintTest.java fails with metal pipeline.

Adding additional issue to issue list: 8247739: Lanai : SwingSet2Demo -More than one radio button can be selected the same time.

Adding additional issue to issue list: 8252501: Sometimes more than one menu items are shown as highlighted on hover.

Adding additional issue to issue list: 8242187: Lanai - JCheckBox and JRadioButton can get into inconsitent visual state -- but recover on window refresh.

Adding additional issue to issue list: 8252907: Motif L&F - SwingSet2 - Two Toolbar buttons are shown as selected.

@jayathirthrao
Copy link
Member Author

@jayathirthrao jayathirthrao commented Dec 18, 2020

/integrate

@openjdk openjdk bot closed this Dec 18, 2020
@openjdk openjdk bot added integrated and removed ready rfr labels Dec 18, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Dec 18, 2020

@jayathirthrao Since your change was applied there have been 57 commits pushed to the master branch:

  • 5c53e62: Automatic merge of jdk:master into master
  • 3f77a60: 8258582: HttpClient: the HttpClient doesn't explicitly shutdown its default executor when stopping.
  • f15528e: Merge
  • 30ca0a5: 8247994: Localize javadoc search
  • 47c180d: 8258515: javac should issue an error if an annotation is nested in a local class or interface
  • cb5a6b1: 8258225: compiler/c2/cr6340864/TestIntVect.java runs faster in interpreter
  • 61cbf0f: 8258293: tools/jpackage/share/RuntimePackageTest.java#id0 with RuntimePackageTest.testUsrInstallDir2
  • 7aac4dc: 8257621: JFR StringPool misses cached items across consecutive recordings
  • 61390d8: 8257999: Parallel GC crash in gc/parallel/TestDynShrinkHeap.java: new region is not in covered_region
  • 952dc70: 8257636: Update usage of "type" terminology in java.lang.Class and java.lang.reflect
  • ... and 47 more: https://git.openjdk.java.net/lanai/compare/a47c1df13d9ffeb64fad0b1209093a885842d7c6...master

Your commit was automatically rebased without conflicts.

Pushed as commit 5263974.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@jayathirthrao jayathirthrao deleted the jayathirthrao:JDK-8247332 branch Dec 18, 2020
@mlbridge
Copy link

@mlbridge mlbridge bot commented Dec 22, 2020

Mailing list message from Alexey Ushakov on lanai-dev:

Hi Jay,

I?ve just updated my sources to the latest Lanai version and found one problem - continuous execution of the following methods for the simple static UI application:

[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called

It?s look like waste of CPU time. Did you measured CPU usage before/after the fix?
I think that we still should consider async model for standard (business) GUI applications.
What do you think?

Best Regards,
Alexey

@mlbridge
Copy link

@mlbridge mlbridge bot commented Dec 22, 2020

Mailing list message from Alan Snyder on lanai-dev:

Do your UI by any chance have a blinking caret?

On Dec 22, 2020, at 8:02 AM, Alexey Ushakov <alexey.ushakov at jetbrains.com> wrote:

Hi Jay,

I?ve just updated my sources to the latest Lanai version and found one problem - continuous execution of the following methods for the simple static UI application:

[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called

It?s look like waste of CPU time. Did you measured CPU usage before/after the fix?
I think that we still should consider async model for standard (business) GUI applications.
What do you think?

Best Regards,
Alexey

@mlbridge
Copy link

@mlbridge mlbridge bot commented Dec 22, 2020

Mailing list message from Alexey Ushakov on lanai-dev:

No, It?s just static picture. CVDisplayLink is the reason of the continuous callbacks. It?s a good thing if you do animation, but for async updated model it is not necessary. Probably we should suspend display link (there is an api for this) after receiving all necessary updates.

On 22 Dec 2020, at 19:43, Alan Snyder <javalists at cbfiddle.com> wrote:

Do your UI by any chance have a blinking caret?

On Dec 22, 2020, at 8:02 AM, Alexey Ushakov <alexey.ushakov at jetbrains.com> wrote:

Hi Jay,

I?ve just updated my sources to the latest Lanai version and found one problem - continuous execution of the following methods for the simple static UI application:

[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called

It?s look like waste of CPU time. Did you measured CPU usage before/after the fix?
I think that we still should consider async model for standard (business) GUI applications.
What do you think?

Best Regards,
Alexey

@mlbridge
Copy link

@mlbridge mlbridge bot commented Dec 23, 2020

Mailing list message from Ajit Ghaisas on lanai-dev:

Hi Alexey,

Jay and I have discussed this internally yesterday.
He is on vacation and will address it on his return.

Regards,
Ajit

On 22-Dec-2020, at 10:41 PM, Alexey Ushakov <alexey.ushakov at jetbrains.com> wrote:

No, It?s just static picture. CVDisplayLink is the reason of the continuous callbacks. It?s a good thing if you do animation, but for async updated model it is not necessary. Probably we should suspend display link (there is an api for this) after receiving all necessary updates.

On 22 Dec 2020, at 19:43, Alan Snyder <javalists at cbfiddle.com> wrote:

Do your UI by any chance have a blinking caret?

On Dec 22, 2020, at 8:02 AM, Alexey Ushakov <alexey.ushakov at jetbrains.com> wrote:

Hi Jay,

I?ve just updated my sources to the latest Lanai version and found one problem - continuous execution of the following methods for the simple static UI application:

[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called

It?s look like waste of CPU time. Did you measured CPU usage before/after the fix?
I think that we still should consider async model for standard (business) GUI applications.
What do you think?

Best Regards,
Alexey

@mlbridge
Copy link

@mlbridge mlbridge bot commented Dec 23, 2020

Mailing list message from Alexey Ushakov on lanai-dev:

Thanks, Ajit!

Nevertheless I think that we still can use CVDisplayLink for our purpose (it should help for such things as live resize/animation) but we need to suspend it in the idle state of application.

Best Regards,
Alexey

On 23 Dec 2020, at 12:31, Ajit Ghaisas <ajit.ghaisas at oracle.com> wrote:

Hi Alexey,

Jay and I have discussed this internally yesterday.
He is on vacation and will address it on his return.

Regards,
Ajit

On 22-Dec-2020, at 10:41 PM, Alexey Ushakov <alexey.ushakov at jetbrains.com> wrote:

No, It?s just static picture. CVDisplayLink is the reason of the continuous callbacks. It?s a good thing if you do animation, but for async updated model it is not necessary. Probably we should suspend display link (there is an api for this) after receiving all necessary updates.

On 22 Dec 2020, at 19:43, Alan Snyder <javalists at cbfiddle.com> wrote:

Do your UI by any chance have a blinking caret?

On Dec 22, 2020, at 8:02 AM, Alexey Ushakov <alexey.ushakov at jetbrains.com> wrote:

Hi Jay,

I?ve just updated my sources to the latest Lanai version and found one problem - continuous execution of the following methods for the simple static UI application:

[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called

It?s look like waste of CPU time. Did you measured CPU usage before/after the fix?
I think that we still should consider async model for standard (business) GUI applications.
What do you think?

Best Regards,
Alexey

@mlbridge
Copy link

@mlbridge mlbridge bot commented Dec 23, 2020

Mailing list message from Kevin Rushforth on lanai-dev:

FWIW, we ran into the same thing for JavaFX. We also added logic to
suspend CVDisplayLink when idle:
https://bugs.openjdk.java.net/browse/JDK-8189926

-- Kevin

On 12/23/2020 1:44 AM, Alexey Ushakov wrote:

Thanks, Ajit!

Nevertheless I think that we still can use CVDisplayLink for our purpose (it should help for such things as live resize/animation) but we need to suspend it in the idle state of application.

Best Regards,
Alexey

On 23 Dec 2020, at 12:31, Ajit Ghaisas <ajit.ghaisas at oracle.com> wrote:

Hi Alexey,

Jay and I have discussed this internally yesterday.
He is on vacation and will address it on his return.

Regards,
Ajit

On 22-Dec-2020, at 10:41 PM, Alexey Ushakov <alexey.ushakov at jetbrains.com> wrote:

No, It?s just static picture. CVDisplayLink is the reason of the continuous callbacks. It?s a good thing if you do animation, but for async updated model it is not necessary. Probably we should suspend display link (there is an api for this) after receiving all necessary updates.

On 22 Dec 2020, at 19:43, Alan Snyder <javalists at cbfiddle.com> wrote:

Do your UI by any chance have a blinking caret?

On Dec 22, 2020, at 8:02 AM, Alexey Ushakov <alexey.ushakov at jetbrains.com> wrote:

Hi Jay,

I?ve just updated my sources to the latest Lanai version and found one problem - continuous execution of the following methods for the simple static UI application:

[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called

It?s look like waste of CPU time. Did you measured CPU usage before/after the fix?
I think that we still should consider async model for standard (business) GUI applications.
What do you think?

Best Regards,
Alexey

@mlbridge
Copy link

@mlbridge mlbridge bot commented Jan 4, 2021

Mailing list message from Jayathirth D v on lanai-dev:

Hi Alexey,

Thanks for noticing this, we had discussion regarding the same before year end break.
Thanks Kevin for the pointer.

I have created https://bugs.openjdk.java.net/browse/JDK-8259038 <https://bugs.openjdk.java.net/browse/JDK-8259038> to handle this issue.

Thanks,
Jay

On 23-Dec-2020, at 6:47 PM, Kevin Rushforth <kevin.rushforth at oracle.com> wrote:

FWIW, we ran into the same thing for JavaFX. We also added logic to suspend CVDisplayLink when idle: https://bugs.openjdk.java.net/browse/JDK-8189926

-- Kevin

On 12/23/2020 1:44 AM, Alexey Ushakov wrote:

Thanks, Ajit!

Nevertheless I think that we still can use CVDisplayLink for our purpose (it should help for such things as live resize/animation) but we need to suspend it in the idle state of application.

Best Regards,
Alexey

On 23 Dec 2020, at 12:31, Ajit Ghaisas <ajit.ghaisas at oracle.com> wrote:

Hi Alexey,

Jay and I have discussed this internally yesterday.
He is on vacation and will address it on his return.

Regards,
Ajit

On 22-Dec-2020, at 10:41 PM, Alexey Ushakov <alexey.ushakov at jetbrains.com> wrote:

No, It?s just static picture. CVDisplayLink is the reason of the continuous callbacks. It?s a good thing if you do animation, but for async updated model it is not necessary. Probably we should suspend display link (there is an api for this) after receiving all necessary updates.

On 22 Dec 2020, at 19:43, Alan Snyder <javalists at cbfiddle.com> wrote:

Do your UI by any chance have a blinking caret?

On Dec 22, 2020, at 8:02 AM, Alexey Ushakov <alexey.ushakov at jetbrains.com> wrote:

Hi Jay,

I?ve just updated my sources to the latest Lanai version and found one problem - continuous execution of the following methods for the simple static UI application:

[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called
[V] MTLLayer_blitTexture
[V] MTLLayer_displayLinkCallback() called
[V] MTLLayer_display() called

It?s look like waste of CPU time. Did you measured CPU usage before/after the fix?
I think that we still should consider async model for standard (business) GUI applications.
What do you think?

Best Regards,
Alexey

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
1 participant