On Wed, 7 Jun 2023 19:19:59 GMT, Jeremy <[email protected]> wrote:
>> # 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](https://github.com/openjdk/jdk/pull/12993); 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](https://github.com/openjdk/jdk/pull/12993#issuecomment-1467528061)).
>> 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:
>>
>> <img width="805" alt="image"
>> src="https://github.com/openjdk/jdk/assets/7669569/497c21a0-ed18-413f-a5c7-3ea146644fd6";>
>>
>> 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:
>> 1. If a JComponent identifies as opaque (see `JComponent.isOpaque`) then the
>> JComponent's background is used. (I...
>
> Jeremy has updated the pull request incrementally with two additional commits
> since the last revision:
>
> - 8303950: Adding "@key headful"
>
> Based on prrace's feedback
> https://github.com/openjdk/jdk/pull/14363#issuecomment-1581365662
> - 8303950: remove call to Window#toBack()
>
> 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.
test/jdk/javax/swing/RepaintManager/8303950/bug8303950_legacyWindowPaintBehavior.java
line 68:
> 66: Window w2 = createWindow( WINDOW_BACKGROUND,
> ROOTPANE_BACKGROUND, x + 400, y, 400, 400, false, "window 2");
> 67: Window w3 = createWindow( WINDOW_BACKGROUND, null, x, y +
> 400, 400, 400, true, "window 3");
> 68: Window w4 = createWindow( WINDOW_BACKGROUND,
> ROOTPANE_BACKGROUND, x + 400, y + 400, 400, 400, true, "window 4");
Suggestion:
Window w4 = createWindow( WINDOW_BACKGROUND,
ROOTPANE_BACKGROUND, x + 400, y + 400, 400, 400, true, "window 4");
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14363#discussion_r1232733209
