[Webkit-unassigned] [Bug 81786] Support fixed position elements in Qt WebKit2

[bugzilla-daemon at webkit.org]

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





--- Comment #20 from Yael <yael.aharon at nokia.com>  2012-03-23 20:44:14 PST ---
(In reply to comment #17)
thanks for the review,
> (From update of attachment 133414 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133414&action=review
> 
> Discussed this on IRC;
> I'd rather we don't pass around all this fixed-position information, and instead do something more aligned with how GraphicsLayer are offseted in WebCore, by setting a scrollOffset on all of them.

Please keep in mind that not all fixed position layers are moved by the same offset. If the position is specified in pixels, and the page is zoomed, that affects the offset differently than percentage.

> > Source/WebCore/page/FrameView.cpp:1717
> > +        if (m_frame->page()->settings()->acceleratedCompositingForFixedPositionEnabled()) {
> > +            updateFixedElementsAfterScrolling();
> > +            repaintFixedElementsAfterScrolling();
> > +        }
> 
> This probably needs a comment.
I might not need this change after all

> 
> > Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.h:109
> > +    HashMap<int, WebCore::Length> m_fixedPositionInformation;
> 
> I don't like using a map for 4 items. Please see previous comment.
ok, I'll change that as we talked on irc

> > Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:126
> > +    if (m_fixedLayers.size()) {
> > +        LayerMap::iterator end = m_fixedLayers.end();
> > +        for (LayerMap::iterator it = m_fixedLayers.begin(); it != end; ++it)
> > +            it->second->syncCompositingStateForThisLayerOnly();
> > +    }
> 
> Why do you need to sync these again?
We need to sync during scrolling. Otherwise, the fixed layer scrolls and jumps back when scrolling is finished. I'll see if I can avoid the jumps in a different way.

> > Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:167
> > +    m_contentsSize = contentsSize;
> 
> how is this different from DrawingArea::contentsRect()?
WebLayerTreeRenderer does not have access to DrawingArea and I did not want to change that. We already have WebLayerTreeRenderer::setVisibleContentsRect .

> > Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:196
> > +            adjustedX = visibleEdgeLeft + left.getFloatValue() * m_visibleContentsRect.width() / 100;
> 
> You're doing manually what WebCore::Length can do automatically.
Not sure how much I can reuse LengthFunctions. Length does not know about the scale, and I need to take that into account too.

> > Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.h:120
> > +    WebKit::PositionInformationMap& fixedPositionInformation() { return m_fixedPositionInformation; }
> > +    bool hasFixedPosition() const { return m_hasFixedPosition; }
> > +    void setHasFixedPosition(bool isFixed) { m_hasFixedPosition = isFixed; }
> > +
> 
> I'd rather call this
> fixedToViewport
> and offsetFromViewport

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