Bug 151361

Summary: Disable flattening the stage iframe of the graphics benchmark when running on iOS
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: AnimationsAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, dino, rniwa, simon.fraser
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch simon.fraser: review+

Description Said Abou-Hallawa 2015-11-17 14:38:56 PST
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.
Comment 1 Said Abou-Hallawa 2015-11-17 14:44:50 PST
Created attachment 265709 [details]
Patch
Comment 2 Simon Fraser (smfr) 2015-11-17 14:47:42 PST
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?
Comment 3 Said Abou-Hallawa 2015-11-17 16:18:23 PST
Committed r192550: <http://trac.webkit.org/changeset/192550>
Comment 4 Said Abou-Hallawa 2015-11-17 16:27:26 PST
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.