[Webkit-unassigned] [Bug 76661] [Qt] [WK2] Support threaded renderer in WK2

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Viatcheslav Ostapenko <ostapenko.viatcheslav at nokia.com>  2012-01-19 16:10:07 PST ---
(In reply to comment #6)
> (From update of attachment 123209 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=123209&action=review
> 
> OK, needs lots of work :)
> 
> > Source/WebKit2/Target.pri:258
> > +    UIProcess/qt/LayerTreeHostProxyNodeQt.h \
> 
> Name is not by convention, since this is not a subclass of a WebCore/WebKit class.
> How about QtLayerTreeSceneGraphNode

Ok. I'm very bad in naming ;)

> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:95
> > +    return 0;
> 
> Why are we returning zero?

But what should it return if LayerTreeHostProxy is not allocated yet?
Allocation is moved to LayerTreeHostProxy::updatePaintNode

> > Source/WebKit2/UIProcess/LayerTreeHostProxy.h:41
> > +#include <QObject>
> 
> ?

Oops!

> > Source/WebKit2/UIProcess/LayerTreeHostProxy.h:68
> > +    void paintToCurrentGLContext(const WebCore::TransformationMatrix&, float, bool syncRemoteLayerData = true);
> 
> I tried to follow the logic of when syncRemoteLayerData is called and when not... I still don't get it.

It's simple.
In Qt case it will be never call from paintToCurrentGLContext, only from updatePaintNode . It left there with default parameter if others use this function.

> > Source/WebKit2/UIProcess/qt/LayerTreeHostProxyQt.cpp:187
> > +template<typename T> struct MainThreadMessageSender {
> > +    struct CallbackContext {
> > +        CallbackContext(const T& message, LayerTreeHostProxy* layerTreeHostProxy) :
> > +                m_messageID(T::messageID)
> > +            ,   m_layerTreeHostProxy(layerTreeHostProxy)
> > +        {
> > +            m_encoder = CoreIPC::ArgumentEncoder::create(0);
> > +            m_encoder->encode(message);
> > +        }
> > +        ~CallbackContext()
> > +        {
> > +        }
> > +        OwnPtr<CoreIPC::ArgumentEncoder> m_encoder;
> > +        CoreIPC::MessageID m_messageID;
> > +        RefPtr<LayerTreeHostProxy> m_layerTreeHostProxy;
> > +    };
> > +    static void mainThreadCallback(void* data)
> > +    {
> > +        CallbackContext* context = static_cast<CallbackContext*>(data);
> > +        DrawingAreaProxy* drawingAreaProxy = context->m_layerTreeHostProxy->drawingAreaProxy();
> > +        if (drawingAreaProxy) {
> > +            // Have to initialize destanation ID here because this data cannot be accessed from
> > +            // painting thread.
> > +            *reinterpret_cast<uint64_t*>(context->m_encoder->buffer()) = drawingAreaProxy->page()->pageID();
> > +            CoreIPC::Connection* connection = drawingAreaProxy->page()->process()->connection();
> > +            if (connection)
> > +                connection->sendMessage(context->m_messageID, context->m_encoder.release(), 0);
> > +        }
> > +        delete context;
> > +    }
> > +
> > +    static void sendMessageOnMainThread(const T& message, LayerTreeHostProxy* layerTreeHostProxy)
> > +    {
> > +        CallbackContext* context = new CallbackContext(message, layerTreeHostProxy);
> > +        callOnMainThread(&mainThreadCallback, context);
> > +    }
> > +};
> 
> You need to do all that just to call updateViewport on the main thread?
> 
> > Source/WebKit2/UIProcess/qt/LayerTreeHostProxyQt.cpp:220
> > +static updateViewportOnMainThread(LayerTreeHostProxy* layerTreeHostProxy)
> > +{
> > +    layerTreeHostProxy->updateViewport();
> > +}
> > +#endif
> 
> the pointer to LayerTreeHostProxy is not thread safe, since that proxy might be deleted before the painting stopped.

No, it can't. Paint node holds the reference to LayerTreeHostProxy. I've mentioned this in ChangeLog.

> > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:178
> > +    if (itemRect.isEmpty())
> > +        return;
> > +
> 
> Is this related?

I don't know, but I was getting division errors in this functions when I was running API tests. The item indeed was 0 size and was resized later.

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