Bug 130555 – [iOS][WK2] Reduce the tiling coverage to the current rect and 1 tile size ahead

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

[iOS][WK2] Reduce the tiling coverage to the current rect and 1 tile size ahead

[Benjamin Poulain]

Created attachment 227371 [details]
Patch

[Benjamin Poulain]

I dropped the acceleration. Taking into account acceleration gives better approximation of futureRect but with the tiling-ahead, that made almost no difference.

Tiling ahead only goes for one tile size. Anything bigger was slowing down rendering so much that the result would be useless by the time it comes to the UIProcess. This version is also much simpler.

The way I push the coverage rect is currently done by pushing is as the clip rect in flushCompositingState(). I am not sure what would be the right way to do that. Pulling the transforms from TileController seems just as bad.

[Benjamin Poulain]

Created attachment 227379 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 227379 [details]
Patch

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

> Source/WebCore/platform/ScrollView.h:188
> +    void setScrollVelocityInformation(double horizontalVelocity, double verticalVelocity, double timestamp);

FloatSize?
"information" seems redundant.

> Source/WebCore/platform/ScrollView.h:189
> +    FloatRect computeCoverageRect(double horizontalMargin, double verticalMargin);

Are these margins on both sides?
Function should be const.

> Source/WebCore/platform/ScrollView.h:418
> +    double m_horizontalVelocity;
> +    double m_verticalVelocity;

FloatSize?

> Source/WebCore/platform/ScrollView.h:419
> +    double m_velocityUpdateTimestamp;

I'd call this m_lastVelocityUpdateTime

> Source/WebCore/platform/ios/ScrollViewIOS.mm:149
> +        futureRect.setWidth(futureRect.width() + horizontalMargin);
> +        if (m_horizontalVelocity < 0)
> +            futureRect.setX(std::max(futureRect.x() - horizontalMargin, 0.));
> +    }
> +
> +    if (m_verticalVelocity) {
> +        futureRect.setHeight(futureRect.height() + verticalMargin);
> +        if (m_verticalVelocity < 0)
> +            futureRect.setY(std::max(futureRect.y() - verticalMargin, 0.));
> +    }

I think we need to clamp when velocity is really high, to avoid making too many tiles.

> Source/WebKit2/Shared/VisibleContentRectUpdateInfo.h:80
> +    double m_horizontalVelocity;
> +    double m_verticalVelocity;

FloatSize?

> Source/WebKit2/UIProcess/ios/WKContentView.mm:66
> +class HistoricalKinematicData {

VelocityData maybe? Kinematic sounds like we're doing multi-body simulation.

> Source/WebKit2/UIProcess/ios/WKContentView.mm:75
> +    CGPoint velocityForNewData(CGPoint newPosition, double timestamp)

I think a CGSize would make more sense as a return value.

[Benjamin Poulain]

Created attachment 227515 [details]
Patch

[Build Bot]

Comment on attachment 227515 [details]
Patch

Attachment 227515 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4648652651888640

New failing tests:
media/W3C/audio/canPlayType/canPlayType_application_octet_stream.html

[Build Bot]

Created attachment 227521 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Benjamin Poulain]

Comment on attachment 227515 [details]
Patch

Clearing flags on attachment: 227515

Committed r166110: <http://trac.webkit.org/changeset/166110>

[Benjamin Poulain]

All reviewed patches have been landed.  Closing bug.