Bug 852489 added the following behaviour (which seems to be still present): 1. ProcessDisplayitems sets the visible region for non transform items when building a layer. It passes down an ancestor clip rect for transform items, but null clip for non-transform items. 2. nsDisplayTransform::BuildLayer passes the "not clipped by ancestor flag" to BuildContainerLayerFor unconditionally. 3. BuildContainerLayerFor uses the ancestor clip rect passed in to set the visible region if the "not clipped by ancestor flag" is not set. So it seems like mAncestorClipRect on ContainerParameters is unused because the only time it is set is for transforms, and they always pass the flag that says to ignore it. We could either clip active transform layers when they aren't being prerendered, or get rid of this mAncestorClipRect code. Patch for the former attached, which I think was the intention of the patch from bug 852489 all along.

This caused bug 964688, bug 966509, and bug 966519. These are worse then what this fixes (a little bit of perf in a few cases). I haven't had a chance to look at the regressions since I've been working on other things. I think we should back this out from beta and aurora to give us time to look into the regressions.

Comment on attachment 8374626 [details] [diff] [review] backout Review of attachment 8374626 [details] [diff] [review]: ----------------------------------------------------------------- I couldn't reproduce one of those regressions when I tested, are we sure they still exist?

I reproduced bug 964688 and bug 966509 with latest nightly

Comment on attachment 8374626 [details] [diff] [review] backout see comment 3 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 947467 (this bug) User impact if declined: rendering problems with 3d transforms Testing completed (on m-c, etc.): this was the status quo on Firefox release for a while Risk to taking this patch (and alternatives if risky): low String or IDL/UUID changes made by this patch: none

checkin-needed for the backout patch on beta and auror

Comment on attachment 8374626 [details] [diff] [review] backout https://hg.mozilla.org/releases/mozilla-aurora/rev/ed268cf8ef4b https://hg.mozilla.org/releases/mozilla-beta/rev/b0c69437217c

Are there websites we can/should be testing to ensure this hasn't regressed anything?

It has regressed things, see the dependent bugs 964688 966509 966519. I haven't had a chance to look into those regressions yet.

Ah yes, I see that now. Thanks for pointing this out, Timothy. Should this be allowed to ride the trains to release given it's known to cause at least 3 regressions?

No, it shouldn't, that's why we backed up it of aurora and beta, to give us time to look at the regressions. If we don't solve the regressions on the nightly channel we should look into backing out completely.

(In reply to Timothy Nikkel (:tn) from comment #13) > No, it shouldn't, that's why we backed up it of aurora and beta, to give us > time to look at the regressions. If we don't solve the regressions on the > nightly channel we should look into backing out completely. Nominating for tracking and needinfo? to Release Management to make sure this is on their radar.

Based on the evident impact to our users and comment 13 we will track this for now to make sure its addressed. Thanks

There are no known open regressions from this bug anymore (on any branch) so this doesn't need to track (that was the reason for setting tracking in the first place). Setting to ? to confirm.

Thanks, untracking then.