[webkit-reviews] review denied: [Bug 54491] [cairo][canvas] Drawing from/into float rectangles with width or height in range 0 to 1 fails : [Attachment 82631] Adjust rounding steps to ensure minimal width or height in such cases

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 17 15:31:51 PST 2011


Martin Robinson <mrobinson at webkit.org> has denied Zan Dobersek
<zandobersek at gmail.com>'s request for review:
Bug 54491: [cairo][canvas] Drawing from/into float rectangles with width or
height in range 0 to 1 fails
https://bugs.webkit.org/show_bug.cgi?id=54491

Attachment 82631: Adjust rounding steps to ensure minimal width or height in
such cases
https://bugs.webkit.org/attachment.cgi?id=82631&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=82631&action=review

Great stuff. Just a bit more cleanup and I think this is a definite r+.

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:755
> -    double x = frect.x();
> -    double y = frect.y();
> +    double x, y;
> +
>      cairo_t* cr = m_data->cr;
> +
> +    x = frect.x();
> +    y = frect.y();

I think you don't need to make this change.

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:766
>      x = frect.width();
>      y = frect.height();

Please create two more variables here width and height. This code needs a
little cleanup and the memory savings are not worth the rampant chaos. :)

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:769
> +    x = !round(x) ? (!x ? 0.0f : (x > 0 ? 1.0f : -1.0f)) : round(x);
> +    y = !round(y) ? (!y ? 0.0f : (y > 0 ? 1.0f : -1.0f)) : round(y);

Instead of rounding twice, I'd rather this be cached.

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:772
>      result.setWidth(static_cast<float>(x));
>      result.setHeight(static_cast<float>(y));

It might be more appropriate to do narrowToFloatPrecision here. I'm not sure.


More information about the webkit-reviews mailing list