Event regions sometimes cause improper layerization of display items

RESOLVED INVALID

Status

()

Core
Layout
RESOLVED INVALID
3 years ago
3 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

While investigating bug 1098654 I came across a situation that I believe is a bug in the event regions code. It has to do with the code that assigns display items to PaintedLayerData objects.

Consider the case where we have a stack of PaintedLayerData objects with the top two objects having animated geometry roots A (topmost) and B (just under A). The next display item (call it X) we process also has animated geometry root B, but intersects the draw region of A. When this happens, we pop off A from the stack at [1] and push on a new PaintedLayerData (call it C), accumulating the display list item into C. Subsequent display items that intersect X will also get assigned to layer C.

Now consider the same case, but where X is an event-regions display item. We still pop off A and create a new PaintedLayerData C, and accumulate the event regions display item into C. However the code for accumulating event regions [2] does not modify the mDrawRegion of C the way it does for all other display items. This means that subsequent display items that intersect the bounds of X will not get assigned to layer C but will actually end up assigned to B which has the same animated geometry root but is lower in the stack.

Therefore, enabling event regions can change the layerization of display items, which is wrong. I think the correct fix here is to simply make sure the mDrawRegion gets updated when accumulating event regions. I need to check if this actually fixes the problem I was seeing.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp?rev=923c99af6863#2534
[2] http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp?rev=923c99af6863#3055
Created attachment 8531906 [details] [diff] [review]
Patch

And indeed this seems to fix the issue I was seeing on bug 1098654.
Assignee: nobody → bugmail.mozilla
Attachment #8531906 - Flags: review?(tnikkel)
I also ran into roc and he said this sounded correct. If the display items go into the other layer then the event regions for the layers might be wrong.
tn pointed out that the draw-above region of layer B should prevent the display items from falling into it. In the case where the draw-above region doesn't intersect the display item's draw region it should be correct for the display items to end up in layer B. The only reason this might be wrong is if this code is not fully aware of possible async transformations in the compositor. That might be the case here, so I should investigate this a bit further.
Attachment #8531906 - Flags: review?(tnikkel)
Haven't had a chance to look into this more, but I was thinking about it and it seems to me that while the modified layerization might still be visually correct, it may be wrong in that the event regions on the layers will be wrong. That is, in the scenario described in comment 0, the subsequent display items will end up in layer B but their event regions item will still end up in layer C, which might be wrong.
Blocks: 1107280
No longer blocks: 1098654, 1107280
I don't know if this is a real bug or not. Closing as INVALID for now; if we run into a real-world case where this matters we can figure it out then.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.