Bug 43416 – [Qt] Implement GraphicsContext::clipOut more efficiently

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Simon Hausmann]

The current implementation of the function uses QPainter::clipRect().boundingRect(), which is very slow and with complex content it can make text selection very slow or even cause crashes in Qt due to complex regions.

http://bugreports.qt.nokia.com/browse/QTBUG-12618 in Qt tracks adding support for a faster method in Qt that would allow to quickly retrieve the clip bounding rect in device coordinates.

Once that function is implemented, then we can use it in GraphicsContexQt and transform the return value into logical coordinates.

[Simon Hausmann]

Created attachment 64096 [details]
Patch

[Ariya Hidayat]

Comment on attachment 64096 [details]
Patch

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index efc27d6e5a204eaeba7775d0a099627bc29adaaf..6b790604d12511157df404aeaad32e0b434add46 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,21 @@
> [phone]  Simon Hausmann  <[e-mail]>
> +
> +        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  <[e-mail]>
>  
>          Reviewed by Eric Seidel.
> diff --git a/WebCore/platform/graphics/qt/GraphicsContextQt.cpp b/WebCore/platform/graphics/qt/GraphicsContextQt.cpp
> index d4a145f9f1e875c4bf012fd24fba24d5c1771257..43d0f7eb66e69240312d6e6a7c4a902619a6125a 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 {

[Andreas Kling]

Comment on attachment 64096 [details]
Patch

Great stuff :) Setting cq+ since Simon is traveling this week.

[WebKit Commit Bot]

Comment on attachment 64096 [details]
Patch

Clearing flags on attachment: 64096

Committed r65650: <http://trac.webkit.org/changeset/65650>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.