Bug 151361 – Disable flattening the stage iframe of the graphics benchmark when running on iOS

• Component: Animations
• Status: RESOLVED FIXED

[Said Abou-Hallawa]

This was happening because the iframe's width and height not fixed. When animating the particles, the size of the stage gets changed. On iOS the iframe, by default, gets flattened if its contents go beyond the its boundaries.

Also ensure the bouncing particles tests do not go outside the boundaries of the graphic benchmark stage.

[Said Abou-Hallawa]

Created attachment 265709 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 265709 [details]
Patch

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

> PerformanceTests/Animometer/runner/resources/animometer.css:359
> +    section#running > #running-test > iframe {

It's rare that you need both the element name and #id, since the #id should be enough. And is #running-test > iframe not sufficient?

> PerformanceTests/Animometer/tests/bouncing-particles/resources/bouncing-particles.js:64
>          // If particle is going to move off right side
>          if (this._position.x + this._size.x > this._stageSize.x) {
> -            // If direction is East-South
> +            // If direction is East-South, go West-South.
>              if (this._angle >= 0 && this._angle < Math.PI / 2)
>                  this._angle = Math.PI - this._angle;
> -            // If angle is East-North
> +            // If angle is East-North, go West-North.
>              else if (this._angle > Math.PI / 2 * 3)
>                  this._angle = this._angle - (this._angle - Math.PI / 2 * 3) * 2;
> +            // Make sure the particle does not go outside the stage boundaries.
> +            this._position.x = this._stageSize.x - this._size.x;
>          }
>          
>          // If particle is going to move off left side
>          if (this._position.x < 0) {
> -            // If angle is West-South
> +            // If angle is West-South, go East-South.
>              if (this._angle > Math.PI / 2 && this._angle < Math.PI)
>                  this._angle = Math.PI - this._angle;
> -            // If angle is West-North
> +            // If angle is West-North, go East-North.
>              else if (this._angle > Math.PI && this._angle < Math.PI / 2 * 3)
>                  this._angle = this._angle + (Math.PI / 2 * 3 - this._angle) * 2;
> +            // Make sure the particle does not go outside the stage boundaries.
> +            this._position.x = 0;
>          }
>  
>          // If particle is going to move off bottom side
>          if (this._position.y + this._size.y > this._stageSize.y) {
> -            // If direction is South
> +            // If direction is South, go North.
>              if (this._angle > 0 && this._angle < Math.PI)
>                  this._angle = Math.PI * 2 - this._angle;
> +            // Make sure the particle does not go outside the stage boundaries.
> +            this._position.y = this._stageSize.y - this._size.y;
>          }
>  
>          // If particle is going to move off top side
>          if (this._position.y < 0) {
> -            // If direction is North
> +            // If direction is North, go South.
>              if (this._angle > Math.PI && this._angle < Math.PI * 2)
>                  this._angle = this._angle - (this._angle - Math.PI) * 2;
> +            // Make sure the particle does not go outside the stage boundaries.
> +            this._position.y = 0;
>          }

Do you still need this bit?

[Said Abou-Hallawa]

Committed r192550: <http://trac.webkit.org/changeset/192550>

[Said Abou-Hallawa]

Comment on attachment 265709 [details]
Patch

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

>> PerformanceTests/Animometer/runner/resources/animometer.css:359
>> +    section#running > #running-test > iframe {
> 
> It's rare that you need both the element name and #id, since the #id should be enough. And is #running-test > iframe not sufficient?

Yes either of the element name to its id is sufficient. And even #running-test > iframe is sufficient to select the iframe. But I found the css more readable when being very specific in writing the css selectors for in this stylesheet. For example, I can read the above line without returning to the HTML file like this:

This css rule is for the <iframe> element which is inside a frame container whose id is #running-test which is inside a <section> element whose id is #running.

I may not following the css best practices by doing that. So I will look for an example and file a bug to clean the animometer.css from this verbosity since the above rule is not the only which has this issue.

>> PerformanceTests/Animometer/tests/bouncing-particles/resources/bouncing-particles.js:64
>>          }
> 
> Do you still need this bit?

No is not needed to disable the flattening but it makes the animation looks cleaner. So I kept it.