Bug 92164 – [CSS Shapes] Shapes should not cause relayout when they are equivalent

• Status: RESOLVED
• Resolution: LATER
• Component: CSS

[Bear Travis]

RenderStyle::diff should not cause relayout and RenderBlock::styleDidChange should not generate a new WrapShapeInfo for two different but equivalent WrapShapes
Currently, just pointers are used for comparison, but eventually they should use a deep compare

[Bem Jones-Bey]

This also affects shape-outside, not just shape-inside.

[Nico Weber]

*** Bug 100811 has been marked as a duplicate of this bug. ***

[Bear Travis]

Created attachment 204744 [details]
Initial patch

[Sam Weinig]

Comment on attachment 204744 [details]
Initial patch

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

> Source/WebCore/ChangeLog:11
> +        The two layouts are equivalent, so there are no new tests.

If this is being done for performance, do you have any perf results that this made better? Is there a benchmark you are using or a test case you are investigating?

> Source/WebCore/rendering/style/BasicShapes.h:98
> +    virtual bool operator==(const BasicShape& other)

You should add OVERRIDE here (and on all the others below).  Given that this is a virtual function, does this really need to be inline?

> Source/WebCore/rendering/style/BasicShapes.h:211
> +        for (size_t i = 0; i < values().size(); i++)
> +            if (getXAt(i) != otherPolygon->getXAt(i) || getYAt(i) != otherPolygon->getYAt(i))
> +                return false;

The for-loop needs braces.

[Zoltan Horvath]

Comment on attachment 204744 [details]
Initial patch

Pulling this out from the review queue, since it's need to be updated. (Changelog: [CSS Exclusions])

[Zoltan Horvath]

Closing bug until we have a perf test to verify the improvement.

[Ahmad Saleem]

Just to add we do have FIXME still in our code referring to bug, which is marked duplicate of this:

https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderBox.cpp#447

// FIXME: A future optimization would do a deep comparison for equality. (bug 100811)
    if (shapeOutside == oldShapeOutside && shapeMargin == oldShapeMargin && shapeImageThreshold == oldShapeImageThreshold)
        return;