Closed Bug 1504270 Opened 6 years ago Closed 2 years ago

CSS animation with css-doodle causes repaint flicker

Categories

(Core :: Web Painting, defect, P1)

Product:
Core
Component:
Web Painting
Version:
65 Branch
Type:
defect

Tracking

()

Status:
RESOLVED WORKSFORME
Tracking Flags:
Tracking Status
firefox65 --- affected

People

(Reporter: svoisen, Unassigned)

References

Details

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:65.0) Gecko/20100101 Firefox/65.0

Visit the following Codepen: https://codepen.io/yuanchuan/pen/QZBLpg

On Mac, the entire page flickers, not just the iframe containing the animation. On Windows with WebRender enabled, there is no reported flicker. On Chrome 70 on Mac, there is no flicker. Everything also runs smoothly on Safari.

If you slow down the animation a bit (remove the animation-delay line from the CSS), the flickering will stop until the lines being animated intersect with the iframe boundaries. Removing the @size: 100% line from the CSS also eliminates the flickering.

Note this example uses the CSS doodle web component: https://css-doodle.com/
This appears to be suffering from bug 779598 and bug 1100357.
Depends on: 779598, 1100357

The flickering looks really bad.

Priority: -- → P1

This looks like Matrix4x4Typed::TransformAndClipBounds isn't clipping properly (or isn't supposed to do what the calling code expects).

LayerManagerComposite.cpp::TransformRect is returning a rect outside of IntRect::MaxIntRect(), which we then union into nsIntRegion and cause overflow. The overflowed region is non-sensical, and then we end up deciding that our all whole tree for the content area isn't visible.

I get these values for the broken run:

(lldb) p intRect
(mozilla::gfx::IntRect) $49 = {
mozilla::gfx::BaseRect<int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::IntMarginTyped<mozilla::gfx::UnknownUnits> > = (x = -1086851840, y = -528531104, width = 1086852992, height = 563617088)
}
(lldb) p aRect
(mozilla::LayerIntRect) $50 = {
mozilla::gfx::BaseRect<int, mozilla::gfx::IntRectTyped<mozilla::LayerPixel>, mozilla::gfx::IntPointTyped<mozilla::LayerPixel>, mozilla::gfx::IntSizeTyped<mozilla::LayerPixel>, mozilla::gfx::IntMarginTyped<mozilla::LayerPixel> > = (x = 141, y = 0, width = 6, height = 39)
}
(lldb) p aTransform
(mozilla::gfx::Matrix4x4) $51 = {
= {
= (_11 = 6.2721405, _12 = 4.25299931, _13 = -0.900095283, _14 = 0.00367685966, _21 = -1.99394608, _22 = -2.66148424, _23 = 0.38301155, _24 = -0.00156458956, _31 = -2.1922586, _32 = -1.68704569, _33 = 0.207679212, _34 = -0.000848362746, _41 = -847.180175, _42 = -520.474365, _43 = 360.419525, _44 = -0.472301841)
components = {
[0] = 6.2721405
[1] = 4.25299931
[2] = -0.900095283
[3] = 0.00367685966
[4] = -1.99394608
[5] = -2.66148424
[6] = 0.38301155
[7] = -0.00156458956
[8] = -2.1922586
[9] = -1.68704569
[10] = 0.207679212
[11] = -0.000848362746
[12] = -847.180175
[13] = -520.474365
[14] = 360.419525
[15] = -0.472301841
}
}
}
(lldb) p Rect::MaxIntRect()
Warning: hit breakpoint while running function, skipping commands and conditions to prevent recursion.(mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float>) $53 = {
mozilla::gfx::BaseRect<float, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float>, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float>, mozilla::gfx::SizeTyped<mozilla::gfx::UnknownUnits, float>, mozilla::gfx::MarginTyped<mozilla::gfx::UnknownUnits, float> > = (x = -1.07374182E+9, y = -1.07374182E+9, width = 2.14748365E+9, height = 2.14748365E+9)
}

