619117 - Invalidation problems on massivehealth.com

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

If you mouse over the images near the bottom of the page we sometimes leave artifacts of the images.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

This is a regression from 3.6 and I expect this has to do layers

Comment 2

[Jeff Muizelaar [:jrmuizel]]

The layout of the photographs doesn't seem that deterministic so you have 'rearrange' them a couple of time to have this show up. You should be able to see it just bellow when one of the photographs is angled so that it hangs low.

Comment 3

[gadjo]

It happens also on Win7. There are no artifacts in 20100718 build (after landing bug 564991 with lot of changes in layers), but there are artifacts in 20100901 build, although they are different than in the latest build.

Comment 4

[Joe Drew (not getting mail)]

Attachment: screenshot

Comment 5

[Joe Drew (not getting mail)]

For now, I think this is something that can wait until .next or a dot release, but I'd like to hear from Timothy or Robert as to whether it's indicative of a larger problem.

Comment 6

[Timothy Nikkel (:tnikkel)]

The other artifacts in this testcase are gone now, so we only need to figure out when the tips of the photos don't get erased (which seems to be a distinct issue).

For that problem
The first bad revision is:
changeset:   51893:f6916eb6b678
user:        Robert O'Callahan <[e-mail]>
date:        Thu Sep 02 14:07:37 2010 -0400
summary:     Bug 584494 - Regression: Transform rotation testcase performs significantly worse - r=dbaron

Comment 7

[Timothy Nikkel (:tnikkel)]

Based on that we might not want to bump this to .next, but I don't really understand the regressing changeset.

Comment 8

[Timothy Nikkel (:tnikkel)]

Bug 624946 is probably the same thing. I think this needs to block.

Comment 9

[Timothy Nikkel (:tnikkel)]

I started debugging this. I don't mind taking this, but I might not get to it in a timely manner.

Comment 10

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

Attachment: reduced testcase

Click on the body to trigger the bug.

Comment 11

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

This was actually reduced from bug 624946.

Comment 12

[Timothy Nikkel (:tnikkel)]

My debugging got as far as neither branch being taking here http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsAbsoluteContainingBlock.cpp#492 because the frame's rect was identical but the overflow rect had changed quite a bit.

Comment 13

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

The bug in this testcase is that nsCSSFrameConstructor::ProcessRestyledFrames indirectly calls nsFrame::InvalidateLayer(frame->GetOverflowRectRelativeToSelf()) which indirectly calls nsIFrame::InvalidateInternalAfterResize, which tries to apply the frame's current transform to the frame-relative damage area (GetOverflowRectRelativeToSelf()) to see what needs to be invalidated. But it uses the new transform, not the old transform, so it doesn't get the whole area that needs to be invalidated.

Comment 14

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

Sorry that's GetVisualOverflowRectRelativeToSelf in the previous comment.

Note that GetVisualOverflowRectRelativeToSelf is specifically coded to get the overflow rect *before* transformation.

Comment 15

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

Attachment: fix

Fixes bug 624946 as well.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

If the transform has changed, don't you need to invalidate both old and new overflow area in the parent?  Which one is this doing?

Comment 18

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

This is invalidating the old overflow area.

The new overflow area is invalidated by nsIFrame::FinishAndStoreOverflow.

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 504621 [details] [diff] [review]
fix

ok, could you clarify the comment above InvalidateTransformLayer to reflect that?

Comment 20

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

Sure.

Comment 21

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

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

Followup test fix:
http://hg.mozilla.org/mozilla-central/rev/1fc29746701e

Comment 22

[gadjo]

Div content is blured after changing scale - filed bug 628854

Comment 23

[Steve Scott (pxbugz)]

Verified fixed on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre

Comment 24

[Aza Raskin [:aza]]

P.S., thanks Roc and JoeDrew and everyone. It's pretty awesome to have Firefox patched for my tiny website :)

Comment 25

[Michael Yoshitaka Erlewine [:mitcho]]

(In reply to comment #24)
> P.S., thanks Roc and JoeDrew and everyone. It's pretty awesome to have Firefox
> patched for my tiny website :)

I don't think that was the *only* reason. ;)