[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 #5 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2012-03-21 09:51:53 PST ---
(From update of attachment 133058)
View in context: https://bugs.webkit.org/attachment.cgi?id=133058&action=review

Any good tests? even manual ones?

> Source/WebCore/platform/graphics/GraphicsLayer.h:421
> +    void setFixedPosition(bool fixed) { m_fixedPosition = fixed; }
> +    bool fixedPosition() const { return m_fixedPosition; }

setUseFixedPosition? setIsFixedPositioned() ?

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:433
> +    PositionInformationMap::iterator end = graphicsLayer->fixedPositionInformation().end();
> +    for (PositionInformationMap::iterator it = graphicsLayer->fixedPositionInformation().begin(); it != end; ++it)
> +        m_state.m_fixedPositionInformation.add(it->first, it->second);

hmmmm, why do you need to copy this?

> Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:186
>  
> +void WebLayerTreeRenderer::adjustPositionForFixedPositionLayer(WebCore::GraphicsLayer* layer)
> +{
> +    ASSERT(layer->fixedPosition());

This should not need to be called unless we are doing overflow

> Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:197
> +            adjustedX = visibleEdgeLeft + left->second.getFloatValue() * m_visibleContentsRect.width() / 100;
> +        else // it->second.isFixed()
> +            adjustedX = visibleEdgeLeft + left->second.getFloatValue() * m_contentsScale;

IF you are zooming in the fixed elements will take over teh whole screen. iOS has a limit for this (say 2.0).

> Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:205
> +        if (right!= map.end()) {
> +            if  (right->second.isPercent())
> +                adjustedX = visibleEdgeRight - right->second.getFloatValue() * m_visibleContentsRect.width() / 100 - layer->size().width();
> +            else // it->second.isFixed()
> +                adjustedX = visibleEdgeRight - right->second.getFloatValue() * m_contentsScale - layer->size().width();

This should be using valueForLength (LengthFunctions.h), I believe

> Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:213
> +            adjustedY = visibleEdgeTop + top->second.getFloatValue() * m_visibleContentsRect.height() / 100;

top->second is pretty unreadable.

> Source/WebKit2/UIProcess/API/C/WKPreferencesPrivate.h:80
>  // Defaults to false.
> +WK_EXPORT void WKPreferencesSetAcceleratedCompositingForFixedPositionEnabled(WKPreferencesRef, bool);
> +WK_EXPORT bool WKPreferencesGetAcceleratedCompositingForFixedPositionEnabled(WKPreferencesRef);

Why are we exposing this to C? Are Apple guys interesting in using it? I would leave this out of this patch

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:124
> +    webPageProxy->pageGroup()->preferences()->setAcceleratedCompositingForFixedPositionEnabled(true);

Cant we just tie this to the resizeToContents and avoid the preference?

> Source/WebKit2/ChangeLog:10
> +        This is not done for pinching. Only when the pinch gesture is finished, the position is adjusted.

huh? why?

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