1503616 - Android: zooming out on a page clips the content

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P1)

Description

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

If you're on a page like hg.mozilla.org the default rendering is in "desktop mode" (i.e. zoomed out). In this view only the top-left corner of the page is visible, the rest is black.

Comment 1

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

I finally tracked down the bug in my WIP that addressed this. It was a silly mistake. I'll clean up the patches and put them up for review today.

The basic problem here is that the SetPinchZoom API that we're currently using applies to everything, including stuff from the chrome document. When actually the semantics of APZ zoom is that they're meant to apply to the root content process document. Rather than doing a bunch of changes inside WR to change the SetPinchZoom API implementation, I'm going with the approach Markus suggested, which is putting an animated transform in the display list and then updating the scale transform on it to reflect the pinch zoom. This seems to work well, and is similar to how we move the scrollbars asynchronously with WR. There will be additional followup changes needed to fine-tune behaviour but this approach gets us much better behaviour and is worth landing.

Comment 2

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

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=9a3ca6df5374282f641a5c11e82e83d4701e927f
https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=872517b719bfe324c205d0d5702a28ec6607c709

Comment 3

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

Attachment: Bug 1503616 - Set a animatable transform property on the root content WR stacking context. r?botond

This sets a placeholder transform on the root stacking context for the
content display list, and allows us to modify that transform directly in
the compositor. This is similar to what
nsDisplayOwnLayer::CreateWebRenderCommands does for scroll thumb layers,
which are similarly manipulated by APZ for async scrolling.

Comment 4

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

Attachment: Bug 1503616 - Propagate the zoom animation id over to the APZ code. r?botond

Depends on D11180

Comment 5

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

Attachment: Bug 1503616 - Use the zoom animation instead of the PinchZoom API to implement async zooming. r?botond

This allows us to apply a zoom on a subset of the WR display list (the part
that is inside the stacking context with the animation property), rather
than applying globally to everything. Since the semantics of APZ zooming
is that it applies to the root content document, this results in more
correct behaviour, particularly with respect to clips from the
chrome-process document which now don't get zoomed but did before.

Depends on D11181

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/025172a6f7f5
Set a animatable transform property on the root content WR stacking context. r=mstange
https://hg.mozilla.org/integration/autoland/rev/7c91c5d04470
Propagate the zoom animation id over to the APZ code. r=botond
https://hg.mozilla.org/integration/autoland/rev/ba461450c817
Use the zoom animation instead of the PinchZoom API to implement async zooming. r=botond

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/025172a6f7f5
https://hg.mozilla.org/mozilla-central/rev/7c91c5d04470
https://hg.mozilla.org/mozilla-central/rev/ba461450c817