1728497 - nsColumnSetFrame::CalculateColumnRuleBounds isn't worth it

Status: RESOLVED FIXED

(Core :: Layout: Columns, enhancement, P3)

Description

[Mats Palmgren (inactive)]

nsColumnSetFrame::CalculateColumnRuleBounds seems unnecessarily complex to me:
https://searchfox.org/mozilla-central/rev/ad2ffab089e4e0c0fe99a1a046ab2b1c45546bdb/layout/generic/nsColumnSetFrame.cpp#167
The only caller is nsDisplayColumnRule::GetBounds:
https://searchfox.org/mozilla-central/rev/ad2ffab089e4e0c0fe99a1a046ab2b1c45546bdb/layout/generic/nsColumnSetFrame.cpp#37-41
but that method could just return the frame's InkOverflowRect() instead since that always contains the column-rule areas. That's slightly larger than the union of the column-gap areas but it doesn't seem worth optimizing for the case where the first/last column is visible but none of the column-rules. That seems like a rare edge case, so CalculateColumnRuleBounds just wastes CPU cycles for the common case.

Let me know if I'm missing something...

(The reason I want to remove it is that I'm generalizing nsDisplayColumnRule for bug 1725495 so that we can use it for Flexbox and Grid too. So I want to get rid of this dependency on nsColumnSetFrame.)

Comment 1

[Mats Palmgren (inactive)]

Attachment: Bug 1728497 - Remove nsColumnSetFrame::CalculateColumnRuleBounds and just return InkOverflowRect() instead. r=TYLin

Comment 2

[Mats Palmgren (inactive)]

https://treeherder.mozilla.org/jobs?repo=try&revision=87a17e5be41cf37c2298c15e1f9bde19c282797a

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Comment 0 sounds reasonable to me.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d68c972998b5
Remove nsColumnSetFrame::CalculateColumnRuleBounds and just return InkOverflowRect() instead.  r=TYLin

Comment 5

[Alexandru Michis [:malexandru]]

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