Bug 41310

Summary: [Qt] Need to implement GraphicsContextQt::clipConvexPolygon()
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: CSSAssignee: Ariya Hidayat <ariya.hidayat>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ariya.hidayat, bdakin, benjamin, eric, hausmann, jesus, kenneth, webkit.review.bot
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch none

Description Beth Dakin 2010-06-28 15:05:10 PDT
http://trac.webkit.org/changeset/62035 introduced a new method of drawing border-radius using paths. Right now, this new code is only enabled for some platforms. To enable the new and much improved code for QT, GraphicsContext::clipConvexPolygon() needs to be implemented, and then QT should be added to the list of platforms that set #define HAVE_PATH_BASED_BORDER_RADIUS_DRAWING in RenderObject.h

I would like to note that QT already has a function implemented called GraphicsContext::drawConvexPolygon(). So hopefully it is straightforward to use some of that same logic for clipping instead of drawing.
Comment 1 Simon Hausmann 2010-07-01 03:57:55 PDT
Most straightforward way I can see is to put the polygon points into a QPainterPath and clip that one, kind of like before....

Any other suggestions?
Comment 2 Kenneth Rohde Christiansen 2010-07-01 04:51:57 PDT
Apparently it is supposed to be clipped with antialiased clipping.
Comment 3 Simon Hausmann 2010-07-01 06:54:45 PDT
The most straightforward patch like this didn't work for me ;(

diff --git a/WebCore/platform/graphics/qt/GraphicsContextQt.cpp b/WebCore/platform/graphics/qt/GraphicsContextQt.cpp
index d1a9318..4a0cadd 100644
--- a/WebCore/platform/graphics/qt/GraphicsContextQt.cpp
+++ b/WebCore/platform/graphics/qt/GraphicsContextQt.cpp
@@ -536,7 +536,10 @@ void GraphicsContext::clipConvexPolygon(size_t numPoints, const FloatPoint* poin
     if (numPoints <= 1)
         return;
     
-    // FIXME: IMPLEMENT!!
+    QPainterPath path(points[0]);
+    for (int i = 1; i < numPoints; ++i)
+        path.lineTo(points[i]);
+    m_data->p()->setClipPath(path, Qt::IntersectClip);
 }
 
 QPen GraphicsContext::pen()
diff --git a/WebCore/rendering/RenderObject.h b/WebCore/rendering/RenderObject.h
index 33271df..476b5ec 100644
--- a/WebCore/rendering/RenderObject.h
+++ b/WebCore/rendering/RenderObject.h
@@ -38,7 +38,7 @@
 #include "TransformationMatrix.h"
 #include <wtf/UnusedParam.h>
 
-#if PLATFORM(CG)
+#if PLATFORM(CG) || PLATFORM(QT)
 #define HAVE_PATH_BASED_BORDER_RADIUS_DRAWING 1
 #endif
Comment 4 Beth Dakin 2010-07-01 09:48:26 PDT
(In reply to comment #2)
> Apparently it is supposed to be clipped with antialiased clipping.

Hi Kenneth,

I just wanted to let you know, with regard to antialiased clipping, I am actually reconsidering this. We have some corner joint issues because of the antialiased polygon clipping. A great example of the worst case scenario: http://ie.microsoft.com/testdrive/Graphics/IE%20Logo/Default.html

You should do what seems right for your platform, but to warn you, I might be switching CG soon to non-antialiased polygon clipping.
Comment 5 Kenneth Rohde Christiansen 2010-07-01 11:20:12 PDT
(In reply to comment #4)
> (In reply to comment #2)
> > Apparently it is supposed to be clipped with antialiased clipping.
> 
> Hi Kenneth,
> 
> I just wanted to let you know, with regard to antialiased clipping, I am actually reconsidering this. We have some corner joint issues because of the antialiased polygon clipping. A great example of the worst case scenario: http://ie.microsoft.com/testdrive/Graphics/IE%20Logo/Default.html
> 
> You should do what seems right for your platform, but to warn you, I might be switching CG soon to non-antialiased polygon clipping.

Thanks for the heads-up!
Comment 6 Ariya Hidayat 2010-08-09 02:58:15 PDT
Last time I checked, Qt does not support anti-aliased clipping.
Comment 7 Simon Hausmann 2010-08-09 03:34:02 PDT
(In reply to comment #6)
> Last time I checked, Qt does not support anti-aliased clipping.

Raster, OpenGL and to some extent OpenVG do support anti-aliased clipping.
Comment 8 Ariya Hidayat 2010-08-09 08:14:53 PDT
> Raster, OpenGL and to some extent OpenVG do support anti-aliased clipping.

Ah, my bad. This sounds good!

Anyway, I can work on this.
Comment 9 Ariya Hidayat 2010-08-09 15:50:01 PDT
Created attachment 63946 [details]
Patch
Comment 10 Ariya Hidayat 2010-08-09 15:52:05 PDT
It's hard to automatically test this (with pixel test), since our border radius is pretty broken. I had to visually inspect some of the good ones.

Seems that fixing border radius is now in my TODO...
Comment 11 Kenneth Rohde Christiansen 2010-08-09 16:34:27 PDT
Comment on attachment 63946 [details]
Patch

Looks fine to me! Great to have you back Ariya!
Comment 12 Ariya Hidayat 2010-08-09 17:00:16 PDT
Comment on attachment 63946 [details]
Patch

Clearing flags on attachment: 63946

Committed r65017: <http://trac.webkit.org/changeset/65017>
Comment 13 Ariya Hidayat 2010-08-09 17:00:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 WebKit Review Bot 2010-08-09 21:00:43 PDT
http://trac.webkit.org/changeset/65017 might have broken Leopard Intel Debug (Tests)
The following changes are on the blame list:
http://trac.webkit.org/changeset/65017
http://trac.webkit.org/changeset/65018