951320 - The requested displayport is sometimes wider than the page

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Doing some rendertrace logging on the peak in the settings app showed that the displayport being requested was sometimes 540 pixels wide even though the page was only 360 pixels wide.

Turns out GetExpandedScrollableRect doesn't bother to convert the composition bounds from a ScreenRect to a CSSRect. The composition bounds isn't always correct (bug 935219) but even so expanding the scrollable rect by a ScreenRect composition bounds is wrong.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

This fixes the issue, but I need to test it more to ensure there aren't any other bad side-effects.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/b2g-inbound/rev/e1b8a4d97075

Requesting uplift to 1.3. There are no user-visible bad effects from this bug, but I suspect it might result in wasted memory usage because the displayport is larger than it should be. I'm not actually sure if the texture that gets created is larger or not though.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Doh. This is actually a regression introduced very recently in bug 949404.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e1b8a4d97075

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8348953 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 949404
User impact if declined: panning around on some pages is very janky in metrofox (see bug 951751)
Testing completed (on m-c, etc.): locally
Risk to taking this patch (and alternatives if risky): low risk, very obvious fix for the regression. affects only b2g and metro
String or IDL/UUID changes made by this patch: none

Comment 7

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dbdd87455003

Comment 8

[Jason Smith [:jsmith]]

Already in 1.3 per comment 7, so clearing nom.