820839 - Lightweight theme previews get out of sync between titlebar and the rest of chrome

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Deb Richardson [:dria] (plz NEEDINFO)]

Attachment: lwt breakage 1

I noticed this when doing a bunch of mouseover previews of new Lightweight themes on AMO -- the theme displayed in the titlebar often didn't update properly so was displaying a previously viewed theme rather than the current them.

Attached images show what I mean.

I replicated this in Beta, Aurora, and Nightly but not the current final -- so Fx 18, 19, and 20 I guess.

Comment 1

[Deb Richardson [:dria] (plz NEEDINFO)]

Attachment: lwt breakage 2

Comment 2

[Deb Richardson [:dria] (plz NEEDINFO)]

Attachment: lwt breakage 3

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Hmm, related to bug 650968 potentially? That got fixed for 18.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

STR:
1) install a persona
2) on getpersonas.com, hover quickly over the various previews.

1) is key, it doesn't seem to happen otherwise.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

I can replicate this but the titlebar and the rest of chrome are only temporarily out of sync here. Is this permanent for you?

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Ok it's now permanent here as well, sometimes.

Comment 7

[Justin Dolske [:Dolske]]

I'd have guessed bug 539356. I can reproduce this too, and resizing the window to force it to repaint fixes it.

Comment 8

[Matt Woodrow (:mattwoodrow)]

This is caused by us painting the ThebesLayers for the content area, and clearing any invalidation state in the process.

We don't paint the titlebar area until we composite the window, and the invalidation state that marked the images/colors has been removed.

This seems like a fairly big problem with using two LayerManagers to paint the same set of content (even if the actual painted areas don't overlap), thinking about possible solutions.

Comment 9

[Matt Woodrow (:mattwoodrow)]

I can think of two options:

1) Add a second set of invalidation state bits (NS_FRAME_NEEDS_PAINT_SECONDARY etc), set both when we mark frames as invalid and clear them in separate passes.

Pretty horrible, but it's simple and low risk.

2) Make the titlebar LayerManager retain content between paints. This would let us separate painting ThebesLayers from compositing, and let us determine when to do the former. We could then paint this at the same time as the content area, before we clear the invalidation state.

Lot more complex and risky, but a much nicer solution.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can we composite the titlebar contents to a buffer during/just after the layer tree update? That would mean we can paint it at the same time we paint the rest of the window, but we don't have to deal with two layermanagers retaining content for the same frames.

Comment 11

[Matt Woodrow (:mattwoodrow)]

Attachment: Retain final buffer for titlebar layer manager

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 702125 [details] [diff] [review]
Retain final buffer for titlebar layer manager

Review of attachment 702125 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/nsIWidget.h
@@ +1647,5 @@
> +    /**
> +     * This function is called when the retained ThebesLayers for this
> +     * widget are about to be painted.
> +     */
> +    virtual void WillPaint() { }

rev nsIWidget IID

Called by who? Why? The comment needs more. Especially to make it clear it's called before we do a layer tree update. (As written it could mean "painted to the widget".)

Comment 13

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/29288a6d1876

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/29288a6d1876

Comment 15

[(no longer active)]

Matt, roc, can we back this out for now?  Bug 831829 looks pretty bad, and highly visible.

Comment 16

[Stephen A Pohl [:spohl]]

Bug 831829 seems to be caused by the change in nsCocoaWindow.mm. Is this a necessary change to fix the issue reported in this bug?

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes.

Bug 831829 is fixed on inbound already.