Bug 170988 – Do not paint the border of the box if the dirty region does not intersect with border area

• Component: New Bugs
• Status: RESOLVED FIXED

[Gwang Yoon Hwang]

Do not paint the border of the box if the dirty region does not intersect with border area

[Gwang Yoon Hwang]

Created attachment 307477 [details]
Patch

[Carlos Garcia Campos]

Sorry, I thought this was cairo specific.

[Simon Fraser (smfr)]

Comment on attachment 307477 [details]
Patch

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

Looks OK but I didn't yet check all the math.

> Source/WebCore/platform/graphics/RoundedRect.cpp:251
> +static bool isCircleContainsPoint(const FloatPoint& point, const FloatPoint& center, const FloatSize& radii)
> +{
> +    FloatPoint transformedPoint(point);
> +    transformedPoint.move(-center.x(), -center.y());
> +    transformedPoint.scale(radii.height(), radii.width());
> +    float radius = radii.width() * radii.height();
> +
> +    if (transformedPoint.x() > radius || transformedPoint.y() > radius)
> +        return false;
> +    if (transformedPoint.x() + transformedPoint.y() <= radius)
> +        return true;
> +    return (transformedPoint.lengthSquared() <= radius * radius);
> +}

Please put this in GeometryUtilities.cpp (and fix the header to use #pragma once at the same time).

> Source/WebCore/platform/graphics/RoundedRect.cpp:260
> +        FloatPoint center(m_rect.x() + topLeft.width(), m_rect.y() + topLeft.height());

We usually do this now like: 

FloatPoint center = { m_rect.x() + topLeft.width(), m_rect.y() + topLeft.height() };

[Gwang Yoon Hwang]

Created attachment 307590 [details]
Patch

[Gwang Yoon Hwang]

(In reply to Simon Fraser (smfr) from comment #3)
> Comment on attachment 307477 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307477&action=review
> 
> Looks OK but I didn't yet check all the math.
> 
The math stuff :)

> > Source/WebCore/platform/graphics/RoundedRect.cpp:251
> > +static bool isCircleContainsPoint(const FloatPoint& point, const FloatPoint& 
> 
> Please put this in GeometryUtilities.cpp (and fix the header to use #pragma
> once at the same time).

I moved it to the GeometryUtilities and renamed it to ellipseContainsPoint
Also I modernized the header.
 
> 
> > Source/WebCore/platform/graphics/RoundedRect.cpp:260
> > +        FloatPoint center(m_rect.x() + topLeft.width(), m_rect.y() + topLeft.height());
> 
> We usually do this now like: 
> 
> FloatPoint center = { m_rect.x() + topLeft.width(), m_rect.y() +
> topLeft.height() };

Thanks, fixed. I removed unneeded FloatSize declarations from here, too.

[Simon Fraser (smfr)]

Comment on attachment 307590 [details]
Patch

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

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1737
> +    // If no borders intersects with the dirty area, we can skip to paint borders.

"skip the border painting"

[Gwang Yoon Hwang]

Created attachment 307736 [details]
Patch

[Gwang Yoon Hwang]

(In reply to Simon Fraser (smfr) from comment #6)
> Comment on attachment 307590 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307590&action=review
> 
> > Source/WebCore/rendering/RenderBoxModelObject.cpp:1737
> > +    // If no borders intersects with the dirty area, we can skip to paint borders.
> 
> "skip the border painting"

Thanks for the correction :)

[WebKit Commit Bot]

Comment on attachment 307736 [details]
Patch

Clearing flags on attachment: 307736

Committed r215613: <http://trac.webkit.org/changeset/215613>

[Gwang Yoon Hwang]

This patch has been landed. Closing

[Simon Fraser (smfr)]

This caused bug 176498.

[Michael Catanzaro]

Simon, I think this commit was just an optimization that could be rolled out, but Yoon doesn't work for us anymore so I can't ask him. It did function as a workaround for bug #126124, though, so please reopen that bug if you roll this out.