[webkit-reviews] review granted: [Bug 80714] [Qt] [WK2] Shouldn't use item for clipping rect calculation in paint node. : [Attachment 131115] Updated patch

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam.rosenthal at nokia.com> has granted Viatcheslav Ostapenko
<ostapenko.viatcheslav at nokia.com>'s request for review:
Bug 80714: [Qt] [WK2] Shouldn't use item for clipping rect calculation in paint
node.
https://bugs.webkit.org/show_bug.cgi?id=80714

Attachment 131115: Updated patch
https://bugs.webkit.org/attachment.cgi?id=131115&action=review

------- Additional Comments from Noam Rosenthal <noam.rosenthal at nokia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=131115&action=review


This is good. Let's fix the nitpicks and commit.

> Source/WebKit2/ChangeLog:11
> +	   Replace item based clipping rect calculation with clipping nodes
based.
> +	   Part of threaded rendering implementation.
> +	   In Qt5 threaded rendering paint nodes live on paint thread and items
on 
> +	   main thread. Paint nodes should not have any direct access to items.


Some rewording if you don't mind :)
Replace item based clip-rect calculation with clipping-nodes based calculation.

This is required for threaded rendering, since we don't have access to the
QSGItems from the render thread.

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:25
> +#include <QPolygonF>

This belongs at the end, before the QtQuick includes.

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:126
> +	   QRectF resultRect(0, 0, -1, -1);

Comment: start with an invalid rect.

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:142
> +		   if (geometry->vertexCount() > 2) {

<nitpick>
if (geometry->vertexCount() < 3)
    continue;

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:151
> +	       if (!currentClip.isEmpty()) {

if (currentClip.isEmpty())
    continue;