Bug 203561 – [Web Animations] Optimize blending for CSS Transitions

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Antoine Quint]

[Web Animations] Optimize blending for CSS Transitions

[Antoine Quint]

Created attachment 382178 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/56705079>

[Antoine Quint]

Created attachment 382183 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 382183 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=382183&action=review

> Source/WebCore/animation/KeyframeEffect.h:144
> +    enum class BlendingKeyframesComputationSource : uint8_t { CSSAnimation, CSSTransition, WebAnimation };

This is a bit longwinded. Maybe KeyframesSource?

[Antoine Quint]

(In reply to Simon Fraser (smfr) from comment #4)
> Comment on attachment 382183 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=382183&action=review
> 
> > Source/WebCore/animation/KeyframeEffect.h:144
> > +    enum class BlendingKeyframesComputationSource : uint8_t { CSSAnimation, CSSTransition, WebAnimation };
> 
> This is a bit longwinded. Maybe KeyframesSource?

We have two types of keyframes: m_parsedKeyframes and m_blendingKeyframes. So I think the Blending part of the name is important. However, I guess we can drop the  Computation part.

[Antoine Quint]

Created attachment 382185 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 382185 [details]
Patch for landing

Rejecting attachment 382185 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 382185, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Traceback (most recent call last):
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch", line 84, in <module>
    main()
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch", line 79, in main
    WebKitPatch(os.path.abspath(__file__)).main()
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 305, in main
    result = command.check_arguments_and_execute(options, args, self)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 123, in check_arguments_and_execute
    return self.execute(options, args, tool) or 0
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 55, in execute
    self._sequence.run_and_handle_errors(tool, options, state)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 73, in run_and_handle_errors
    self._run(tool, options, state)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 67, in _run
    step(tool, options).run(state)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 53, in run
    changelog_entry = ChangeLog(changelog_path).latest_entry()
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/common/checkout/changelog.py", line 379, in latest_entry
    return self.parse_latest_entry_from_file(changelog_file)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/common/checkout/changelog.py", line 325, in parse_latest_entry_from_file
    return next(cls.parse_entries_from_file(changelog_file))
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/common/checkout/changelog.py", line 364, in parse_entries_from_file
    yield ChangeLogEntry(''.join(entry_lines[:-1]), revision=most_probable_revision)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/common/checkout/changelog.py", line 114, in __init__
    self._parse_entry()
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/common/checkout/changelog.py", line 232, in _parse_entry
    self._author = self._committer_list.contributor_by_email(self.author_email()) or self._committer_list.contributor_by_name(self.author_name())
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/common/config/committers.py", line 386, in contributor_by_email
    return self._email_to_account_map().get(email.lower()) if email else None
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/common/config/committers.py", line 289, in _email_to_account_map
    assert(email not in self._accounts_by_email)  # We should never have duplicate emails.
AssertionError

Full output: https://webkit-queues.webkit.org/results/13188437

[Antoine Quint]

Committed r251706: <https://trac.webkit.org/changeset/251706>

[Truitt Savell]

The changes in https://trac.webkit.org/changeset/251706/webkit

broke webanimations/empty-keyframes-crash.html casting it to crash on Mac debug.

History: https://results.webkit.org/?suite=layout-tests&test=webanimations%2Fempty-keyframes-crash.html

Log:

Application Specific Information:
CRASHING TEST: webanimations/empty-keyframes-crash.html

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x00000002eb1b4810 WTFCrash + 16 (Assertions.cpp:305)
1   com.apple.WebCore             	0x00000002d8006c6b WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.WebCore             	0x00000002d9d78ff8 WebCore::KeyframeEffect::setAnimatedPropertiesInStyle(WebCore::RenderStyle&, double) + 168 (KeyframeEffect.cpp:1070)
3   com.apple.WebCore             	0x00000002d9d78ce3 WebCore::KeyframeEffect::apply(WebCore::RenderStyle&) + 195 (KeyframeEffect.cpp:1029)


https://build.webkit.org/results/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/r251715%20(10173)/webanimations/empty-keyframes-crash-crash-log.txt

[Antoine Quint]

Reopening to attach new patch.

[Antoine Quint]

Created attachment 382292 [details]
Patch

[Dean Jackson]

Comment on attachment 382292 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=382292&action=review

> Source/WebCore/animation/KeyframeEffect.cpp:1075
> +        for (auto cssPropertyId : properties)
> +            CSSPropertyAnimation::blendProperties(this, cssPropertyId, &targetStyle, m_blendingKeyframes[0].style(), m_blendingKeyframes[1].style(), iterationProgress);

I think you can do *properties.begin()