JDK-8303950: [macos]Translucent Windows Flicker on Repaint #14363

mickleness · 2023-06-07T17:15:50Z

Problem Summary

For non-opaque windows, Window#paint calls gg.fillRect(0, 0, getWidth(), getHeight()) before super.paint(g).

This can cause flickering on Mac, and the flickering seems to have gotten much worse in recent JVMs. (See movie attachments to original ticket.)

Discussion

This is my 2nd PR for this ticket. The original is here; that proposed change was IMO more invasive/scarier. It was auto-closed after 8 weeks of inactivity, and I'd rather offer this PR instead.

In that previous discussion Alan Snyder framed the core problem as a "lack of synchronization" (see comment here). If the monitor refreshes/flushes after we've called fillRect but before we finish super.paint: it makes sense that we'd see a flicker.

I agree with Alan, but I think the problem can also be framed as a mixing-Swing-with-AWT issue. (Which IMO will involve an easier fix.)

This PR is a low-risk change (relative to the previous PR) that intercepts calls to repaint a Window that is also RootPaneContainer. Now we'll redirect those calls to paint the JRootPane instead. This means we'll exclusively paint within Swing's/RepaintManager's double-buffered architecture, so we bypass the risky call to fillRect on the screen's Graphics2D. (And this change occurs within RepaintManager, so we're clearly already in Swing's architecture.)

So with this change: we paint everything to the double-buffer, and the only time we paint to the Window's Graphics2D is when have set up a AlphaComposite.Src and replace its contents with our buffer's contents.

Tests

This PR includes a new test for 8303950 itself. This is pretty self-explanatory: we repaint a trivial animation for a few seconds and use the Robot to see if a pixel is the expected color.

This PR also includes a test called bug8303950_legacyWindowPaintBehavior that creates a grid of 4 windows with varying opacity/backgrounds:

I was surprised by how these windows rendered, but I don't think that's worth debating here. This test simply makes sure that we preserve this preexisting behavior. The broad "rules" appear to be:

