Bug 155723 – Update benchmark tests

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Jon Lee]

Condense some of the tests, and remove redundant ones. Enhance the master suite.

[Jon Lee]

Created attachment 274604 [details]
1. Merge text tests

[Jon Lee]

Created attachment 274605 [details]
2. Clean up miscellaneous test suite.

[Jon Lee]

Created attachment 274606 [details]
3. Move compositing transforms test.

[Jon Lee]

Created attachment 274607 [details]
4. Move 3D suite.

[Jon Lee]

Created attachment 274608 [details]
5. Add canvas tests that include all stroke and fill paths.

[Jon Lee]

Created attachment 274609 [details]
6. Add a helper method.

[Jon Lee]

Created attachment 274610 [details]
7. Add quadratic and bezier segments.

[Jon Lee]

Created attachment 274611 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 274611 [details]
Patch

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

> PerformanceTests/Animometer/tests/master/resources/canvas-tests.js:87
> +        this.color = colors[Math.floor(Pseudo.random() * colors.length)];

Can't we use Stage.randomElementInArray() here?

> PerformanceTests/Animometer/tests/master/resources/canvas-tests.js:95
> +            nextPoint = this.randomPoint(stage, new Point(this.gridSizeX / 2, this.gridSizeY / 2));

this.gridSize.center() can be if this.gridSize is defined as a Point object.

> PerformanceTests/Animometer/tests/master/resources/canvas-tests.js:101
> +    gridSizeY: 40,

Should we use a point object instead of two variables?

> PerformanceTests/Animometer/tests/master/resources/canvas-tests.js:123
> +        var y = (coordinate.y + .5) * stage.size.y / (this.gridSizeY + 1);

What is the need for adding .5 to coordinate.x and coordinate.y? And why do we add 1 to this.gridSizeX and this.gridSizeY?

Can't we cache the value: gridCells = stage.size.divide(gridSize) and use it instead of this.gridSizeX and this.gridSizeY?

> PerformanceTests/Animometer/tests/master/resources/text.js:26
> +    ],

Do we need to add a class for Color? I think this array would be more readable if it is written as array of color pairs.

[Jon Lee]

Comment on attachment 274611 [details]
Patch

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

>> PerformanceTests/Animometer/tests/master/resources/canvas-tests.js:87
>> +        this.color = colors[Math.floor(Pseudo.random() * colors.length)];
> 
> Can't we use Stage.randomElementInArray() here?

Done.

>> PerformanceTests/Animometer/tests/master/resources/canvas-tests.js:123
>> +        var y = (coordinate.y + .5) * stage.size.y / (this.gridSizeY + 1);
> 
> What is the need for adding .5 to coordinate.x and coordinate.y? And why do we add 1 to this.gridSizeX and this.gridSizeY?
> 
> Can't we cache the value: gridCells = stage.size.divide(gridSize) and use it instead of this.gridSizeX and this.gridSizeY?

As I say in the Changelog, it's to scale down the whole thing a little bit so that you can see the full stroke along the edges of the grid.

I can move these numbers into stage.

>> PerformanceTests/Animometer/tests/master/resources/text.js:26
>> +    ],
> 
> Do we need to add a class for Color? I think this array would be more readable if it is written as array of color pairs.

It would probably be good to add.

[Jon Lee]

Committed r198509: <http://trac.webkit.org/changeset/198509>