1636071 - nsIFrame::GetTransformMatrix should also stop at out of flow frames when passed the STOP_AT_STACKING_CONTEXT_AND_DISPLAY_PORT flag

Status: RESOLVED FIXED

(Core :: Web Painting, task)

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1636071. No longer stop at out of flow frames in nsLayoutUtils::GetTransformToAncestor when using the STOP_AT_STACKING_CONTEXT_AND_DISPLAY_PORT flag. r?miko

https://hg.mozilla.org/mozilla-central/rev/2c2e56a87ad1 (bug 1428993) made nsLayoutUtils::GetTransformToAncestor stop at out of flow frames when passed the STOP_AT_STACKING_CONTEXT_AND_DISPLAY_PORT flag. But it didn't make the same change to nsIFrame::GetTransformMatrix, which seems like an oversight.

Just noticed this while reading the code, not sure if it causes any problems.

Comment 2

[Miko Mynttinen]

Sorry for the delay, this code is a bit tricky.

This change would make nsIFrame::GetTransformMatrix()1 to stop traversal of the frame tree when it encounters an OOF frame that does not hit the other conditions. Assume that we are processing a modified OOF frame in ComputeRebuildRegion():
If the modified OOF frame has an OOF parent, this would cause an additional ProcessFrameInternal() iteration, which should be no-op, since the parent frame would contain the child frame overflow2.

I think that the OOF frame stop condition from bug 1428993 is no longer needed, since the patch from bug 1530584 changed how OOF frame placeholder overflow area is transformed. It might even be possible to avoid some existing no-op iterations in ComputeRebuildRegion() by changing nsLayoutUtils::TransformFrameRectToAncestor() to directly convert the modified frame overflow area coordinate space to nearest frame with display port or containing block that is a stacking contex (as opposed to just stacking context).

Matt, what do you think?

Comment 3

[Miko Mynttinen]

Attachment: testcase.html

ComputeRebuildRegion output:

Patch:

Computing rebuild regions for 1 frames:
Considering OOFs for 0x108d4fe50
Considering OOFs for 0x108d4ff10
Considering OOFs for 0x108d4fd10
Considering OOFs for 0x108d4fdd0
Considering OOFs for 0x108d4fc50
Considering OOFs for 0x108d4fb40
Considering OOFs for 0x108d4fa80
Considering OOFs for 0x108d4f0c0
Considering OOFs for 0x108d4f198
Considering OOFs for 0x108d4f020
Processing frame 0x108d4fe50 with agr 0x108d4f0c0
currentFrame: 0x108d4fe50 (placeholder=0), aOverflow: 0 0 9000 9000
Processing placeholder 0x108d4ff10 for OOF frame 0x108d4fe50
OOF frame draw area: 6000 6000 9000 9000
currentFrame: 0x108d4ff10 (placeholder=1), aOverflow: 6000 6000 9000 9000
currentFrame: 0x108d4fd10 (placeholder=0), aOverflow: 6000 6000 9000 9000
Processing placeholder 0x108d4fdd0 for OOF frame 0x108d4fd10
OOF frame draw area: 12000 12000 9000 9000
currentFrame: 0x108d4fdd0 (placeholder=1), aOverflow: 12000 12000 9000 9000
Frame belongs to stacking context frame 0x108d4fc50
Adding area to stacking context draw area: 12000 12000 9000 9000
Frame belongs to stacking context frame 0x108d4fc50
Adding area to stacking context draw area: 12000 12000 9000 9000

Current:

Computing rebuild regions for 1 frames:
Considering OOFs for 0x1124efe50
Considering OOFs for 0x1124eff10
Considering OOFs for 0x1124efd10
Considering OOFs for 0x1124efdd0
Considering OOFs for 0x1124efc50
Considering OOFs for 0x1124efb40
Considering OOFs for 0x1124efa80
Considering OOFs for 0x1124ef0c0
Considering OOFs for 0x1124ef198
Considering OOFs for 0x1124ef020
Processing frame 0x1124efe50 with agr 0x1124ef0c0
currentFrame: 0x1124efe50 (placeholder=0), aOverflow: 0 0 9000 9000
Processing placeholder 0x1124eff10 for OOF frame 0x1124efe50
OOF frame draw area: 6000 6000 9000 9000
currentFrame: 0x1124eff10 (placeholder=1), aOverflow: 6000 6000 9000 9000
Frame belongs to stacking context frame 0x1124efc50
Adding area to stacking context draw area: 12000 12000 9000 9000

Comment 4

[Matt Woodrow (:mattwoodrow)]

(In reply to Miko Mynttinen [:miko] from comment #2)

Sorry for the delay, this code is a bit tricky.

This change would make nsIFrame::GetTransformMatrix()1 to stop traversal of the frame tree when it encounters an OOF frame that does not hit the other conditions. Assume that we are processing a modified OOF frame in ComputeRebuildRegion():
If the modified OOF frame has an OOF parent, this would cause an additional ProcessFrameInternal() iteration, which should be no-op, since the parent frame would contain the child frame overflow2.

I think that the OOF frame stop condition from bug 1428993 is no longer needed, since the patch from bug 1530584 changed how OOF frame placeholder overflow area is transformed. It might even be possible to avoid some existing no-op iterations in ComputeRebuildRegion() by changing nsLayoutUtils::TransformFrameRectToAncestor() to directly convert the modified frame overflow area coordinate space to nearest frame with display port or containing block that is a stacking contex (as opposed to just stacking context).

Matt, what do you think?

I am really struggling to page this code back in. Your explanation makes sense to me, and I think it's correct, but I'm not 100% sure.

Happy for you to go ahead with trying that, or you can talk me through it in more detail.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac416f5226eb
No longer stop at out of flow frames in nsLayoutUtils::GetTransformToAncestor when using the STOP_AT_STACKING_CONTEXT_AND_DISPLAY_PORT flag. r=miko

Comment 6

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/ac416f5226eb