If a JComponent identifies as opaque (see JComponent.isOpaque) then the JComponent's background is used. (In this case: that's the opaque green top two windows.) This is probably coming from ComponentUI.update
If a JRootPane has a translucent background, that color "wins". It doesn't composite on top of a window's translucent background: it simply replaces it. (See the blue-ish bottom-right window. Note it's blue -- not purple.) This is probably because the RepaintManager uses AlphaComposite.Src in a couple of places to replace its destination Graphics.
If a JRootPane has a null background color, then the Window's background color paints as expected. (See the red-ish bottom-left window.)

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Integration blocker

⚠️ Title mismatch between PR and JBS for issue JDK-8303950

Issue

JDK-8303950: [macos] Translucent Windows Flicker on Repaint (Bug - P3) ⚠️ Title mismatch between PR and JBS.

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/14363/head:pull/14363
$ git checkout pull/14363

Update a local copy of the PR:
$ git checkout pull/14363
$ git pull https://git.openjdk.org/jdk.git pull/14363/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 14363

View PR using the GUI difftool:
$ git pr show -t 14363

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/14363.diff

Webrev

Link to Webrev Comment

Merge openjdk/jdk into mickleness/jdk

Merge openjdk/jdk

Merge from openjdk/jdk

Updating mickleness/jdk from openjdk/jdk

Apparently calling System.exit(0) was coming across as a failing condition. So instead I'll just let the app exit on its own. Also small formatting/typo tweaks.

This test passes in the current JDK. The test criteria don't really reflect a rational expected behavior; they just reflect the current status quo.

… behavior One way of framing bug8303950 is: that problem had to do with mixing non-Swing and Swing components. This adds special behavior for RootPaneContainers to address this issue. With this approach the results of bug8303950_legacyWindowPaintBehavior are unchanged.

Since the bottom two windows are translucent: it will help safeguard test results if they're positioned above an opaque white background. Also adding a couple of text lines to the windows to help explain why they rendered the way they did.

mrserb recommended against this in a separate PR openjdk#13408 (comment)

The resolution is working, but the unit test itself isn't always synchronizing well. I'm trying to avoid baking in a permanent timer (for ex: waiting 2 seconds to let the UI load), but if I see more intermittent failures I may resort to that yet.

toBack() moves the Window behind *everything*. If it moved windows only behind other windows in the Swing app that would be great, but that's not what it does.

bridgekeeper · 2023-06-07T17:25:42Z

👋 Welcome back mickleness! 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 · 2023-06-07T17:48:57Z

@mickleness The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2023-06-07T17:53:42Z

Webrevs

prrace · 2023-06-07T19:09:10Z

I haven't yet looked at this properly but I'm mightily relieved to see that previous patch gone since
as you say it did look risky and intrusive and I didn't think anyone looking at it would know what it
was intended to solve without plenty of background .. which is often a clue that there's a better fix.

We'll need to test this and since it sounds like you don't have much in the way of testing resources we'll have to see
if we can do that here.
But before we get to that, your tests need "@key headful" .. else the test infrastructure won't know it needs a display.

Based on prrace's feedback openjdk#14363 (comment)

mlbridge · 2023-06-08T05:23:31Z

Mailing list message from Alan Snyder on client-libs-dev:

Are you able to test on platforms other than macOS? (I?m not, so I can?t help with that.)

Alan

mlbridge · 2023-06-08T05:23:33Z

Mailing list message from Jeremy Wood on client-libs-dev:

Alan,

No, I?m not currently set up to test OpenJDK PR?s on other platforms.

I?m fuzzy on exactly what happens automatically in an OpenJDK PR. Are my
new unit tests automatically run on a variety of platforms? (That is:
are my new unit tests part of what executes here
<https://github.com/mickleness/jdk/actions/runs/5197802507/jobs/9386648003>?
Or does that skip several UI-based unit tests?)

I do have a Windows machine, but IIRC setting up OpenJDK to build on my
Mac was difficult, and I don?t want to go through that again on Windows
unless I really have to.

- Jeremy

------ Original Message ------
From "Alan Snyder" <javalists at cbfiddle.com>
To "Jeremy" <duke at openjdk.org>
Cc client-libs-dev at openjdk.org
Date 6/7/23, 2:06:23 PM
Subject Re: RFR: JDK-8303950: [macos]Translucent Windows Flicker on
Repaint

Are you able to test on platforms other than macOS? (I?m not, so I can?t help with that.)

Alan

mlbridge · 2023-06-08T05:23:35Z

Mailing list message from Philip Race on client-libs-dev:

if you are thinking of github actions exactly ZERO client tests are run.
The testing is all on you.

-phil.

On 6/7/23 11:58 AM, Jeremy Wood wrote:

Alan,

No, I?m not currently set up to test OpenJDK PR?s on other platforms.

I?m fuzzy on exactly what happens automatically in an OpenJDK PR. Are
my new unit tests automatically run on a variety of platforms? (That
is: are my new unit tests part of what executes here
<https://github.com/mickleness/jdk/actions/runs/5197802507/jobs/9386648003>?
Or does that skip several UI-based unit tests?)

I do have a Windows machine, but IIRC setting up OpenJDK to build on
my Mac was difficult, and I don?t want to go through that again on
Windows unless I really have to.

?- Jeremy

------ Original Message ------
From "Alan Snyder" <javalists at cbfiddle.com>
To "Jeremy" <duke at openjdk.org>
Cc client-libs-dev at openjdk.org
Date 6/7/23, 2:06:23 PM
Subject Re: RFR: JDK-8303950: [macos]Translucent Windows Flicker on
Repaint

Are you able to test on platforms other than macOS?? (I?m not, so I
can?t help with that.)

? Alan

turbanoff · 2023-06-16T20:14:06Z

test/jdk/javax/swing/RepaintManager/8303950/bug8303950_legacyWindowPaintBehavior.java

+                Window w1 = createWindow( WINDOW_BACKGROUND, null, x, y, 400, 400, false, "window 1");
+                Window w2 = createWindow( WINDOW_BACKGROUND, ROOTPANE_BACKGROUND, x + 400, y, 400, 400, false, "window 2");
+                Window w3 = createWindow( WINDOW_BACKGROUND, null, x, y + 400, 400, 400, true, "window 3");
+                Window w4 =  createWindow( WINDOW_BACKGROUND, ROOTPANE_BACKGROUND, x + 400, y + 400, 400, 400, true, "window 4");


Suggested change

Window w4 = createWindow( WINDOW_BACKGROUND, ROOTPANE_BACKGROUND, x + 400, y + 400, 400, 400, true, "window 4");

Window w4 = createWindow( WINDOW_BACKGROUND, ROOTPANE_BACKGROUND, x + 400, y + 400, 400, 400, true, "window 4");

See openjdk#14363 (review)

bridgekeeper · 2023-07-15T01:41:47Z

@mickleness This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

mickleness · 2023-07-17T03:39:39Z

@aivanov-jdk IIRC this ticket (a P3) was initially assigned to you, and I asked to look at it instead. It looks like for now this PR is stalled, so please feel free to reassign the openJDK ticket (or make any other appropriate changes) if needed.

aivanov-jdk · 2023-07-21T18:54:06Z

@aivanov-jdk IIRC this ticket (a P3) was initially assigned to you, and I asked to look at it instead. It looks like for now this PR is stalled, so please feel free to reassign the openJDK ticket (or make any other appropriate changes) if needed.

@mickleness It's not assigned to me now. I can't even find that it was assigned to me.

mickleness · 2023-07-21T20:07:57Z

OK, maybe I'm mistaken. Thanks for checking.

bridgekeeper · 2023-08-18T23:00:05Z

@mickleness This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper · 2023-09-15T23:51:35Z

@mickleness This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

bric3 · 2023-09-20T07:52:33Z

I encountered an issue very similar to this problem, is there any chance to see progress on the fix ?

aivanov-jdk · 2023-09-20T11:14:07Z

I encountered an issue very similar to this problem, is there any chance to see progress on the fix ?

@bric3 This is not how it works… Yet feel free to propose a patch that addresses the problem. It could be based on this PR.

I looks @mickleness couldn't run the tests.

I haven't looked into the code changes or tests…

bric3 · 2023-09-20T11:19:05Z

Yes, I was more thinking about discussing the patch and PR than expecting the PR to be merged right away, since the last comments don't really address the technical issue.

mickleness · 2023-09-20T20:15:18Z

I'm happy to help with/discuss anything if anyone has any questions. This PR currently resolves the original issue I was seeing and it preserves the legacy painting behavior. Tested on Mac.

mickleness and others added 12 commits May 22, 2022 04:50

Merge pull request #1 from openjdk/master

5dbf0a6

Merge openjdk/jdk into mickleness/jdk

Merge pull request #2 from openjdk/master

333c87c

Merge openjdk/jdk

Merge pull request #3 from openjdk/master

57346a4

Merge from openjdk/jdk

Merge pull request #4 from openjdk/master

69481fc

Updating mickleness/jdk from openjdk/jdk

8303950: adding failing test to identify problem

55caec2

8303950: fixing false negative

ca9ea41

Apparently calling System.exit(0) was coming across as a failing condition. So instead I'll just let the app exit on its own. Also small formatting/typo tweaks.

8303950: adding unit test for legacy window behavior

946fcd8

This test passes in the current JDK. The test criteria don't really reflect a rational expected behavior; they just reflect the current status quo.

8303950: avoid System.exit(1)

0c7ba0b

mrserb recommended against this in a separate PR openjdk#13408 (comment)

8303950: remove call to Window#toBack()

10f93e5

toBack() moves the Window behind *everything*. If it moved windows only behind other windows in the Swing app that would be great, but that's not what it does.

openjdk bot added the rfr Pull request is ready for review label Jun 7, 2023

openjdk bot added the client client-libs-dev@openjdk.org label Jun 7, 2023

8303950: Adding "@key headful"

2c09321

Based on prrace's feedback openjdk#14363 (comment)

turbanoff reviewed Jun 16, 2023

View reviewed changes

8303950: removing unnecessary whitespace in unit test

12d8950

See openjdk#14363 (review)

bridgekeeper bot closed this Sep 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK-8303950: [macos]Translucent Windows Flicker on Repaint #14363

JDK-8303950: [macos]Translucent Windows Flicker on Repaint #14363

mickleness commented Jun 7, 2023 •

edited by openjdk bot

bridgekeeper bot commented Jun 7, 2023

openjdk bot commented Jun 7, 2023

mlbridge bot commented Jun 7, 2023 •

edited

prrace commented Jun 7, 2023

mlbridge bot commented Jun 8, 2023

mlbridge bot commented Jun 8, 2023

mlbridge bot commented Jun 8, 2023

turbanoff Jun 16, 2023

bridgekeeper bot commented Jul 15, 2023

mickleness commented Jul 17, 2023

aivanov-jdk commented Jul 21, 2023

mickleness commented Jul 21, 2023

bridgekeeper bot commented Aug 18, 2023

bridgekeeper bot commented Sep 15, 2023

bric3 commented Sep 20, 2023

aivanov-jdk commented Sep 20, 2023

bric3 commented Sep 20, 2023 •

edited

mickleness commented Sep 20, 2023

	Window w4 = createWindow( WINDOW_BACKGROUND, ROOTPANE_BACKGROUND, x + 400, y + 400, 400, 400, true, "window 4");
	Window w4 = createWindow( WINDOW_BACKGROUND, ROOTPANE_BACKGROUND, x + 400, y + 400, 400, 400, true, "window 4");

JDK-8303950: [macos]Translucent Windows Flicker on Repaint #14363

JDK-8303950: [macos]Translucent Windows Flicker on Repaint #14363

Conversation

mickleness commented Jun 7, 2023 • edited by openjdk bot

Problem Summary

Discussion

Tests

Progress

Integration blocker

Issue

Reviewing

Webrev

bridgekeeper bot commented Jun 7, 2023

openjdk bot commented Jun 7, 2023

mlbridge bot commented Jun 7, 2023 • edited

Webrevs

prrace commented Jun 7, 2023

mlbridge bot commented Jun 8, 2023

mlbridge bot commented Jun 8, 2023

mlbridge bot commented Jun 8, 2023

turbanoff Jun 16, 2023

Choose a reason for hiding this comment

bridgekeeper bot commented Jul 15, 2023

mickleness commented Jul 17, 2023

aivanov-jdk commented Jul 21, 2023

mickleness commented Jul 21, 2023

bridgekeeper bot commented Aug 18, 2023

bridgekeeper bot commented Sep 15, 2023

bric3 commented Sep 20, 2023

aivanov-jdk commented Sep 20, 2023

bric3 commented Sep 20, 2023 • edited

mickleness commented Sep 20, 2023

mickleness commented Jun 7, 2023 •

edited by openjdk bot

mlbridge bot commented Jun 7, 2023 •

edited

bric3 commented Sep 20, 2023 •

edited