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

RESOLVED FIXED in mozilla21

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: deb, Assigned: mattwoodrow)

Tracking

unspecified
mozilla21
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox18 affected)

Details

Attachments

(4 attachments)

Created attachment 691343 [details]
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.
Created attachment 691345 [details]
lwt breakage 2
Created attachment 691346 [details]
lwt breakage 3
Hmm, related to bug 650968 potentially? That got fixed for 18.
Keywords: regressionwindow-wanted
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.
I can replicate this but the titlebar and the rest of chrome are only temporarily out of sync here. Is this permanent for you?
Ok it's now permanent here as well, sometimes.
I'd have guessed bug 539356. I can reproduce this too, and resizing the window to force it to repaint fixes it.
Blocks: 539356
Assignee: nobody → matt.woodrow
(Assignee)

Comment 8

6 years ago
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.
Keywords: regressionwindow-wanted
(Assignee)

Comment 9

6 years ago
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.
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.
(Assignee)

Comment 11

6 years ago
Created attachment 702125 [details] [diff] [review]
Retain final buffer for titlebar layer manager
Attachment #702125 - Flags: review?(roc)
Component: General → Layout
Product: Firefox → Core
status-firefox18: --- → affected
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".)
Attachment #702125 - Flags: review?(roc) → review+
https://hg.mozilla.org/mozilla-central/rev/29288a6d1876
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21

Updated

6 years ago
Depends on: 831829

Comment 15

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

Updated

6 years ago
Depends on: 832198
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?
Depends on: 834225
You need to log in before you can comment on or make changes to this bug.