This bug tracks the move of the WebKit2-Qt build from QGraphicsView to the Qt Scene Graph. The goals are: 1) Building WebKit2-Qt requires Qt 5 2) The rendering is done exclusively with the Qt Scene Graph - we don't need to support any other rendering paths (like QGraphicsView or QWidget) 3) No patches that remove Qt 4 support should be landed until the build bots are set up accordingly
Proposed steps: 1) Move QGrahicsWKView over to a QSGPainted item first and make the MiniBrowser use QSGCanvas. (simple patch work in progress) 2) Then we turn BrowserView.cpp/h into the class that controls the viewport, i.e. scroll position, etc. 3) Make QGrahpicsWKView a plain QObject, make the tiles scene graph nodes, make BrowserView.cpp/h tell QGraphicsWKView the scroll positions, etc. and let QGraphicsWKView position the nodes accordingly (maybe let TiledDrawingAreaProxyQt actually do that :) 4) Move BrowserView.cpp/h into the library, give it a better name, merge QGraphicsWKView/QWKPage (or improve the split simply) 5) Move MiniBrowser over to real QML 6) ... 7) Profit :)
Created attachment 97771 [details] Simple patch to illustrate scene graph port This is a simple patch (500 loc) that is work in progress that illustrates the porting path to the scene graph, initially based on the FBO based painted item. It requires QML_NO_THREADED_RENDERER=1: paint() is otherwise called from the rendering thread, which will start a timer that was constructed in the gui thread. That's not possible. I think the NO_THREADED_RENDERER workaround is fine since we anyway want to move away from PaintedItem and position the tiles in the scene graph ourselves. If the node updates can only be done from the rendering thread, then we need to find another solution for the timer problem...
Simon, Similar work already happening in http://gitorious.net/+mob-team/webkit/mob-team-qtwebkit2/commits/qml2-inprogress-sl We can probably collaborate. Thanks -- Gopal
Comment on attachment 97771 [details] Simple patch to illustrate scene graph port Looks pretty nice and sounds like a good plan
Created attachment 100807 [details] Patch
Created attachment 100808 [details] Patch
Sorry about the previous patch, I forgot to squash one local change. This is very the basics to get the existing code running on Qt 5. More changes are needed to get the webview to be more stable and to have good performance.
Comment on attachment 100808 [details] Patch Attachment 100808 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9064231
Comment on attachment 100808 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100808&action=review This patch works actually pretty nicely :) Some comments: > Source/WebKit2/UIProcess/qt/TouchViewInterface.cpp:76 > -void TouchViewInterface::pinchGestureRequestUpdate(const QPointF& pinchCenterInPageViewCoordinate, qreal totalScaleFactor) > +void TouchViewInterface::pinchGestureRequestUpdate(const QPointF&, qreal totalScaleFactor) > { > - // FIXME: it is a bit more complicated than that, changes of the center position should move the page even > - // if the zoom factor does not change. Both the zoom and the panning should be handled through the physics > - // engine. > + // FIXME: it is a more complicated than that: > + // -the scale should be done centered on the pinch center. > + // -changes of the center position should move the page even if the zoom factor > + // does not change. Both the zoom and the panning should be handled through the physics engine. > const qreal scale = m_pinchStartScale * totalScaleFactor; > - m_pageView->setTransformOriginPoint(pinchCenterInPageViewCoordinate); > m_pageView->setScale(scale); Even though you added the FIXME, this makes pinch zooming pretty difficult to use meanwhile. This should work OK I think: QPointF oldPinchCenterOnParent = m_pageView->mapToItem(m_pageView->parentItem(), pinchCenterInPageViewCoordinate); m_pageView->setScale(scale); QPointF newPinchCenterOnParent = m_pageView->mapToItem(m_pageView->parentItem(), pinchCenterInPageViewCoordinate); m_pageView->setPos(m_pageView->pos() - (newPinchCenterOnParent - oldPinchCenterOnParent)); > Tools/MiniBrowser/qt/main.cpp:39 > + qputenv("QML_NO_THREADED_RENDERER", QByteArray("1")); It would be nice to have a comment on why this is needed here, so that we can know when we can remove it.
Comment on attachment 100808 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100808&action=review r=me. I must say the event delivery looks pretty disgusting, but there's not much we can do about that here. :( All in all, looks like a good place to start. Please consider Jocelyn's comments before landing. > Source/WebKit2/ChangeLog:14 > + QTouchWebView become a QSGItem with the only role of clipping > + children items. become -> becomes children items -> child items > Source/WebKit2/ChangeLog:17 > + QTouchWebPage become a QSGPaintedItem in order to render the content > + of the flattened page. This is a temporary work around to get > + something running withtout deep change on the drawing area proxy. become -> becomes withtout -> without deep change -> deep changes
Comment on attachment 100808 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100808&action=review > Source/WebKit2/UIProcess/API/qt/qdesktopwebview.h:31 > +#include <stdio.h> Derp.
Created attachment 102160 [details] Patch
Comment on attachment 102160 [details] Patch Clearing flags on attachment: 102160 Committed r91863: <http://trac.webkit.org/changeset/91863>
All reviewed patches have been landed. Closing bug.
(In reply to comment #13) > (From update of attachment 102160 [details]) > Clearing flags on attachment: 102160 > > Committed r91863: <http://trac.webkit.org/changeset/91863> It broke the WK2 build. :-/ http://build.webkit.sed.hu/builders/x86-32%20Linux%20Qt%20Release%20WebKit2/builds/9717/steps/compile-webkit/logs/stdio Reopen to fix it.
I really don't understand why did you land code which doesn't build ...
(In reply to comment #16) > I really don't understand why did you land code which doesn't build ... Calm down Ossy! Our environments are different especially with Qt5. I'm sure it built in the environment where it was developed.
(In reply to comment #17) > (In reply to comment #16) > > I really don't understand why did you land code which doesn't build ... > > Calm down Ossy! Our environments are different especially with Qt5. I'm sure it built in the environment where it was developed. But we should have _same_ environment as all developers ... A bot with different environment is absolutely useless ... And I think the author of the patch should check the bot after his commit.
(In reply to comment #18) > (In reply to comment #17) > > (In reply to comment #16) > > > I really don't understand why did you land code which doesn't build ... > > > > Calm down Ossy! Our environments are different especially with Qt5. I'm sure it built in the environment where it was developed. > > But we should have _same_ environment as all developers ... > A bot with different environment is absolutely useless ... > I need to quote the Black Adder: "have you ever take a journey on earth"? :)
(In reply to comment #18) > (In reply to comment #17) > > (In reply to comment #16) > > > I really don't understand why did you land code which doesn't build ... > > > > Calm down Ossy! Our environments are different especially with Qt5. I'm sure it built in the environment where it was developed. > > But we should have _same_ environment as all developers ... > A bot with different environment is absolutely useless ... > > And I think the author of the patch should check the bot after his commit. The Qt5 environment is not yet stable, so you should really calm down a bit. I updated the Qt5 env on the bot, and now the patch builds.