Bug 27398 – Handle opacity and opacity animations on transform layers in Leopard

• Component: CSS
• Status: RESOLVED FIXED

[Chris Marrin]

QuartzCore in Leopard does not support opacity on TransformLayers. So we must propagate opacity down the tree to the nearest non-transform layer and also into the transform layer's content layer. Because of this we will need to do opacity animartion in software because (not surprisingly) TransformLayers in Leopard also don't support animation of opacity.

[Chris Marrin]

Created attachment 32990 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 32990 [details]
Patch

> Index: WebCore/ChangeLog
> ===================================================================

> +        Second, I turn off all opacity animation.

Not always, I hope!

> Index: WebCore/platform/graphics/GraphicsLayer.h
> ===================================================================

>  private:
> +    void setOpacityOnLayer();

I think it should be called "update" if it takes no argument, or "set" if it takes one.

> Index: WebCore/platform/graphics/mac/GraphicsLayerCA.mm
> ===================================================================

> +float GraphicsLayerCA::accumulatedOpacity() const
> +{
> +    if (!preserves3D())
> +        return 1;
> +        
> +    return m_opacity * (parent() ? parent()->accumulatedOpacity() : 1);
> +}
> +
> +void GraphicsLayerCA::distributeOpacity(float opacity)

Argument could be "accumulatedOpacity" for clarity.

> +{
> +    // If this is a transform layer we need to distribute our opacity to all our children
> +    // and set the opacity on our m_layer. Otherwise we just set our own opacity.
> +    
> +    // Incoming opacity is the contribution from our parent(s). We mutiply this by our own
> +    // opacity to get the value that is either passed down to the children or set on our
> +    // own layer.
> +    opacity *= m_opacity;
> +    
> +    if (preserves3D()) {
> +        if (m_layer.get())
> +            [m_layer.get() setOpacity: opacity];
> +
> +        size_t numChildren = children().size();
> +        for (size_t i = 0; i < numChildren; ++i)
> +            children()[i]->distributeOpacity(opacity);
> +    } else
> +        [primaryLayer() setOpacity: opacity];
> +}

I think put the platform-neutral logic into base class methods, and then add a small virtual function 
to wrap the [layer setOpacity:] call. BTW, webcore style is no space after colons.

[Chris Marrin]

Created attachment 33089 [details]
Replacement patch

Patch with suggested changes from Simon

[Simon Fraser (smfr)]

Comment on attachment 33089 [details]
Replacement patch


> +float GraphicsLayer::accumulatedOpacity() const
> +{
> +    if (!preserves3D())
> +        return 1;
> +        
> +    return m_opacity * (parent() ? parent()->accumulatedOpacity() : 1);
> +}
> +
> +void GraphicsLayer::distributeOpacity(float accumulatedOpacity)
> +{
> +    // If this is a transform layer we need to distribute our opacity to all our children
> +    
> +    // Incoming accumulatedOpacity is the contribution from our parent(s). We mutiply this by our own
> +    // opacity to get the total contribution
> +    accumulatedOpacity *= m_opacity;
> +    
> +    if (preserves3D()) {
> +        size_t numChildren = children().size();
> +        for (size_t i = 0; i < numChildren; ++i)
> +            children()[i]->distributeOpacity(accumulatedOpacity);
> +    }
> +}



> +void GraphicsLayerCA::distributeOpacity(float accumulatedOpacity)
> +{
> +    GraphicsLayer::distributeOpacity(accumulatedOpacity);
> +    
> +    // If this is a transform layer we need set the opacity on our m_layer. Otherwise 
> +    // we just set our own opacity.
> +    accumulatedOpacity *= m_opacity;
> +    
> +    if (preserves3D()) {
> +        if (m_layer.get())
> +            [m_layer.get() setOpacity: accumulatedOpacity];
> +    } else
> +        [primaryLayer() setOpacity: accumulatedOpacity];
> +}

I don't like this duplication of logic between the base class and the derived class. Logic should live in just one place.

You still have spaces after colons.

I'm also a little concerned about the fact that you're walking the GraphicsLayer tree, rather than the RenderLayer tree to distribute opacity. This only happens to work because we have other logic that makes compositing layers on ancestors of layers with opacity < 1 (in computeCompositingRequirements()). That logic may changes, so this code needs to work when a composited layer has a software-rendered parent with opacity < 1.

[Chris Marrin]

Created attachment 33112 [details]
Replacement patch

[Chris Marrin]

Created attachment 33113 [details]
Fixes a typo in the previous patch

[Simon Fraser (smfr)]

Comment on attachment 33113 [details]
Fixes a typo in the previous patch

> Index: WebCore/ChangeLog
> ===================================================================

> +        * platform/graphics/GraphicsLayer.h:
> +        (WebCore::GraphicsLayer::distributeOpacity):
> +        (WebCore::GraphicsLayer::accumulatedOpacity):
> +        * platform/graphics/mac/GraphicsLayerCA.h:
> +        * platform/graphics/mac/GraphicsLayerCA.mm:
> +        (WebCore::GraphicsLayerCA::setPreserves3D):
> +        (WebCore::GraphicsLayerCA::setOpacity):
> +        (WebCore::GraphicsLayerCA::animateFloat):
> +        (WebCore::GraphicsLayerCA::swapFromOrToTiledLayer):
> +        (WebCore::GraphicsLayerCA::accumulatedOpacity):
> +        (WebCore::GraphicsLayerCA::distributeOpacity):
> +        (WebCore::GraphicsLayerCA::setOpacityOnLayer):

This changelist seems stale.

> Index: WebCore/platform/graphics/GraphicsLayer.h
> ===================================================================

>  protected:
> +    virtual void distributeOpacityHelper(float) { }

Please rename this to internalSetOpacity(), or even setOpacityOnLayer(); the name should say what it does.

> Index: WebCore/platform/graphics/mac/GraphicsLayerCA.mm
> ===================================================================

> +void GraphicsLayerCA::distributeOpacityHelper(float accumulatedOpacity)
> +{
> +    [ (preserves3D() ? m_layer.get() : primaryLayer()) setOpacity:accumulatedOpacity];
> +}

You've got an extra space after the first [

r=me with those adjustments.

[Chris Marrin]

Sending        WebCore/ChangeLog
Sending        WebCore/platform/graphics/GraphicsLayer.cpp
Sending        WebCore/platform/graphics/GraphicsLayer.h
Sending        WebCore/platform/graphics/mac/GraphicsLayerCA.h
Sending        WebCore/platform/graphics/mac/GraphicsLayerCA.mm
Transmitting file data .....
Committed revision 46135.