Bug 223173 - Use references instead of pointers as animation wrapper function arguments in CSSPropertyAnimation.cpp
Summary: Use references instead of pointers as animation wrapper function arguments in...
Status: RESOLVED DUPLICATE of bug 224433
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
: 223172 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-03-15 00:42 PDT by Antoine Quint
Modified: 2021-04-12 13:12 PDT (History)
5 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2021-03-15 00:42:53 PDT 
This is a followup to bug 223148 to address some additional review feedback that Darin Adler provided.
Comment 1 Antoine Quint 2021-03-15 03:09:41 PDT 
Actually, looking at the code again:

    double aspectRatioDst = WebCore::blend(log(a->logicalAspectRatio()), log(b->logicalAspectRatio()), progress);
    dst->setAspectRatio(exp(aspectRatioDst), 1);
    dst->setAspectRatioType(progress < 0.5 ? a->aspectRatioType() : b->aspectRatioType());

Since in the discrete case the progress is either 0 or 1, I think the blend function could be just those three lines. While it's a bit extra work to actually blend the values when we know it'll be one or the other, it simplifies the logic.

That assumes that canInterpolate() will correctly identify all cases where the animation should be discrete, which is its contract.
Comment 2 Antoine Quint 2021-03-15 03:12:46 PDT 
Don't mind this last comment, it was mistakenly typed in this bug rather than bug 220848.
Comment 3 Alexey Proskuryakov 2021-03-15 09:12:29 PDT 
*** Bug 223172 has been marked as a duplicate of this bug. ***
Comment 4 Radar WebKit Bug Importer 2021-03-22 04:35:57 PDT 
<rdar://problem/75685299>
Comment 5 Antoine Quint 2021-04-12 13:12:23 PDT 
Will address this in a larger refactor in bug 224433.

*** This bug has been marked as a duplicate of bug 224433 ***