WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
27398
Handle opacity and opacity animations on transform layers in Leopard
https://bugs.webkit.org/show_bug.cgi?id=27398
Summary
Handle opacity and opacity animations on transform layers in Leopard
Chris Marrin
Reported
2009-07-17 17:05:56 PDT
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.
Attachments
Patch
(8.04 KB, patch)
2009-07-17 17:11 PDT
,
Chris Marrin
simon.fraser
: review-
Details
Formatted Diff
Diff
Replacement patch
(8.64 KB, patch)
2009-07-20 09:29 PDT
,
Chris Marrin
simon.fraser
: review-
Details
Formatted Diff
Diff
Replacement patch
(8.47 KB, patch)
2009-07-20 15:14 PDT
,
Chris Marrin
no flags
Details
Formatted Diff
Diff
Fixes a typo in the previous patch
(8.47 KB, patch)
2009-07-20 15:16 PDT
,
Chris Marrin
simon.fraser
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Chris Marrin
Comment 1
2009-07-17 17:11:04 PDT
Created
attachment 32990
[details]
Patch
Simon Fraser (smfr)
Comment 2
2009-07-17 17:41:53 PDT
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
Comment 3
2009-07-20 09:29:37 PDT
Created
attachment 33089
[details]
Replacement patch Patch with suggested changes from Simon
Simon Fraser (smfr)
Comment 4
2009-07-20 12:15:26 PDT
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
Comment 5
2009-07-20 15:14:23 PDT
Created
attachment 33112
[details]
Replacement patch
Chris Marrin
Comment 6
2009-07-20 15:16:11 PDT
Created
attachment 33113
[details]
Fixes a typo in the previous patch
Simon Fraser (smfr)
Comment 7
2009-07-20 15:33:47 PDT
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
Comment 8
2009-07-20 15:45:59 PDT
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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug