[Webkit-unassigned] [Bug 80714] [Qt] [WK2] Shouldn't use item for clipping rect calculation in paint node.

[bugzilla-daemon at webkit.org]

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





--- Comment #3 from Viatcheslav Ostapenko <ostapenko.viatcheslav at nokia.com>  2012-03-09 14:05:06 PST ---
(In reply to comment #2)
> (From update of attachment 131067 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=131067&action=review
> 
> > Source/WebKit2/ChangeLog:8
> > +        Replace item based clipping rect calculation with clipping nodes based.
> 
> Explain that this is needed for threaded rendering, where you don't have access to the items.
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:121
> > +    static void adjustBoundingRect(QRectF& rect, const QPointF& point)
> 
> rename uniteRectWithPoint
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:131
> > +        if (rect.top() > point.y())
> > +            rect.setTop(point.y());
> > +        else if (rect.bottom() < point.y())
> > +            rect.setBottom(point.y());
> > +
> > +        if (rect.left() > point.x())
> > +            rect.setLeft(point.x());
> > +        else if (rect.right() < point.x())
> > +            rect.setRight(point.x());
> 
> rect |= QRectF(point, QSizeF(1, 1));

This will give rect bigger by 1 on right and bottom sides.
QSize(0, 0) will not work becase QRectF operator| has check for null rect and will return original rect.

> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:137
> > +        QRectF retRect(0, 0, -1, -1);
> 
> retRect -> resultRect
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:139
> > +        while (clip) {
> 
> Would be nicer as a for loop
> for (const QSGClipNode* clip = clipList(); clip; clip = clip->clipList)
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:140
> > +            QMatrix4x4 m;
> 
> m -> matrix or clipMatrix
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:142
> > +                m *= *clip->matrix();

Remaining artifacts ;)

> Why multiply instead of assign?
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:152
> > +                if (geometry->vertexCount() > 1) {
> 
> Shouldn't it be 2? I don't see the point in a single-line clip.
> Also, this requires a comment.

Yes, 2 make more sense.

> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:153
> > +                    currentClip.setTopLeft(m.map(QPoint(geometryPoints[0].x, geometryPoints[0].y)));
> 
> If you use my unite suggestion, you don't need this and can simply start from 0.

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