Handle foreignobject better with blob invalidation

RESOLVED FIXED in Firefox 64

Status

()

defect
P2
normal
RESOLVED FIXED
Last year
2 months ago

People

(Reporter: jrmuizel, Assigned: Gankro)

Tracking

(Blocks 2 bugs, Regressed 1 bug)

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

No description provided.
This is intended to fix APZ scrolling inside scrollable elements which are inside a <foreignObject> inside an <svg>.

We need to:
 - Introduce a display item type called nsDisplayForeignObject
 - Make it wrap the contents of the foreign object frame
 - Detect it by its display item type in WebRenderCommandBuilder.cpp and cause it to set mDoGrouping to false for its contents, see the TODO comment at https://searchfox.org/mozilla-central/rev/08df4e6e11284186d477d7e5b0ae48483ecc979c/gfx/layers/wr/WebRenderCommandBuilder.cpp#1307-1311
Assignee: nobody → jmuizelaar
Comment hidden (mozreview-request)

Comment 3

Last year
mozreview-review
Comment on attachment 8972995 [details]
Bug 1449634. Handle foreignobject better with blob invalidation.

https://reviewboard.mozilla.org/r/241538/#review247600

::: layout/painting/nsDisplayList.cpp:10143
(Diff revision 1)
> +nsDisplayForeignObject::BuildLayer(nsDisplayListBuilder* aBuilder,
> +                                   LayerManager* aManager,
> +                                   const ContainerLayerParameters& aContainerParameters)
> +{
> +  ContainerLayerParameters newContainerParameters = aContainerParameters;
> +  newContainerParameters.mDisableSubpixelAntialiasingInDescendants = true;

Why?

::: layout/painting/nsDisplayList.cpp:10163
(Diff revision 1)
> +                                             aResources,
> +                                             aSc,
> +                                             aManager,
> +                                             aDisplayListBuilder);

indent

::: layout/painting/nsDisplayList.cpp:10167
(Diff revision 1)
> +    return nsDisplayWrapList::CreateWebRenderCommands(aBuilder,
> +                                             aResources,
> +                                             aSc,
> +                                             aManager,
> +                                             aDisplayListBuilder);
> +  } else {

let's make the return false an early return
Attachment #8972995 - Flags: review?(mstange) → review+
Assignee

Updated

10 months ago
Blocks: 1455192
Assignee

Updated

10 months ago
Assignee: jmuizelaar → a.beingessner
Assignee

Comment 5

10 months ago
It looks like the only failures are just Bug 1452337 rearing its head, in which case it should be fine for us to proceed with this patch. Sent reftest expectation adjustments to jrmuizel.
Duplicate of this bug: 1488996
As mentioned in the duplicate, this affects our tscrollx talos test.

Comment 8

10 months ago
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e2a6bb9192d
Handle foreignobject better with blob invalidation. r=mstange

Comment 9

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4e2a6bb9192d
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
a perf improvement:
== Change summary for alert #15704 (as of Fri, 07 Sep 2018 00:45:49 GMT) ==

Improvements:

 24%  tscrollx linux64-qr opt e10s stylo     0.41 -> 0.31

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=15704
Regressions: 1547342
You need to log in before you can comment on or make changes to this bug.