478079 - make canvas-color tracking and last-resort repaint entirely the responsibility of the pres shell

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement)

Description

[Zack Weinberg (:zwol)]

Attachment: proposed change

We currently maintain a cache of the canvas frame's background color in the view manager.  This is used to draw the background under circumstances where we can't do a full repaint, and is propagated across pages by the docshell in order to reduce window flashing in between pages with colored backgrounds.

Almost all of the drawing work that's relevant here is being done by the presentation shell (the only exception is when refresh has been disabled in the view manager).  It removes some awkward corner cases (having mostly to do with partial transparency) if we move the cached background color there, and have the view manager always call the presentation shell to do repaint, even when refresh is disabled.  The attached patch does just this.

Unfortunately, this involves interface bumps on all three of nsIViewObserver, nsIViewManager, and nsIPresShell, so I can't recommend it for 1.9.1...

Comment 1

[Zack Weinberg (:zwol)]

forgot to mark dependency of this patch on the patch in bug 476557.

Comment 2

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

Any particular reason you asked Boris for review? This is more my thing.

Comment 3

[Zack Weinberg (:zwol)]

Comment on attachment 361826 [details] [diff] [review]
proposed change

> Any particular reason you asked Boris for review? This is more my thing.

No, I was just mixed up.

Comment 4

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

Comment on attachment 361826 [details] [diff] [review]
proposed change

Thanks for doing this, it's good cleanup.

+  NS_IMETHOD PaintDefaultBg(nsIView *aView,
+                            nsIRenderingContext* aRenderingContext,
+                            const nsRect& aDirtyRect);

Might as well call it PaintDefaultBackground.

Comment 5

[Zack Weinberg (:zwol)]

This patch needs to be revised for the new patch in bug (In reply to comment #4)
> Might as well call it PaintDefaultBackground.

Ok.

The patch also needs to be revised for the new patch in bug 476557 and some other tree changes.  In particular, bug 442774 bumped the IID on nsIPresShell.  Do I need to bump it again?

Comment 6

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

You might as well.

Comment 7

[Zack Weinberg (:zwol)]

Attachment: updated patch, ready for landing

rediffed for changes in bug 476557 + other changes that landed in nsIPresShell.h.

must land after bug 476557; not for 1.9.1.

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 362357 [details] [diff] [review]
updated patch, ready for landing

patching file layout/base/nsPresShell.cpp
Hunk #2 FAILED at 5379
1 out of 2 hunks FAILED

Comment 9

[Zack Weinberg (:zwol)]

Attachment: rediffed after bug 478784 [Checkin: Comment 10]

here's an updated patch.

Comment 10

[Serge Gautherie (:sgautherie)]

Comment on attachment 363146 [details] [diff] [review]
rediffed after bug 478784
[Checkin: Comment 10]


http://hg.mozilla.org/mozilla-central/rev/c67bc280f52f