Summary: | Do not paint the border of the box if the dirty region does not intersect with border area | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gwang Yoon Hwang <yoon> | ||||||||
Component: | New Bugs | Assignee: | Gwang Yoon Hwang <yoon> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bugs-noreply, cgarcia, commit-queue, mcatanzaro, simon.fraser | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=126124 | ||||||||||
Attachments: |
|
Description
Gwang Yoon Hwang
2017-04-19 05:36:38 PDT
Created attachment 307477 [details]
Patch
Sorry, I thought this was cairo specific. 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() }; Created attachment 307590 [details]
Patch
(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. 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" Created attachment 307736 [details]
Patch
(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 :) Comment on attachment 307736 [details] Patch Clearing flags on attachment: 307736 Committed r215613: <http://trac.webkit.org/changeset/215613> This patch has been landed. Closing This caused bug 176498. 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. |