Closed Bug 1141488 Opened 5 years ago Closed 5 years ago

Very slow transition on transform:scale (with assertion) with APZC turned off

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: paul, Assigned: botond)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file, 1 obsolete file)

B2G Desktop, on Mac.

Many transition on transform:scale are very slow (almost no frames between the beginning and the end of the transition).

And I'm getting this assertion:

> ###!!! ASSERTION: overwriting animated transform!: '!aLayer->AsLayerComposite()->GetShadowTransformSetByAnimation()', file /Users/paul/mozilla/gecko-projects/gfx/layers/composite/AsyncCompositionManager.cpp, line 961

(Not sure how relevant this is)

It's very smooth with APZC.

Simple transition as:

  .tab-thumbnail {
    background: white;
    background-size: cover;
    border: 1px solid rgba(0,0,0,0.07);
    border-radius: 4px;
    height: 62.5px;
    margin: 16px 0 0;
    transform: scale(0.8);
    transition: transform 150ms ease;
    width: 100px;
    will-change: transform;
  }

  .tab.selected .tab-thumbnail {
    transform: scale(1);
  }

Transitions happen in the main process.
Botond, any ideas?
Flags: needinfo?(botond)
That code (TransformScrollableLayer) probably shouldn't even be running on OS X with APZ disabled. ... or any platform other than Fennec. This isn't a recent regression, is it?
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
> That code (TransformScrollableLayer) probably shouldn't even be running on
> OS X with APZ disabled. ... or any platform other than Fennec.

I use B2G on Desktop. Maybe there's a preference we set we should not.

> This isn't a
> recent regression, is it?

I don't think so. It was here 5 weeks ago.
Blocks: graphene
I have no idea why, but if the mouse moves at the same time the transitions are triggered, it's very smooth.
Whiteboard: [gfx-noted]
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
> That code (TransformScrollableLayer) probably shouldn't even be running on
> OS X with APZ disabled. ... or any platform other than Fennec. This isn't a
> recent regression, is it?

By my reading of the code, TransformScrollableLayer runs on all non-APZ OMTC platforms, and always has (in recent memory):

  - CompositorParent::CompositeToTarget() calls AsyncCompositionManager::TransformShadowTree()
    unconditionally.

  - TransformShadowTree() calls ApplyAsyncContentTransformToTree(), which does nothing and
    returns false on non-APZ platforms, in which case we collect all the scrollable layers
    (or, on Fennec, all the "root scrollable layers") and call TransformScrollableLayer()
    on each.

It's true that TransformScrollableLayer() does some things that are no-ops on non-Fennec platforms (SetFirstPaintViewport(), SetPageRect(), SyncViewportInfo()), but I don't think the entire function is necessarily a no-op on non-Fennec platforms. For example, the AlignFixedAndStickLayers() call at the bottom might actually do something if there is a shadow-transform set by an animation.
I think the transition being slow is consistent with the transform set for purposes of the transition being overwritten by the async transform (which is empty on non-APZ non-Fennec platforms).

We had this problem on APZ platforms, and solved it in bug 1091296 by applying the async transform on top of any existing shadow transform. I suggest we give the same thing a try for non-APZ platforms too.
Flags: needinfo?(botond)
Attached file MozReview Request: bz://1141488/botond (obsolete) —
/r/5185 - Bug 1141488 - On non-APZ platforms, combine any async transform with the OMTA transform rather than clobbering it the OMTA transform. r=kats

Pull down this commit:

hg pull review -r c0c812c1cfb0f71ebf1ed610678195a3e53c2725
Attachment #8576104 - Flags: review?(bugmail.mozilla)
Paul, could you try this patch and let me know if it solves the issue for you?
Flags: needinfo?(paul)
Comment on attachment 8576104 [details]
MozReview Request: bz://1141488/botond

https://reviewboard.mozilla.org/r/5183/#review4205

LGTM, thanks!
Attachment #8576104 - Flags: review?(bugmail.mozilla) → review+
(In reply to Botond Ballo [:botond] from comment #8)
> Paul, could you try this patch and let me know if it solves the issue for
> you?

Yes! It works! Thanks a lot for the very quick response.
Flags: needinfo?(paul)
Can we land this patch?
Flags: needinfo?(botond)
Flags: needinfo?(botond)
https://hg.mozilla.org/mozilla-central/rev/c05f34c01e6d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Attachment #8576104 - Attachment is obsolete: true
Attachment #8619713 - Flags: review+
You need to log in before you can comment on or make changes to this bug.