[webkit-reviews] review granted: [Bug 222816] Improve blending of Length and other Length-related types : [Attachment 422490] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Antoine Quint
<graouts at webkit.org>'s request for review:
Bug 222816: Improve blending of Length and other Length-related types
https://bugs.webkit.org/show_bug.cgi?id=222816

Attachment 422490: Patch

https://bugs.webkit.org/attachment.cgi?id=422490&action=review




--- Comment #5 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 422490
  --> https://bugs.webkit.org/attachment.cgi?id=422490
Patch

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

> Source/WebCore/animation/CSSPropertyAnimation.cpp:703
>  class LengthPropertyWrapper : public PropertyWrapperGetter<const Length&> {
>      WTF_MAKE_FAST_ALLOCATED;
>  public:
> -    LengthPropertyWrapper(CSSPropertyID prop, const Length&
(RenderStyle::*getter)() const, void (RenderStyle::*setter)(Length&&))
> +    enum class Flags {
> +	   IsLengthPercentage	       = 1 << 0,
> +	   NegativeLengthsAreInvalid   = 1 << 1,
> +    };
> +    LengthPropertyWrapper(CSSPropertyID prop, const Length&
(RenderStyle::*getter)() const, void (RenderStyle::*setter)(Length&&),
OptionSet<Flags> flags = { })

I think you could make the flags a template argument instead as they are fixed
for a given property type.

Not sure if this has much practical significance though and could be done
separately.

> Source/WebCore/animation/CSSPropertyAnimation.cpp:712
> -	   return !this->value(a).isAuto() && !this->value(b).isAuto();
> +	   return canInterpolateLengths(this->value(a), this->value(b),
m_flags.contains(Flags::IsLengthPercentage));

Then canInterpolateLengths would also take IsLengthPercentage as template
argument.