Closed Bug 1676789 Opened 1 year ago Closed 3 months ago

Sample APZ animations before sampling OMT animations

Categories

(Core :: Graphics, task)

task

Tracking

()

RESOLVED FIXED
93 Branch
Tracking Status
firefox93 --- fixed

People

(Reporter: hiro, Assigned: boris)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

We do currently sample OMT animations first then sample APZ animations. But for scroll linked animations, I am pretty sure we need to sample APZ first.

This is the sampling place for WebRender, and this is the place for non WebRender.

As you can see, there is a comment in the non WebRender code;

NB: we must sample animations before sampling pan/zoom transforms

The comment has been there in the first place we implemented OMTA in bug 706179 and unfortunately there is no comment the reason why we had to do.

I am assuming the reason is we do use the OMTA transform value for APZ for architectural restrictions, I can't think of any reasons why.

Oh, maybe https://hg.mozilla.org/mozilla-central/rev/9af1395c410c might be able to answer the question.

Assignee: nobody → boris.chiou
Status: NEW → ASSIGNED

Simply reordering these two sampling functions may cause a lot of issues (try). I'm not familiar with APZ animations now but I guess we probably need to get the transform matrix of OMT animations first, and then applying the async zooming or scrolling animations to get the correct result. I can imagine that something like double-tap zoom needs to sample zoom animations after we sample the OMT animations. So in order to support scroll-linked animations, we may have to split part of APZ sampler (especially the scrolling information) and doing that before sampling OMT animations.

Not sure what we have to do now for this bug, so I may have to work on this together with other bugs to see the whole picture first.

Have you tracked down what's going on there? For example, position-fixed-transformed-1.html is one of the failures in the try link, and I am pretty sure it's not including any OMTAs. So that doesn't make sense the order change causes some effects on the test.

We also need to modify this UpdateDynamicProperties call and this AppendTransformProperties.

With those modifications on WebRender it should work fine, I hope!

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)

We also need to modify this UpdateDynamicProperties call and this AppendTransformProperties.

With those modifications on WebRender it should work fine, I hope!

Yes. Just notice: https://searchfox.org/mozilla-central/rev/f71cb98fc35da418d2cb9ce31a0416d532dc9d69/gfx/layers/wr/OMTASampler.cpp#117-121

We clean up the property in OMTASamper...

Attachment #9235719 - Attachment description: Bug 1676789 - Sample apz animation before sampling omt animations for webrender. → Bug 1676789 - Sample apz animations before sampling omt animations for webrender.

Don't land this patch because layers are going away.

In this patch, we do:

  1. Force to test with non-webrender in CI
  2. The simplest way to reorder the OMT animations and APZ animations is:
    a) Store the APZ content transform for each layer
    b) Apply the APZ content transform after we got its OMTA transform.

This is a little bit stupid but it doesn't hit the try errors.

Note:
When force to test with legacy layers, there are two wpts failed always:
css/css-transforms/perspective-split-by-zero-w.html
css/filter-effects/svg-shorthand-hue-rotate-001.html

(In reply to Hiroyuki Ikezoe (:hiro) from comment #0)

I am assuming the reason is we do use the OMTA transform value for APZ for architectural restrictions, I can't think of any reasons why.

Oh, maybe https://hg.mozilla.org/mozilla-central/rev/9af1395c410c might be able to answer the question.

I put the hack patch for layers: D123383. It should be fine to reorder omta and apz. Just make sure the shadow base transform is correct in each layer.

Legacy layer is going away, so only D122308 needs to be reviewed and landed.

Attachment #9235719 - Attachment description: Bug 1676789 - Sample apz animations before sampling omt animations for webrender. → Bug 1676789 - Sample APZ animations before sampling OMTA animations for webrender.
Attachment #9237570 - Attachment is obsolete: true

If we don't have OMTA sampler or APZ sampler, we may be failed to reset
the previous pending properties, so adding a reset message and use it
before appending any result from the sampling. OMTA sampler may be
always there, but APZ is not, and we would like to do APZ sampling first
in this patch series, so this message becomes necessary.

Now we always reset the properties for each transaction, so no need to
append/update if these vectors are empty.

Pushed by bchiou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ec14bf08defc
Add reset_dynamic_properties() to transaction message. r=hiro
https://hg.mozilla.org/integration/autoland/rev/a5b2e568f5cb
Sample APZ animations before sampling OMTA animations for webrender. r=hiro
https://hg.mozilla.org/integration/autoland/rev/93c46fbfe1cc
No need to append omta properties if they are empty. r=hiro
Flags: needinfo?(boris.chiou)
Flags: needinfo?(boris.chiou)
Pushed by bchiou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fab426558c1e
Add reset_dynamic_properties() to transaction message. r=hiro
https://hg.mozilla.org/integration/autoland/rev/202c42992775
Sample APZ animations before sampling OMTA animations for webrender. r=hiro
https://hg.mozilla.org/integration/autoland/rev/f4ad9b76e5f8
No need to append omta properties if they are empty. r=hiro
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 93 Branch
You need to log in before you can comment on or make changes to this bug.