[webkit-reviews] review granted: [Bug 65528] [Qt][WK2] Create scene graph nodes for tiles in QTouchWebView instead of using imperative painting. : [Attachment 102776] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has granted Jocelyn Turcotte
<jocelyn.turcotte at nokia.com>'s request for review:
Bug 65528: [Qt][WK2] Create scene graph nodes for tiles in QTouchWebView
instead of using imperative painting.
https://bugs.webkit.org/show_bug.cgi?id=65528

Attachment 102776: Patch
https://bugs.webkit.org/attachment.cgi?id=102776&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=102776&action=review


> Source/WebKit2/UIProcess/API/qt/qtouchwebpage_p.h:35
>  
> +
>  class QTouchWebPagePrivate {

hum :)

> Source/WebKit2/UIProcess/TiledDrawingAreaProxy.cpp:111
>  
> -
>  static IntPoint innerBottomRight(const IntRect& rect)

Uh :)

>> Source/WebKit2/UIProcess/qt/SGAgent.cpp:99
>> +SGAgent::~SGAgent()
>> +{
>> +	foreach (NodeUpdate* nodeUpdate, nodeUpdatesQueue)
>> +	    delete nodeUpdate;
>> +}
> 
> Deque of OwnPtr and you could remove this, see comment later

Deque of OwnPtr and you could remove this, see comment later

> Source/WebKit2/UIProcess/qt/SGAgent.cpp:136
> +	       SGTileNode* tileNode  = new SGTileNode;

One space too many before the equal sign.

> Source/WebKit2/UIProcess/qt/SGAgent.cpp:137
> +	       QSGNode* parentNode = createTileUpdate->parentNodeID ?
nodes.value(createTileUpdate->parentNodeID) : itemNode;

Wouldn't that mean something went horribly wrong if
createTileUpdate->parentNodeID is 0?
I guess the tile should always be in the scale node of the tileset.

> Source/WebKit2/UIProcess/qt/SGAgent.cpp:144
> +	       QSGTransformNode* scaleNode  = new QSGTransformNode;

One space too many before the equal sign.

> Source/WebKit2/UIProcess/qt/SGAgent.cpp:172
> +	       ASSERT(false);

ASSERT_NOT_REACHED();