Bug 225556 – Add web process drawing to the focus test in MotionMark

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Myles C. Maxfield]

Add web process drawing to the focus test in MotionMark

[Myles C. Maxfield]

Created attachment 428079 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 428079 [details]
Patch

When we do version bumps, are they normally part of the patch?

[Myles C. Maxfield]

(In reply to Simon Fraser (smfr) from comment #2)
> Comment on attachment 428079 [details]
> Patch
> 
> When we do version bumps, are they normally part of the patch?

I think we batch up multiple patches into a single version bump.

This patch only updates what's in PerformanceTests. When we want to publish, we copy that into Websites/browserbench.org.

[Darin Adler]

Comment on attachment 428079 [details]
Patch

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

> PerformanceTests/ChangeLog:11
> +        Previously, the MotionMark focus test just animated the blur, opacity, and position of each element.
> +        All of these get mapped to properties on the Core Animation layers, which means that each rAF() tick,
> +        the only thing we were doing was pushing layer state changes to the window server.

This comment talks a lot about window server and Core Animation. But this is a cross-platform, cross-browser benchmark, and window server is specific to Mac, and Core Animation specific to Apple’s macOS/iOS/iPadOS/tvOS/watchOS.

I’m sure that these these improvements help us have a more repeatable test on macOS, but are also helpful on other platforms, including iOS, and to accurately test other web browsers as well. But that’s not stated here and probably should be. We would not want to make the changes if they were harmful in those other configurations.

I’m not sure we need quite so long a change log comment. Maybe there is somewhere else we should leave this information if we are going to need it in the future. In the change log it might or might not be found.

[Simon Fraser (smfr)]

Comment on attachment 428079 [details]
Patch

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

>> PerformanceTests/ChangeLog:11
>> +        the only thing we were doing was pushing layer state changes to the window server.
> 
> This comment talks a lot about window server and Core Animation. But this is a cross-platform, cross-browser benchmark, and window server is specific to Mac, and Core Animation specific to Apple’s macOS/iOS/iPadOS/tvOS/watchOS.
> 
> I’m sure that these these improvements help us have a more repeatable test on macOS, but are also helpful on other platforms, including iOS, and to accurately test other web browsers as well. But that’s not stated here and probably should be. We would not want to make the changes if they were harmful in those other configurations.
> 
> I’m not sure we need quite so long a change log comment. Maybe there is somewhere else we should leave this information if we are going to need it in the future. In the change log it might or might not be found.

Agreed that we can trim down this changelog.

[Myles C. Maxfield]

Created attachment 428347 [details]
Patch for committing

[Radar WebKit Bug Importer]

<rdar://problem/78047813>

[Myles C. Maxfield]

Focus test has been removed.