Bug 117199 – [CSS Regions] Improve getRegionsByContent by testing in flow thread coordinates instead of view coordinates

• Status: RESOLVED
• Resolution: WONTFIX
• Component: CSS

[Mihnea Ovidenie]

If the object for which we want to check whether it is inside a region is not a box, we can do the testing in flow thread coordinates instead of converting everything to render view coordinates. We can thus avoid calls to absoluteBoundingBoxRect for both the object and the region.

[Mihnea Ovidenie]

Created attachment 203702 [details]
Patch

[Alexandru Chiculita]

Comment on attachment 203702 [details]
Patch

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

> Source/WebCore/rendering/RenderFlowThread.cpp:818
> +    LayoutRect boundingBox;

Why not just use localToContainerQuad instead (now that it can also return coordinates in the flowthread space)?

> Source/WebCore/rendering/RenderFlowThread.cpp:824
> +        ASSERT(false);

nit: this could be ASSERT_NOT_REACHED.

Why only renderInlines and text? Looks like the caller could send any node in here.

> Source/WebKit2/Shared/WebPreferencesStore.h:98
> +    macro(RegionBasedColumnsEnabled, regionBasedColumnsEnabled, Bool, bool, true) \

I don't see any comments in the changelog about this change.

[Mihnea Ovidenie]

(In reply to comment #2)
> (From update of attachment 203702 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=203702&action=review
> 
> > Source/WebCore/rendering/RenderFlowThread.cpp:818
> > +    LayoutRect boundingBox;
> 
> Why not just use localToContainerQuad instead (now that it can also return coordinates in the flowthread space)?
> 

I can try that too.

> > Source/WebCore/rendering/RenderFlowThread.cpp:824
> > +        ASSERT(false);
> 
> nit: this could be ASSERT_NOT_REACHED.
> 
> Why only renderInlines and text? Looks like the caller could send any node in here.
> 

Because for boxes i have checked above and relied on getRegionRangeForBox.

> > Source/WebKit2/Shared/WebPreferencesStore.h:98
> > +    macro(RegionBasedColumnsEnabled, regionBasedColumnsEnabled, Bool, bool, true) \
> 
> I don't see any comments in the changelog about this change.

This slipped in from a research, will remove, thx for catching.

[Brent Fulgham]

CSS Regions were removed in Bug 174978.