Bug 68638 – [CSSRegions] Make RenderFlowThread track if it has variable width regions

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dave Hyatt]

Make RenderFlowThread track if it has variable width regions. This will be relevant for speed as we begin adding code to shrink lines and do custom painting of blocks, etc.

[Dave Hyatt]

Created attachment 108359 [details]
Patch

[Dave Hyatt]

Created attachment 108361 [details]
Patch

Comment on attachment 108361 [details]
Patch

r=me
I think the naming can be improved, “variable widths” seems like the width can change, maybe you can use “nonUniformLogicalWidth” or flip it and call it “regionsHaveUniformLogicalWidth”.

[Adam Roben (:aroben)]

Comment on attachment 108361 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=108361&action=review

> Source/WebCore/rendering/RenderFlowThread.cpp:308
> +        m_hasValidRegions = false;
> +        LayoutUnit previousRegionLogicalWidth = 0;

Seems like you need to set m_regionsHaveVariableLogicalWidth to false here, too. Otherwise once it goes true it will never become false again!

> Source/WebCore/rendering/RenderFlowThread.h:108
> +    bool regionsHaveVariableLogicalWidth() const { return m_regionsHaveVariableLogicalWidth; }

Seems like this isn't needed yet, but maybe you intend to use it in the future?

[Dave Hyatt]

(In reply to comment #4)

> Seems like you need to set m_regionsHaveVariableLogicalWidth to false here, too. Otherwise once it goes true it will never become false again!
> 

Nice catch! Wouldn't be a correctness issue, but would be a performance one!

[Dave Hyatt]

Fixed in r95740.