[Web Animations] Ensure renderers with accelerated animations have layers
We have done some work already in https://bugs.webkit.org/show_bug.cgi?id=189784 to prevent never-ending calls to DocumentTimeline::updateAnimations(). This was due to the change made for https://bugs.webkit.org/show_bug.cgi?id=186930 where we queued calls to updateAnimations() in KeyframeEffectReadOnly::applyPendingAcceleratedActions() while we were waiting for a renderer with a layer backing for a given animation target. Instead of doing this, we ought to ensure renderers always have a layer when they have an accelerated animation applied.
<rdar://problem/44791222>
Created attachment 350852 [details] Patch
Comment on attachment 350852 [details] Patch Attachment 350852 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/9354714 New failing tests: transforms/3d/general/perspective-non-layer.html fast/table/overflow-table-collapsed-borders-section-self-painting-layer-painting.html fast/transforms/transform-on-inline.html fast/reflections/inline-crash.html fast/table/overflow-table-collapsed-borders-section-self-painting-layer-table-self-painting-layer.html
Created attachment 350853 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 350852 [details] Patch Attachment 350852 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/9354760 New failing tests: transforms/3d/general/perspective-non-layer.html fast/table/overflow-table-collapsed-borders-section-self-painting-layer-painting.html fast/transforms/transform-on-inline.html fast/reflections/inline-crash.html fast/table/overflow-table-collapsed-borders-section-self-painting-layer-table-self-painting-layer.html
Created attachment 350854 [details] Archive of layout-test-results from ews100 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 350856 [details] Patch
Comment on attachment 350856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350856&action=review > Source/WebCore/rendering/RenderBoxModelObject.cpp:2697 > + return timeline->runningAnimationsForElementAreAllAccelerated(*node); I hope this function is not going to show up in traces.
Committed r236501: <https://trac.webkit.org/changeset/236501>
Comment on attachment 350856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350856&action=review > Source/WebCore/rendering/RenderBox.h:57 > return isDocumentElementRenderer() || isPositioned() || createsGroup() || hasClipPath() || hasOverflowClip() > || hasTransformRelatedProperty() || hasHiddenBackface() || hasReflection() || style().specifiesColumns() > - || !style().hasAutoZIndex(); > + || !style().hasAutoZIndex() || hasRunningAcceleratedAnimations(); > } This is pretty hot code. I'm worried about perf impact. > Source/WebCore/rendering/RenderInline.h:123 > + bool requiresLayer() const override { return isInFlowPositioned() || createsGroup() || hasClipPath() || willChangeCreatesStackingContext() || hasRunningAcceleratedAnimations(); } This is pretty hot code. I'm worried about perf impact.
I suppose we could set a flag on the RenderBoxModelObject to indicate that an accelerated animation is in progress as the accelerated actions are updated on the KeyframeEffectReadOnly. Then it would just be a bit to read on the RenderBoxModelObject.
We used to have a bit on the style, and removed it. I think for a time we also used to have a bit on renderers, and that was removed too (I can't find the patches).
(In reply to Simon Fraser (smfr) from comment #13) > We used to have a bit on the style, and removed it. I think for a time we > also used to have a bit on renderers, and that was removed too (I can't find > the patches). Do you have a preference between the two?
Reopening to attach new patch.
Created attachment 350956 [details] Patch
Comment on attachment 350956 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350956&action=review > Source/WebCore/rendering/RenderBoxModelObject.h:333 > + bool m_hasRunningAcceleratedAnimations { false }; smfr, what's our current (memory)policy about adding bitfield like members to RenderObject subclasses? Should this be part of RenderObject's boolean bitfield (or even rare data)?
Created attachment 351247 [details] Patch
Committed r236670: <https://trac.webkit.org/changeset/236670>