Bug 119955 – Support interpolation between cross-fade() images

• Component: CSS
• Status: RESOLVED FIXED

[Dirk Schulze]

Support interpolation between cross-fade() images:

cross-fade(url(t1.png), url(t2.png), 0%)

to

cross-fade(url(t1.png), url(t2.png), 100%)

[Dirk Schulze]

Created attachment 210459 [details]
Patch

[Darin Adler]

Comment on attachment 210459 [details]
Patch

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

> LayoutTests/ChangeLog:10
> +        * animations/cross-fade-background-image.html:

Can we change this into a reference test instead of a pixel test?

Also, we normally try to avoid having any red in expected results from tests, but I see red in this one.

> Source/WebCore/css/CSSCrossfadeValue.h:72
> +    bool equalInputImages(const CSSCrossfadeValue&) const;

Naming here is a bit awkward. The function name “equal input images” sounds like a function that returns two equal images, not a function that answers the question, do these fades have input images that are equal.

> Source/WebCore/page/animation/CSSPropertyAnimation.cpp:332
> +            CSSCrossfadeValue* fromCrossfade = toCSSCrossfadeValue(fromGenerated);
> +            CSSCrossfadeValue* toCrossfade = toCSSCrossfadeValue(toGenerated);
> +            if (fromCrossfade->equalInputImages(*toCrossfade)) {
> +                RefPtr<CSSCrossfadeValue> result = toCrossfade->blend(*fromCrossfade, progress);
> +                return StyleGeneratedImage::create(result.get());
> +            }

I recommend writing this with references instead of pointers and eliminating the local variable for tighter code:

    if (fromGenerated->isCrossfadeValue() && toGenerated->isCrossfadeValue()) {
        CSSCrossfadeValue& fromCrossfade = *toCSSCrossfadeValue(fromGenerated);
        CSSCrossfadeValue& toCrossFade = *toCSSCrossfadeValue(toGenerated);
        if (fromCrossfade.equalInputImages(toCrossfade))
            return StyleGeneratedImage::create(toCrossfade.blend(fromCrossfade, progress).get());
    }

Might even want to use shorter variable names since these have tiny scope.

[Dirk Schulze]

(In reply to comment #2)
> (From update of attachment 210459 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210459&action=review
> 
> > LayoutTests/ChangeLog:10
> > +        * animations/cross-fade-background-image.html:
> 
> Can we change this into a reference test instead of a pixel test?
> 
> Also, we normally try to avoid having any red in expected results from tests, but I see red in this one.
> 

The red is not intentional. I think this is a real failure but was already present before. See bug 120596.

I'll investigate.

[Dirk Schulze]

Created attachment 210482 [details]
Patch

[Dirk Schulze]

Created attachment 210544 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 210544 [details]
Patch

Rejecting attachment 210544 [details] from commit-queue.

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

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/1692787

[Dirk Schulze]

Created attachment 210550 [details]
Patch

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 210550 [details]:

css2.1/20110323/overflow-applies-to-012.htm bug 120727 (author: [e-mail])
css2.1/20110323/overflow-applies-to-013.htm bug 120728 (author: [e-mail])
css2.1/20110323/c541-word-sp-001.htm bug 120729 (author: [e-mail])
css2.1/20110323/overflow-applies-to-014.htm bug 120730 (author: [e-mail])
css2.1/20110323/overflow-applies-to-015.htm bug 120731 (author: [e-mail])
css2.1/20110323/overflow-applies-to-007.htm bug 120732 (author: [e-mail])
css2.1/20110323/overflow-applies-to-010.htm bug 120733 (author: [e-mail])
css2.1/20110323/border-conflict-element-001d.htm bug 120734 (author: [e-mail])
css2.1/20110323/text-indent-014.htm bug 120735 (author: [e-mail])
http/tests/inspector/inspect-element.html bug 78869 (author: [e-mail])
fast/regions/counters/extract-list-items-006.html bug 120736 (author: [e-mail])
fast/regions/counters/extract-list-items-015.html bug 120737 (author: [e-mail])
fast/regions/counters/extract-numbered-paragraphs-divs-002.html bug 120738 (author: [e-mail])
fast/regions/counters/extract-list-items-003.html bug 120739 (author: [e-mail])
fast/regions/counters/extract-list-items-001.html bug 120740 (author: [e-mail])
fast/regions/counters/extract-list-items-013.html bug 120741 (author: [e-mail])
fast/regions/counters/extract-list-items-014.html bug 120742 (author: [e-mail])
fast/inline/layout-after-inserting-nested-br.html bug 120743 (author: [e-mail])
fast/multicol/newmulticol/float-paginate.html bug 120744 (author: [e-mail])
fast/multicol/newmulticol/columns-shorthand-parsing.html bug 120745
fast/multicol/newmulticol/float-paginate-complex.html bug 120746 (author: [e-mail])
fast/multicol/newmulticol/direct-child-column-span-all.html bug 120747 (author: [e-mail])
fast/multicol/newmulticol/column-rules-fixed-height.html bug 120748 (author: [e-mail])
fast/multicol/newmulticol/positioned-split.html bug 120749 (author: [e-mail])
fast/multicol/newmulticol/layers-in-multicol.html bug 120750 (author: [e-mail])
fast/multicol/newmulticol/positioned-with-constrained-height.html bug 120751 (author: [e-mail])
fast/shapes/shape-inside/shape-inside-overflow-fixed-dimensions-block-content.html bug 120752 (author: [e-mail])
fast/shapes/shape-outside-floats/shape-outside-floats-image-002.html bug 120753 (author: [e-mail])
fast/shapes/shape-outside-floats/shape-outside-floats-image-001.html bug 120754 (author: [e-mail])
animations/cross-fade-background-image.html bug 120755 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 210550 [details]
Patch

Clearing flags on attachment: 210550

Committed r155100: <http://trac.webkit.org/changeset/155100>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

This patch caused animations/cross-fade-background-image.html to start failing on Mac:
http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=animations%2Fcross-fade-background-image.html