[webkit-reviews] review granted: [Bug 43416] [Qt] Implement GraphicsContext::clipOut more efficiently : [Attachment 64096] Patch

[bugzilla-daemon at webkit.org]

Ariya Hidayat <ariya.hidayat at gmail.com> has granted Simon Hausmann
<hausmann at webkit.org>'s request for review:
Bug 43416: [Qt] Implement GraphicsContext::clipOut more efficiently
https://bugs.webkit.org/show_bug.cgi?id=43416

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

------- Additional Comments from Ariya Hidayat <ariya.hidayat at gmail.com>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index
efc27d6e5a204eaeba7775d0a099627bc29adaaf..6b790604d12511157df404aeaad32e0b434ad
d46 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,21 @@
> +2010-08-11  Simon Hausmann  <simon.hausmann at nokia.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   [Qt] Implement GraphicsContext::clipOut more efficiently
> +	   https://bugs.webkit.org/show_bug.cgi?id=43416
> +
> +	   The current implementation of clipOut uses
QPainter::clipRegion().boundingRect(),
> +	   which is a very slow function because it converts the entire clip
region - which
> +	   may potentially contain a complex path - into a set of QRegion
rectangles, just
> +	   to throw away all that information and use the bounding rect.
> +
> +	   QTBUG-12618 implements a faster function in QPainter:
QPainter::clipBoundingRect().
> +
> +	   * platform/graphics/qt/GraphicsContextQt.cpp:
> +	   (WebCore::GraphicsContext::clipOut): Use
QPainter::clipBoundingRect() with Qt >= 4.8.
> +	   (WebCore::GraphicsContext::clipOutEllipseInRect): Ditto.
> +
>  2010-08-11  Adam Barth  <abarth at webkit.org>
>  
>	   Reviewed by Eric Seidel.
> diff --git a/WebCore/platform/graphics/qt/GraphicsContextQt.cpp
b/WebCore/platform/graphics/qt/GraphicsContextQt.cpp
> index
d4a145f9f1e875c4bf012fd24fba24d5c1771257..43d0f7eb66e69240312d6e6a7c4a902619a61
25a 100644
> --- a/WebCore/platform/graphics/qt/GraphicsContextQt.cpp
> +++ b/WebCore/platform/graphics/qt/GraphicsContextQt.cpp
> @@ -1121,7 +1121,11 @@ void GraphicsContext::clipOut(const Path& path)
>      QPainterPath newClip;
>      newClip.setFillRule(Qt::OddEvenFill);
>      if (p->hasClipping()) {
> +#if QT_VERSION >= QT_VERSION_CHECK(4, 8, 0)
> +	   newClip.addRect(p->clipBoundingRect());
> +#else
>	   newClip.addRect(p->clipRegion().boundingRect());
> +#endif
>	   newClip.addPath(clippedOut);
>	   p->setClipPath(newClip, Qt::IntersectClip);
>      } else {
> @@ -1191,7 +1195,11 @@ void GraphicsContext::clipOut(const IntRect& rect)
>      QPainterPath newClip;
>      newClip.setFillRule(Qt::OddEvenFill);
>      if (p->hasClipping()) {
> +#if QT_VERSION >= QT_VERSION_CHECK(4, 8, 0)
> +	   newClip.addRect(p->clipBoundingRect());
> +#else
>	   newClip.addRect(p->clipRegion().boundingRect());
> +#endif
>	   newClip.addRect(QRect(rect));
>	   p->setClipPath(newClip, Qt::IntersectClip);
>      } else {
> @@ -1213,7 +1221,11 @@ void GraphicsContext::clipOutEllipseInRect(const
IntRect& rect)
>      QPainterPath newClip;
>      newClip.setFillRule(Qt::OddEvenFill);
>      if (p->hasClipping()) {
> +#if QT_VERSION >= QT_VERSION_CHECK(4, 8, 0)
> +	   newClip.addRect(p->clipBoundingRect());
> +#else
>	   newClip.addRect(p->clipRegion().boundingRect());
> +#endif
>	   newClip.addEllipse(QRect(rect));
>	   p->setClipPath(newClip, Qt::IntersectClip);
>      } else {