[webkit-reviews] review granted: [Bug 177883] TextDecorationPainter::m_wavyOffset should be a float : [Attachment 322687] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 4 11:19:36 PDT 2017


Simon Fraser (smfr) <simon.fraser at apple.com> has granted Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 177883: TextDecorationPainter::m_wavyOffset should be a float
https://bugs.webkit.org/show_bug.cgi?id=177883

Attachment 322687: Patch

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




--- Comment #3 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 322687
  --> https://bugs.webkit.org/attachment.cgi?id=322687
Patch

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

> Source/WebCore/rendering/TextDecorationPainter.cpp:250
> +    : m_context { context }
> +    , m_decoration { decoration }
> +    , m_wavyOffset { wavyOffsetFromDecoration() }
> +    , m_isPrinting { renderer.document().printing() }
> +    , m_styles { stylesForRenderer(renderer, m_decoration, isFirstLine) }
> +    , m_lineStyle { isFirstLine ? renderer.firstLineStyle() :
renderer.style() }

Are we going to do this everywhere? It seems like a lot of churn for zero gain.

> Source/WebCore/rendering/TextDecorationPainter.h:67
>      TextDecoration m_decoration;
> -    int m_wavyOffset { 0 };
> +    float m_wavyOffset;

Looks like these could be re-ordered for better packing (but maybe we never
heap-allocate these).


More information about the webkit-reviews mailing list