Kip, we're passing Rect::MaxIntRect() as aClip to TransformAndClipBounds, and getting a result value that is outside of that area. Is that expected?

I had a look at the code, and am struggling to follow it :)

Flags: needinfo?(kgilbert)

(In reply to Matt Woodrow (:mattwoodrow) from comment #3)

This looks like Matrix4x4Typed::TransformAndClipBounds isn't clipping properly (or isn't supposed to do what the calling code expects).

LayerManagerComposite.cpp::TransformRect is returning a rect outside of IntRect::MaxIntRect(), which we then union into nsIntRegion and cause overflow. The overflowed region is non-sensical, and then we end up deciding that our all whole tree for the content area isn't visible.

I get these values for the broken run:

(lldb) p intRect
(mozilla::gfx::IntRect) $49 = {
mozilla::gfx::BaseRect<int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::IntMarginTyped<mozilla::gfx::UnknownUnits> > = (x = -1086851840, y = -528531104, width = 1086852992, height = 563617088)
}
(lldb) p aRect
(mozilla::LayerIntRect) $50 = {
mozilla::gfx::BaseRect<int, mozilla::gfx::IntRectTyped<mozilla::LayerPixel>, mozilla::gfx::IntPointTyped<mozilla::LayerPixel>, mozilla::gfx::IntSizeTyped<mozilla::LayerPixel>, mozilla::gfx::IntMarginTyped<mozilla::LayerPixel> > = (x = 141, y = 0, width = 6, height = 39)
}
(lldb) p aTransform
(mozilla::gfx::Matrix4x4) $51 = {
= {
= (_11 = 6.2721405, _12 = 4.25299931, _13 = -0.900095283, _14 = 0.00367685966, _21 = -1.99394608, _22 = -2.66148424, _23 = 0.38301155, _24 = -0.00156458956, _31 = -2.1922586, _32 = -1.68704569, _33 = 0.207679212, _34 = -0.000848362746, _41 = -847.180175, _42 = -520.474365, _43 = 360.419525, _44 = -0.472301841)
components = {
[0] = 6.2721405
[1] = 4.25299931
[2] = -0.900095283
[3] = 0.00367685966
[4] = -1.99394608
[5] = -2.66148424
[6] = 0.38301155
[7] = -0.00156458956
[8] = -2.1922586
[9] = -1.68704569
[10] = 0.207679212
[11] = -0.000848362746
[12] = -847.180175
[13] = -520.474365
[14] = 360.419525
[15] = -0.472301841
}
}
}
(lldb) p Rect::MaxIntRect()
Warning: hit breakpoint while running function, skipping commands and conditions to prevent recursion.(mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float>) $53 = {
mozilla::gfx::BaseRect<float, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float>, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float>, mozilla::gfx::SizeTyped<mozilla::gfx::UnknownUnits, float>, mozilla::gfx::MarginTyped<mozilla::gfx::UnknownUnits, float> > = (x = -1.07374182E+9, y = -1.07374182E+9, width = 2.14748365E+9, height = 2.14748365E+9)
}

Kip, we're passing Rect::MaxIntRect() as aClip to TransformAndClipBounds, and getting a result value that is outside of that area. Is that expected?

I had a look at the code, and am struggling to follow it :)

Hi Matt!

I haven't looked too deeply yet, but perhaps the issues you are running into could be caused by Rect::MaxIntRect returning component values up to std::numeric_limits<int32_t>::max(), which would be a 32-bit signed integer. The Matrix4x4 is operating on single precision float's which have a mantissa of 23 bits. Effectively, you may be seeing rounding for numbers outside of the 24-bit signed integer range to a value higher than to 2^31. Upgrading to a 64-bit float would enable a full 52 bits of precision before losing non-fractional significant digits.

