[Webkit-unassigned] [Bug 56929] [Qt] Tiled painting still causes synchronous layout when accelerated compositing and texture mapper are enabled

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=56929





--- Comment #16 from Carol Szabo <carol.szabo at nokia.com>  2011-03-31 09:24:31 PST ---
(In reply to comment #15)
> > +    if (!layers & (QWebFrame::PanIconLayer | QWebFrame::ScrollBarLayer))
> I am pretty sure ! has precedence over &.

Sorry, I was mostly just copying code here and I missed the issue.

> >> Source/WebKit/qt/tests/qgraphicswebview/resources/56929.html:7
> >> +//  resizeDiv();
This was here for easy debugging, I'll take it out, this is also why I put the page as a resource, so that it can be easily loaded in a browser to see what I am after.

> >> Source/WebKit/qt/tests/qgraphicswebview/tst_qgraphicswebview.cpp:25
> >> +#if defined(ENABLE_TILED_BACKING_STORE) && ENABLE_TILED_BACKING_STORE
> > 
> > You can remove the #ifdefine here. This will still build without ENABLE_TILED_BACKING_STORE so we can avoid adding complexity here.

Yes, it will build without TILED_BACKING_STORE enabled, but it will fail at runtime, because the assertions do not hold if TILED_BACKING_STORE is not enabled. Regular rendering requires layout in paint, which will show the scrollbars and an inconsistent image (remember bug 49184, still did not get around to it, and probably I never will as I am getting ready to leave the WebKit arena).

> > Source/WebKit/qt/tests/qgraphicswebview/tst_qgraphicswebview.cpp:205
> > +    QGraphicsView view(new QGraphicsScene(QRectF(0.0, 0.0, 100.0, 100.0)));
> Aren't you leaking the QGraphicsScene? I don't think the scene is reparented to the view since you can have multiple views for a single scene.
Yes, I am. Originally I constructed the scene passing view as its parent, but that caused a crash since the view was not yet initialized. I later removed the parent and have not accounted for that.

> > Source/WebKit/qt/tests/qgraphicswebview/tst_qgraphicswebview.cpp:208
> > +    webView.setGeometry(view.sceneRect());
> 
> This looks strange, isn't the sceneRect() depending on what is in the scene() (so webView in this case)?
Actually not according to the 4.7 doc. What you are describing only applies if the said rect has not been explicitly set which I did in this case. (http://doc.qt.nokia.com/latest/qgraphicsscene.html#sceneRect-prop).

> > Source/WebKit/qt/tests/qgraphicswebview/tst_qgraphicswebview.cpp:217
> > +    // This will not paint anything as the tiles are not ready
> 
> <Kenneth>. at the end</Kenneth>
I like this it shows a really careful review (send my best to Kenneth, but this kind of review item is Darin Adler's trademark, I believe ;-)).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.