Summary: | [Web Animations] Animation with a single keyframe is not accelerated | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | changseok, dino, esprehn+autocc, ews-watchlist, glenn, graouts, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Safari Technology Preview | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=214088 | ||||||||||
Attachments: |
|
Description
Antoine Quint
2018-08-19 07:59:07 PDT
Created attachment 399484 [details]
Patch
Comment on attachment 399484 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399484&action=review > Source/WebCore/animation/KeyframeEffect.cpp:1583 > + keyframes = WTFMove(explicitKeyframes); I think this leaves you with a ref to garbage data. > Source/WebCore/rendering/style/KeyframeList.cpp:97 > + int initialSize = size(); auto. size is size_t. > Source/WebCore/rendering/style/KeyframeList.cpp:102 > + static StyleRuleKeyframe* zeroPercentKeyframe; > + if (!zeroPercentKeyframe) { > + zeroPercentKeyframe = &StyleRuleKeyframe::create(MutableStyleProperties::create()).leakRef(); > + zeroPercentKeyframe->setKey(0); NeverDestroyed? > Source/WebCore/rendering/style/KeyframeList.cpp:115 > + static StyleRuleKeyframe* hundredPercentKeyframe; > + if (!hundredPercentKeyframe) { > + hundredPercentKeyframe = &StyleRuleKeyframe::create(MutableStyleProperties::create()).leakRef(); > + hundredPercentKeyframe->setKey(1); > + } Ditto Created attachment 399492 [details]
Patch
(In reply to Simon Fraser (smfr) from comment #3) > Comment on attachment 399484 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=399484&action=review > > > Source/WebCore/animation/KeyframeEffect.cpp:1583 > > + keyframes = WTFMove(explicitKeyframes); > > I think this leaves you with a ref to garbage data. It was actually setting m_blendingKeyframes, which is not what I intended. I now use a lambda to generate a new KeyframeList only when needed. > > Source/WebCore/rendering/style/KeyframeList.cpp:97 > > + int initialSize = size(); > > auto. size is size_t. Fixed in updated patch. > > Source/WebCore/rendering/style/KeyframeList.cpp:102 > > + static StyleRuleKeyframe* zeroPercentKeyframe; > > + if (!zeroPercentKeyframe) { > > + zeroPercentKeyframe = &StyleRuleKeyframe::create(MutableStyleProperties::create()).leakRef(); > > + zeroPercentKeyframe->setKey(0); > > NeverDestroyed? > > > Source/WebCore/rendering/style/KeyframeList.cpp:115 > > + static StyleRuleKeyframe* hundredPercentKeyframe; > > + if (!hundredPercentKeyframe) { > > + hundredPercentKeyframe = &StyleRuleKeyframe::create(MutableStyleProperties::create()).leakRef(); > > + hundredPercentKeyframe->setKey(1); > > + } > > Ditto Comment on attachment 399492 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399492&action=review > Source/WebCore/rendering/style/KeyframeList.cpp:81 > + return size() && (m_keyframes[0].key() || m_keyframes[size() - 1].key() != 1); Are you sure m_keyframes has values? > Source/WebCore/rendering/style/KeyframeList.cpp:103 > + static StyleRuleKeyframe* zeroPercentKeyframe; > + if (!zeroPercentKeyframe) { > + zeroPercentKeyframe = &StyleRuleKeyframe::create(MutableStyleProperties::create()).leakRef(); > + zeroPercentKeyframe->setKey(0); > + } Should do the NeverDestroyed thing that smfr suggested. (In reply to Dean Jackson from comment #6) > Comment on attachment 399492 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=399492&action=review > > > Source/WebCore/rendering/style/KeyframeList.cpp:81 > > + return size() && (m_keyframes[0].key() || m_keyframes[size() - 1].key() != 1); > > Are you sure m_keyframes has values? Yes, given size() is true. > > Source/WebCore/rendering/style/KeyframeList.cpp:103 > > + static StyleRuleKeyframe* zeroPercentKeyframe; > > + if (!zeroPercentKeyframe) { > > + zeroPercentKeyframe = &StyleRuleKeyframe::create(MutableStyleProperties::create()).leakRef(); > > + zeroPercentKeyframe->setKey(0); > > + } > > Should do the NeverDestroyed thing that smfr suggested. Let's try it. Created attachment 399501 [details]
Patch
Committed r261756: <https://trac.webkit.org/changeset/261756> |