Summary: | Disable flattening the stage iframe of the graphics benchmark when running on iOS | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Said Abou-Hallawa <sabouhallawa> | ||||
Component: | Animations | Assignee: | 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
Said Abou-Hallawa
2015-11-17 14:38:56 PST
Created attachment 265709 [details]
Patch
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? Committed r192550: <http://trac.webkit.org/changeset/192550> 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. |