584617 - some RenderDocument cases need to be more careful for flushing layers

Status: RESOLVED FIXED

(Core :: Layout, defect)

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: Part 1. Drawing ignoring viewport scrolling should use the passed in dirty rect, not the overflow of the root.

If we are drawing ignoring viewport scrolling then using the overflow rect of the root frame is potentially wrong. And if we are going to flush layers anyways, we may as well use the passed in dirty region instead of the whole overflow rect of the root frame.

Comment 2

[Timothy Nikkel (:tnikkel)]

Attachment: Part 2. If RenderDocument uses an unusual backstop color we need to flush layers.

If RenderDocument passes something unusual for the background color (to be painted behind the document) then that color will be put into a display item and will go into the retained layer contents. So we have to flush layers if this happens. Most callers of RenderDocument pass white, and PresShell::Paint usually also passes white for these cases, so hopefully we don't need to do this often.

This patch also makes RenderDocument stop trying to make nsLayoutUtils::PaintFrame use widget layers in cases where we know it can't.

Comment 3

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

(In reply to comment #1)
> If we are drawing ignoring viewport scrolling then using the overflow rect of
> the root frame is potentially wrong. And if we are going to flush layers
> anyways, we may as well use the passed in dirty region instead of the whole
> overflow rect of the root frame. 

Don't do that latter optimization. It could break stuff down the road. For example, Mac unified toolbar drawing assumes that if we paint the chrome window at all, we will build a display list that includes any toolbars.

Comment 4

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

Well, I guess that doesn't matter so much since we wouldn't be drawing to the widget with these flags.

Comment 5

[Timothy Nikkel (:tnikkel)]

Attachment: reftest failure log

Part 1 is causing failures in layout/reftests/svg/gradient-live-01a.svg (and b-d) on Linux and sometimes Windows. Part 1 changes this because RenderDocument always passes the ignore suppression flag, so we use the passed in dirty rect (and flush layers). The differences aren't spottable to human eyes, but it looks like there are minor differences along lines where the gradient stops are. I can't reproduce this locally.

I could just change the patch to only include the correctness fix, that is only use the passed in dirty rect when we are drawing ignoring viewport scrolling.

Comment 6

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

Yes, let's do that.

Comment 7

[Timothy Nikkel (:tnikkel)]

Attachment: Part 1. Drawing ignoring viewport scrolling should use the passed in dirty rect, not the overflow of the root.

Having the willFlushLayers variable in this part is kind of weird, but it goes with part 2 so I think that is fine.

Comment 8

[Timothy Nikkel (:tnikkel)]

Comment on attachment 463767 [details] [diff] [review]
Part 1. Drawing ignoring viewport scrolling should use the passed in dirty rect, not the overflow of the root.

Regressions from retained layers.

Comment 9

[Timothy Nikkel (:tnikkel)]

http://hg.mozilla.org/mozilla-central/rev/2b0c12b6cdc0
http://hg.mozilla.org/mozilla-central/rev/4b65c4c0a9c5