Bug 188532 – [MotionMark] Update Multiply test

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

[Jon Lee]

Update Multiply test

[Radar WebKit Bug Importer]

<rdar://problem/43252125>

[Radar WebKit Bug Importer]

<rdar://problem/43252151>

[Jon Lee]

Created attachment 347044 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 347044 [details]
Patch

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

> PerformanceTests/MotionMark/tests/dom/resources/multiply.js:33
> +            options.visibleCSS = [["opacity", 0, 1]];

I would suggest writing this as an structures:
    options.visibleCSS = [{ property: "visibility", initialValue: "hidden", animateValue: "visible" }];

> PerformanceTests/MotionMark/tests/master/resources/multiply.js:39
> +    visibleCSS: [
> +        ["visibility", "hidden", "visible"],
> +        ["opacity", 0, 1],
> +        ["display", "none", "block"]
> +    ],

Ditto:

visibleCSS: [
    { property: "visibility", initialValue: "hidden", animateValue: "visible" },
    { property: "opacity", initialValue: 0, animateValue: 1 },
    { property: "display", initialValue: "none", animateValue: "block" }
],

> PerformanceTests/MotionMark/tests/master/resources/multiply.js:45
> -        var tileSize = Math.round(this.size.height / 25);
> +        var tileSize = Math.round(this.size.height / this.totalRows);

Shouldn't we use the screen.height instead of the stage height to control how many tiles can be shown in the page?

> PerformanceTests/MotionMark/tests/master/resources/multiply.js:96
> +        tile.style[visibleCSS[0]] = visibleCSS[1];

tile.style[visibleCSS.property] = visibleCSS.initialValue;

> PerformanceTests/MotionMark/tests/master/resources/multiply.js:130
> +            tile.element.style[tile.visibleCSS[0]] = tile.visibleCSS[2];

tile.style[tile.visibleCSS.property] = tile.visibleCSS.animateValue;

> PerformanceTests/MotionMark/tests/master/resources/multiply.js:141
> +            tile.element.style[tile.visibleCSS[0]] = tile.visibleCSS[1];

tile.style[tile.visibleCSS.property] = tile.visibleCSS.initialValue;

[Jon Lee]

Comment on attachment 347044 [details]
Patch

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

>> PerformanceTests/MotionMark/tests/master/resources/multiply.js:39
>> +    ],
> 
> Ditto:
> 
> visibleCSS: [
>     { property: "visibility", initialValue: "hidden", animateValue: "visible" },
>     { property: "opacity", initialValue: 0, animateValue: 1 },
>     { property: "display", initialValue: "none", animateValue: "block" }
> ],

I'll do this update in a later patch.

>> PerformanceTests/MotionMark/tests/master/resources/multiply.js:45
>> +        var tileSize = Math.round(this.size.height / this.totalRows);
> 
> Shouldn't we use the screen.height instead of the stage height to control how many tiles can be shown in the page?

I don't think so. It does mean literally equal work in terms of point area among all device classes, but if we did this then desktop spaces would have high 10,000s max # of particles, which might be detrimental to getting high-enough scores given the # of renderers that need to be iterated.

[WebKit Commit Bot]

Comment on attachment 347044 [details]
Patch

Clearing flags on attachment: 347044

Committed r234832: <https://trac.webkit.org/changeset/234832>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.