OMTA for layers free

RESOLVED FIXED in Firefox 56

Status

()

Core
Graphics: WebRender
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: ethlin, Assigned: ethlin)

Tracking

(Blocks: 1 bug)

unspecified
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Assignee)

Description

5 months ago
There is a prototype patch: https://reviewboard.mozilla.org/r/152562/diff/1#index_header. We should make it works correctly.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 4

4 months ago
Some changes are about fixing the unified build errors. The OMTA works with the patches, but the non-OMTA path doesn't work in layers-free mode. I guess the restyle manager retrieves the layer data somewhere, or FLB updates some information to frame. I will fix non-OMTA path in another bug.

Comment 5

4 months ago
mozreview-review
Comment on attachment 8887860 [details]
Bug 1378602 - Part1. Add WR support for nsDisplayOpacity.

https://reviewboard.mozilla.org/r/158790/#review164208
Attachment #8887860 - Flags: review?(bugmail) → review+

Comment 6

4 months ago
mozreview-review
Comment on attachment 8887861 [details]
Bug 1378602 - Part2. Move animation data from layer to AnimationInfo.

https://reviewboard.mozilla.org/r/158792/#review164182

::: gfx/layers/ImageLayers.cpp:12
(Diff revision 1)
>  #include "ImageContainer.h"             // for ImageContainer
>  #include "gfxRect.h"                    // for gfxRect
>  #include "nsDebug.h"                    // for NS_ASSERTION
>  #include "nsISupportsImpl.h"            // for ImageContainer::Release, etc
>  #include "gfx2DGlue.h"
> +#include "gfxPoint.h"                   // for SizeDouble

If you need SizeDouble you should include mozilla/gfx/Point.h, not gfxPoint.h

::: gfx/layers/Layers.h:1952
(Diff revision 1)
>    nsTArray<RefPtr<AsyncPanZoomController> > mApzcs;
> -  bool mUseTileSourceRect;
>  #ifdef DEBUG
>    uint32_t mDebugColorIndex;
>  #endif
> -  // If this layer is used for OMTA, then this counter is used to ensure we
> +  bool mUseTileSourceRect;

Why did you move this? It seems unrelated to the patch, please move it back

::: gfx/layers/wr/WebRenderUserData.h:105
(Diff revision 1)
>  protected:
>    nsAutoPtr<nsDisplayItemGeometry> mGeometry;
>    nsRect mBounds;
>  };
>  
> +class WebRenderAnimationData : public WebRenderUserData

The WebRenderAnimationData changes don't really belong in this patch, they would be better moved to part 3.

::: layout/painting/nsDisplayList.cpp:491
(Diff revision 1)
>  static void
>  AddAnimationForProperty(nsIFrame* aFrame, const AnimationProperty& aProperty,
> -                        dom::Animation* aAnimation, Layer* aLayer,
> +                        dom::Animation* aAnimation, AnimationInfo& aAnimationInfo,
>                          AnimationData& aData, bool aPending)
>  {
> -  MOZ_ASSERT(aLayer->AsContainerLayer(), "Should only animate ContainerLayer");
> +  //MOZ_ASSERT(aLayer->AsContainerLayer(), "Should only animate ContainerLayer");

Remove the assertion

::: layout/painting/nsDisplayList.cpp:862
(Diff revision 1)
> -    // We need to schedule another refresh driver run so that EffectCompositor
> -    // gets a chance to unthrottle the animation.
> -    aFrame->SchedulePaint();
> +  if (animationInfo.IsMutated()) {
> +    aLayer->Mutated();
> +    animationInfo.ResetMutated();
> -    return;
>    }

Instead of doing this manually here, I'd prefer to remove the IsMutated() and ResetMutated() functions on AnimationInfo, and instead add one like this:

void TransferMutatedFlagToLayer(Layer* aLayer) {
  if (mMutated) {
    aLayer->Mutated();
    mMutated = false;
  }
}

and call that from here instead.
Attachment #8887861 - Flags: review?(bugmail) → review+

Comment 7

4 months ago
mozreview-review
Comment on attachment 8887862 [details]
Bug 1378602 - Part3. Add OMTA support for nsDisplayTransform and nsDisplayOpacity.

https://reviewboard.mozilla.org/r/158794/#review164218

Nice work!
Attachment #8887862 - Flags: review?(bugmail) → review+
(Assignee)

Comment 8

4 months ago
mozreview-review-reply
Comment on attachment 8887861 [details]
Bug 1378602 - Part2. Move animation data from layer to AnimationInfo.

https://reviewboard.mozilla.org/r/158792/#review164182

> The WebRenderAnimationData changes don't really belong in this patch, they would be better moved to part 3.

Sorry, I splitted wrong. I will move the changes of WebRenderUserData to part 3.

> Instead of doing this manually here, I'd prefer to remove the IsMutated() and ResetMutated() functions on AnimationInfo, and instead add one like this:
> 
> void TransferMutatedFlagToLayer(Layer* aLayer) {
>   if (mMutated) {
>     aLayer->Mutated();
>     mMutated = false;
>   }
> }
> 
> and call that from here instead.

OKay, I will use this function to replace IsMutated() and ResetMutated().
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

4 months ago
Attachment #8887860 - Attachment is obsolete: true
(Assignee)

Updated

4 months ago
Attachment #8887862 - Attachment is obsolete: true
(Assignee)

Updated

4 months ago
Attachment #8887860 - Attachment is obsolete: false
(Assignee)

Updated

4 months ago
Attachment #8887862 - Attachment is obsolete: false
(Assignee)

Comment 14

4 months ago
try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7251d25bf4c9bce807e18bd57a97838826e820e&selectedJob=116229852

Comment 15

4 months ago
Pushed by ethlin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5f0fe749678
Part1. Add WR support for nsDisplayOpacity. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/13c0405ebb33
Part2. Move animation data from layer to AnimationInfo. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/d38d77c05d52
Part3. Add OMTA support for nsDisplayTransform and nsDisplayOpacity. r=kats

Comment 16

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e5f0fe749678
https://hg.mozilla.org/mozilla-central/rev/13c0405ebb33
https://hg.mozilla.org/mozilla-central/rev/d38d77c05d52
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.