[webkit-reviews] review granted: [Bug 94364] Add saturation arithmetic support to FractionalLayoutUnit : [Attachment 159955] Patch

[bugzilla-daemon at webkit.org]

Levi Weintraub <leviw at chromium.org> has granted Emil A Eklund
<eae at chromium.org>'s request for review:
Bug 94364: Add saturation arithmetic support to FractionalLayoutUnit
https://bugs.webkit.org/show_bug.cgi?id=94364

Attachment 159955: Patch
https://bugs.webkit.org/attachment.cgi?id=159955&action=review

------- Additional Comments from Levi Weintraub <leviw at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=159955&action=review


Looks good once the REPORT_OVERFLOW corrections are made. I'm excited to see
how this fares in the PLT. It'd be great to be able to turn this on...

> Source/WebCore/platform/FractionalLayoutUnit.h:82
> +	   REPORT_OVERFLOW(isInBounds(value));

REPORT_OVERFLOW should only trigger without saturated arithmetic.

> Source/WebCore/platform/FractionalLayoutUnit.h:91
> +	   REPORT_OVERFLOW(isInBounds(value));

Ditto.

> Source/WebCore/platform/FractionalLayoutUnit.h:109
>	   REPORT_OVERFLOW(isInBounds(value));

Ditto.

> Source/WebCore/platform/FractionalLayoutUnit.h:142
>      static FractionalLayoutUnit fromFloatRound(float value)
>      {
> +#if ENABLE(SATURATED_LAYOUT_ARITHMETIC)
> +	   if (value >= 0)
> +	       return clamp(value + epsilon() / 2.0f);
> +	   return clamp(value - epsilon() / 2.0f);
> +#else
>	   if (value >= 0)
>	       return FractionalLayoutUnit(value + epsilon() / 2.0f);
>	   return FractionalLayoutUnit(value - epsilon() / 2.0f);
> +#endif
>      }

This function needs a REPORT_OVERFLOW for the non-saturated case.