1507347 - Handle FrameMetrics::mCumulativeResolution being 0 gracefully

Status: RESOLVED FIXED

(Core :: Panning and Zooming, enhancement, P3)

Description

[Botond Ballo [:botond]]

This came up in a conversation with Kats a little while ago: FrameMetrics::mCumulativeResolution can have a 0 component in situations where an element has a scale transform of 0 in that direction. However, there are places where we divide by it without guarding against a 0 value (e.g. here [1]).

We should audit the usages of mCumulativeResolution and make sure all divisions by it are checked for a 0 value.

(This issue came up before in bug 1277814, but in that bug we only added a check for one particular place where we perform a division.)

[1] https://searchfox.org/mozilla-central/rev/d850d799a0009f851b5535580e0a8b4bb2c591d7/gfx/layers/apz/src/AsyncPanZoomController.cpp#4331

Comment 1

[Alaa Emad]

Attachment: Bug 1507347 - Handle FrameMetrics::mCumulativeResolution being 0 gracefully

Comment 2

[Alaa Emad]

Attachment: Bug 1507347 - Handle FrameMetrics::mCumulativeResolution being 0 gracefully

Comment 3

[Botond Ballo [:botond]]

Just wanted to document a clarification here:

(In reply to Botond Ballo [:botond] from comment #0)

FrameMetrics::mCumulativeResolution can have a 0 component in situations
where an element has a scale transform of 0 in that direction

A scale transform can indeed be zero in one direction and nonzero in another. This comment seems to suggest that in such cases, only the corresponding component of mCumulativeResolution will be zero.

However, in bug 1277814, where we fixed one instance of this problem, the test case involved a transform with only one component (scaleY) being zero, but the fix only added a check for CSSToParentLayerScale2D(0, 0), and that fix caused the test case to pass, suggesting that a transform with a zero scale in one direction results in a mCumulativeResolution that is zero in both directions.

The explanation for this appears to lie in the implementation of BaseMatrix::ScaleFactors(), which returns a scale that's zero in both dimensions if the matrix's determinant is zero. Having a zero scale along either axis as part of the transform will result in the matrix having determinant zero and this code path being hit. Finally, the code that calculates mCumulativeResolution calls nsLayoutUtils::GetTransformToAncestorScale() (which uses this ScaleFactors() function) to pick up the CSS-driven resolution.

(The conclusion relevant to the patch is that it's fine to test for CSSToParentLayerScale2D(0, 0) and not necessary to test the dimensions individually.)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a0b522fbb1d
Handle FrameMetrics::mCumulativeResolution being 0 gracefully r=botond

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/3a0b522fbb1d