Bug 190106 – Style::determineChange() does a full RenderStyle compare when it needs !=

• Status: NEW
• Component: CSS

[Simon Fraser (smfr)]

Style::determineChange() has:

    if (s1 != s2) {
        if (s1.inheritedNotEqual(&s2))
            return Inherit;

        if (s1.alignItems() != s2.alignItems() || s1.justifyItems() != s2.justifyItems())
            return Inherit;

        return NoInherit;
    }

That s1 != s2 is a deep, full compare since:

    bool operator!=(const RenderStyle& other) const { return !(*this == other); }

We should probably implement operator != for the common types, which early returns.

[Antti Koivisto]

> We should probably implement operator != for the common types, which early
> returns.

In case styles are equal operator!= is not usually very deep. It just compares a small set of pointers and bits for equality.

[Simon Fraser (smfr)]

(In reply to Antti Koivisto from comment #1)
> > We should probably implement operator != for the common types, which early
> > returns.
> 
> In case styles are equal operator!= is not usually very deep. It just
> compares a small set of pointers and bits for equality.

Right, but it's the != case where only one or two properties changed that affect benchmark and animated content performance (e.g. things that just change transform).

[Ahmad Saleem]

We have following:

https://github.com/WebKit/WebKit/blob/6344a735d399205f67cc29e54ac57e23a803ea4f/Source/WebCore/style/StyleChange.cpp#L34

and it seems to have added some cases "PseudoId::FirstLetter", "columnSpan" etc. 

Do we need to tackle this anymore or Webkit now able to do early returns similar to proposed in Comment 0?

Appreciate if someone can comment based on current state and we can mark this bug accordingly. Thanks!