As an experiment, could you try substituting a smaller rect for Rect::MaxIntRect(), with ranges of +-1,000,000 and see if the clipping appears more constrained?

If so, bumping to a double-precision for Matrix4x4 could perhaps solve this.

It just so happens that I have prepared a Matrix4x4Double (via templates) as part of my WIP WebXR implementation here:

https://bugzilla.mozilla.org/show_bug.cgi?id=1419190

Please NI me again after trying the smaller clip rect and I'll help look a bit deeper.

Flags: needinfo?(kgilbert)

The Matrix4x4Double and QuaternionDouble classes are in Bug 1532375. I'm looking for a reviewer for this patch. If its useful to you, would you like to take a look?

Flags: needinfo?(matt.woodrow)

Indeed, the smaller clip does seem to fix the issue.

I think we'd prefer to avoid switching to using double matrices, as clipping to the maximum valid int32 rect is something we do in quite a few places, so we'd probably have to switch the entirety of layout/gfx over to double.

I think there's something more to this though, as I saw a value coming out of TransformAndClipBounds of x=-2.44998042E+9, which is massively outside the MaxIntRect range.

Flags: needinfo?(matt.woodrow)

I did some more experimentation, and I think there are multiple issues at play.

If I add a manual clip (using IntersectRect) to Rect::MaxIntRect() to the result of TransformAndClipBounds, then we still get broken behaviour.

This looks to be pretty clearly a float precision issue, as once we convert to int32, our rect has x=-1073741824, whereas IntRect::MaxIntRect().x == -1073741823.

That single bit of difference is probably enough to allow the region's bounds to now span across a bigger range than int32 supports, and we get overflow.

It doesn't explain why TransformAndClipBounds returns a value of x=-2.44998042E+9, that doesn't sound like a precision issue.

Any ideas Kip?

It seems like we should make Rect::MaxIntRect() return a rectangle that is guaranteed to be inside the int32_t max rect, taking floating point precision into account and pass that to TransformAndClipBounds, but we'd need the buggy behaviour there to be fixed.

Alternatively we could just clip a second time, after we've converted to an int rect (which I've tested as working).

Flags: needinfo?(kgilbert)

(In reply to Matt Woodrow (:mattwoodrow) from comment #7)

I did some more experimentation, and I think there are multiple issues at play.

If I add a manual clip (using IntersectRect) to Rect::MaxIntRect() to the result of TransformAndClipBounds, then we still get broken behaviour.

This looks to be pretty clearly a float precision issue, as once we convert to int32, our rect has x=-1073741824, whereas IntRect::MaxIntRect().x == -1073741823.

That single bit of difference is probably enough to allow the region's bounds to now span across a bigger range than int32 supports, and we get overflow.

It doesn't explain why TransformAndClipBounds returns a value of x=-2.44998042E+9, that doesn't sound like a precision issue.

Any ideas Kip?

It seems like we should make Rect::MaxIntRect() return a rectangle that is guaranteed to be inside the int32_t max rect, taking floating point precision into account and pass that to TransformAndClipBounds, but we'd need the buggy behaviour there to be fixed.

Alternatively we could just clip a second time, after we've converted to an int rect (which I've tested as working).
Thanks for following up. Indeed, there may be something funny going on. I'll try dissecting further. Perhaps such a value may be the result of dividing by a nearly-zero value. Such is the case often when using complex numbers and rounding gets involved.

IMHO, this warrants some investigation to validate the TransformAndClipBounds is operating correctly; however, even if fixed, it would be a good idea to perform additional clipping after translating to int32 to address the precision issues above.

I'll keep this NI open until I can follow up with more info on what is going on in the function.

Hey Kip, have you had any time to look into this further?

Clearing old NI.. I won't have time to look at this soon and would invite others to take a look.

Flags: needinfo?(kgilbert)

I reproduced in a 2019-01-01 build but all seems well in a current build. We've changed a lot since then (webrender, frame layer builder removal).

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.