Bug 43416

Summary: [Qt] Implement GraphicsContext::clipOut more efficiently
Product: WebKit Reporter: Simon Hausmann <hausmann>
Component: PlatformAssignee: Simon Hausmann <hausmann>
Status: RESOLVED FIXED    
Severity: Normal CC: ariya.hidayat, benjamin, commit-queue, kling
Priority: P3 Keywords: Performance, Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch none

Simon Hausmann
Reported 2010-08-03 06:09:48 PDT
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.
Attachments
Patch (2.78 KB, patch)
2010-08-11 04:50 PDT, Simon Hausmann 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Simon Hausmann
Comment 1 2010-08-11 04:50:33 PDT
Created attachment 64096 [details] Patch
Ariya Hidayat
Comment 2 2010-08-11 04:55:57 PDT
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 @@ > +2010-08-11 Simon Hausmann <simon.hausmann@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@webkit.org> > > 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 3 2010-08-18 20:36:50 PDT
Comment on attachment 64096 [details] Patch Great stuff :) Setting cq+ since Simon is traveling this week.
WebKit Commit Bot
Comment 4 2010-08-18 22:20:47 PDT
Comment on attachment 64096 [details] Patch Clearing flags on attachment: 64096 Committed r65650: <http://trac.webkit.org/changeset/65650>
WebKit Commit Bot
Comment 5 2010-08-18 22:20:51 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.