1008915 - Unnecessary invalidation during scrolling due to floating point precision loss

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Markus Stange [:mstange]]

Attachment: testcase

STR:
 1. Load the testcase and enable paint flashing.
 2. Scroll up and down a bit.

We unnecessarily invalidate the rectangle.

Comment 1

[Markus Stange [:mstange]]

Attachment: v1

The problem here is that the X translation component of the final matrix computed in nsDisplayTransform::GetTransform varies depending on the vertical scroll position.

Without this patch, the matrix is computed as follows:

 1. nsDisplayTransform::GetResultingTransformMatrixInternal arrives at this matrix before the final basis change:

>   0.000000,    1.210612,  0.000000, 0.000000
>   -2.537108,   0.000000,  0.000000, 0.000000
>   0.000000,    0.000000,  1.000000, 0.000000
>   2282.956055, 82.881538, 0.000000, 1.000000

 2. nsDisplayTransform::GetResultingTransformMatrixInternal calls nsLayoutUtils::ChangeMatrixBasis.
   - aProperties.mToTransformOrigin is -99.983330, -599.983337
   - roundedOrigin is 107.983330, 506.983337
                   or 107.983330, 507.983337
                                   ^^ depending on the scroll position

 3. nsDisplayTransform::GetTransform calls mTransform.Translate(rounded), with rounded being the same as roundedOrigin in the previous step.

With roundedOrigin.y == 506.983337, the final matrix is:

>   0.000000,   1.210612,   0.000000, 0.000000
>   -2.537108,  0.000000,   0.000000, 0.000000
>   0.000000,   0.000000,   1.000000, 0.000000
>   768.733521, 110.922562, 0.000000, 1.000000
           ^^^    ^

With roundedOrigin.y == 507.983337, the final matrix is:

>   0.000000,   1.210612,   0.000000, 0.000000
>   -2.537108,  0.000000,   0.000000, 0.000000
>   0.000000,   0.000000,   1.000000, 0.000000
>   768.733826, 111.922562, 0.000000, 1.000000
           ^^^    ^

Note the difference in the translation components: The 1px difference in the Y component is expected, because roundedOrigin has changed by 1px due to the scroll, but the difference in the X component is unexpected and causes the invalidation.

With this patch applied, the final matrices look like this:

roundedOrigin.y == 506.983337:
>   0.000000,   1.210612,   0.000000, 0.000000
>   -2.537108,  0.000000,   0.000000, 0.000000
>   0.000000,   0.000000,   1.000000, 0.000000
>   768.733704, 110.922562, 0.000000, 1.000000

roundedOrigin.y == 507.983337:
>   0.000000,   1.210612,   0.000000, 0.000000
>   -2.537108,  0.000000,   0.000000, 0.000000
>   0.000000,   0.000000,   1.000000, 0.000000
>   768.733704, 111.922562, 0.000000, 1.000000

Comment 2

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8421006 [details] [diff] [review]
v1

Review of attachment 8421006 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsDisplayList.h
@@ +3325,5 @@
>     *        for the frame's bounding rectangle. Otherwise, it will use the
>     *        value of aBoundsOverride.  This is mostly for internal use and in
>     *        most cases you will not need to specify a value.
> +   * @param aOffsetByOrigin If true, the resulting matrix will be translated
> +   *        by aOrigin.

The origin offset is applied *before* the CSS transform, the comment should make this clear.

Comment 3

[Markus Stange [:mstange]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d7968acde595

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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