Bug 41310 – [Qt] Need to implement GraphicsContextQt::clipConvexPolygon()

• Component: CSS
• Status: RESOLVED FIXED

[Beth Dakin]

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.

[Simon Hausmann]

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?

[Kenneth Rohde Christiansen]

Apparently it is supposed to be clipped with antialiased clipping.

[Simon Hausmann]

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

[Beth Dakin]

(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.

[Kenneth Rohde Christiansen]

(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!

[Ariya Hidayat]

Last time I checked, Qt does not support anti-aliased clipping.

[Simon Hausmann]

(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.

[Ariya Hidayat]

> Raster, OpenGL and to some extent OpenVG do support anti-aliased clipping.

Ah, my bad. This sounds good!

Anyway, I can work on this.

[Ariya Hidayat]

Created attachment 63946 [details]
Patch

[Ariya Hidayat]

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...

[Kenneth Rohde Christiansen]

Comment on attachment 63946 [details]
Patch

Looks fine to me! Great to have you back Ariya!

[Ariya Hidayat]

Comment on attachment 63946 [details]
Patch

Clearing flags on attachment: 63946

Committed r65017: <http://trac.webkit.org/changeset/65017>

[Ariya Hidayat]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

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