1749380 - Refactor / fix the various correctness and performance issues WR has with render target scaling, bounding rect inflation and raster root creation

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Comment 1

[Glenn Watson [:gw]]

WR currently suffers from a number of correctness and performance issues related to how we handle picture bounding rects, including:

• We don't cleanly separate the bounding rect of the primitive (picture) from the bounding rect of the content (surface). This is fine for most cases, but breaks down in the case of blurs, drop-shadows and other complex SVG primitives.

• The way we determine render target scaling differs between whether a surface establishes a raster root or draws in screen space. The scaling factor determination happens at the wrong part of the frame, complicating correctness and the implementation code.

• Needing a precise vs. estimated picture bounding rect requires extra per-primitive work, and complicates surface rect allocation.

• Extra per-primitive work is done to calculate surface clipped rects, and exact picture bounds, which is mostly redundant when we could instead make use of the tile assignment information to determine these.

• Ensuring every surface is a raster root allows simplifying many of the shaders (esp. the image shader) in terms of UV determination, allowing us to fix some correctness issues. Additionally, this will allow us to (in future) cache / tile child surfaces without introducing seam artifacts when drawing them.

Comment 2

[Glenn Watson [:gw]]

Some notes to self from current work, which will need to be filed as follow ups from this work:

Clip chain building:

• We don't reduce pic_coverage_rect by clips in same coord system at all (only in complex transform case). We should do this as an optimization.
• We only reduce pic_coverage_rect if needing a mask. This means that prims may affect (add dependencies on) more tiles than they need to.
• We can probably reduce the produced local_clip_rect in more cases than we currently do.

SurfaceInfo:

• We should completely remove the raster/surface spatial node distinction once raster roots everywhere is stable (they are implicitly the same in this case).

Surface Size:

• Consider other method to reduce size of allocated off-screen surface. The inverse_rect_footprint method means we do more work in some cases than we need to. This would also help with child surface picture caching.

General:

• We should be able to completely remove create_raster_mappers and associated code once raster roots is stable.
• Consider changing how surface.opaque_rect is built to not rely on clusters, but instead use the existing picture cache backdrop functionality.
• Simplify (remove) complexity in get_unclipped_device_rect since the raster root is implicit.

Comment 3

[Glenn Watson [:gw]]

Another follow up once initial work lands:

• Port DropShadow and SVGFilter filter types to make use of the picture graph, one entry per draw, each referencing the same surface.

Comment 4

[Glenn Watson [:gw]]

Attachment: Bug 1749380 - Part 1 - Improve how WR handles bounding rects for off-screen surfaces

This patch introduces a number of subtle but important changes
to how we deal with off-screen surfaces. The overall goals are:

• Improve rendering correctness in a number of edge cases.
• Begin reducing complexity related to surfaces, scaling
  factors, surface size adjustments and clipping.
• Improve CPU performance by removing some per-primitive work.
• Simplify implementation of future SVG and CSS filters by
  having explicit support for picture rects + inflation regions.
• Lay the groundwork for caching child picture surfaces,
  reduction of per-primitive work during visibility pass,
  simplifying picture code.

Unfortunately, the nature of the changes make it impossible to
split up in to small isolated patches. Details below:

• Introduce LocalRectKind concept. This allows us to separate
  out the bounding rect of the surface (a group of primitives
  backed by a texture) from the bounding rect of the picture
  compositing that surface (e.g. a drop-shadow which draws the
  surface once at the local origin and once at a specific offset

  • blur-radius). This fixes a number of correctness bugs we have
    related to culling, clipping, invalidation regions of complex
    primitives such as drop-shadows and blur filters. Importantly,
    it makes it simpler to implement (or fix) SVG filter chains,
    backdrop-filter implementations.

• Establish raster roots for all off-screen surfaces. Every off-screen
  surface uses the spatial node of the enclosing stacking context as
  a coordinate system root, ensuring that each off-screen surface is
  drawn in a 2D coordinate system, with appropriate scaling factors
  applied to ensure high quality rendering. The primary goal is to make
  it possible to correctly inflate and clip off-screen surfaces, removing
  some correctness issues we currently have with complex filters interacting
  with transforms. The initial work here doesn't reduce complexity a huge
  amount, but will allow us to simplify large parts of the picture/surface
  handling code in future, as well as simplify a number of shaders that
  currently must handle arbitrarily complex transform matrices. This will
  also allow us to simplify the implementation of features such as
  mix-blend-mode and backdrop-filter, which rely on readback and UV mapping
  from the parent surface.

• Remove concepts of estimated and precise local rects for pictures. This
  is both a performance optimization and a code simplification. Instead, we
  only determine the estimated local rect during bounding rect propagation,
  and rely on the clipping regions from the tile dirty regions to reduce which
  parts of the picture we allocate if drawing to an off-screen surface. This
  removes some per-primitive work during the visibility pass, and also means
  we can rely on the final picture bounding rect from the start of the visibility
  pass. This also removes much of the complexity in take_context where we
  previously determined surface scale factors and device pixel ratio - instead
  these can be determined earlier during propagate_bounding_rects.

• Remove some complexity in update_prim_visibility. This is still recursive,
  but follow up patches will aim to remove this recursion and integrate this
  pass with the picture graph (similar to how propagate_bounding_rects works).

• Remove PictureOptions struct. Instead, store inflate_if_required with
  the Blur filter enum, which is the only place that uses it.

• Remove root_scaling_factor from text runs - this is handled implicitly
  by the surface device-pixel scale.

• Skip calling update_clip_task for pass-through pictures (since they have
  no defined local rect).

• Improve scaling factors used for determining the render task cache size for
  complex line decorations.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b7f88e5c537b
Improve how WR handles bounding rects for off-screen surfaces r=gfx-reviewers,kvark

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/b7f88e5c537b

Comment 7

[Cristian Tuns]

Backed out as requested by gwatson

• Backout link

Comment 8

[Glenn Watson [:gw]]

This is backed out for now, as it's causing some performance and quality issues that are tricky to fix.

I understand the root cause of them, but applying the fix I expected to work causes quality issues elsewhere (I don't understand these yet, which is why I need to investigate further). Those issues may be a WR bug in these cases, or it may be a Gecko bug, or it may actually be that the content itself that causes the quality issues (in which case we'll need to find an acceptable workaround to handle this with these changes).

The two main issues are:

• Anything that creates an off-screen surface (e.g. filter, border-radius) on an element that has a very large local rect + a scale transform << 1 will result in render targets being larger than needed. The fix is simple, but causes unexpected issues elsewhere that I need to investigate.

• Fractional / device-pixel snapping behavior is un-specced but needs to work in very specific ways when there are simple 2d off-screen surfaces on sub-fractional pixel boundaries. Fixing this is relatively simple, but again causes unexpected issues in some content (e.g. the uBO add-on popup, that appears to expect 1:1 perfect device pixel snapping, even though the display list has both sub-fractional pixel boundaries and a fractional device-pixel scale from the content - the bug here probably lies elsewhere in Gecko or WR, and was working previously by chance).

Comment 9

[Glenn Watson [:gw]]

Attachment: Bug 1749380 - Part 2 - Performance and quality fixes for part 1.

• Add support for local scale factors to a surface, allowing it to
  be rasterized in root coordinate space. This allows snapping to
  work across surfaces where the surface transform is a fractional
  offset.

• Calculate scaling factors per rasterized surface and propagate
  them. Ensures correct scale factor calculations when dealing with
  nested preserve-3d contexts with 90-degree axis rotations.

• Support determining exact surface device rect for 2d surfaces
  with fractional surface transforms.

• Fix line decoration cache key size calculations based on world
  scaling factor.

• Remove get_clipped_device_rect usage for calculating clip-mask
  surface allocations, use surface.get_surface_rect instead. The
  prior method doesn't correctly account for expanded local regions
  from the current dirty rect, resulting in invalidation issues in
  some animated edge cases. Also unifies the way clip-mask surface
  allocations work with the way general render target surface
  allocations work.

Comment 10

[Acasandrei Beatrice (needinfo me)]

(In reply to Cristian Tuns from comment #7)

Backed out as requested by gwatson

• Backout link

== Change summary for alert #33282 (as of Wed, 16 Feb 2022 15:05:10 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
62%	JS	linux1804-64-shippable-qr	tp6	172,583,949.80 -> 279,674,467.52
58%	JS	linux1804-64-shippable-qr	tp6	180,268,213.99 -> 285,519,276.78
47%	Images	linux1804-64-shippable-qr	tp6	6,256,069.68 -> 9,213,581.09

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
30%	Heap Unclassified	linux1804-64-shippable-qr	tp6	310,776,305.67 -> 217,056,909.69
26%	Heap Unclassified	linux1804-64-shippable-qr	tp6	304,276,383.15 -> 224,697,881.33

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=33282

Comment 11

[Alex Finder|PTO, back on Nov 25th]

(In reply to Cristian Tuns from comment #7)

Backed out as requested by gwatson

• Backout link

== Change summary for alert #33286 (as of Wed, 16 Feb 2022 17:14:10 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
34%	google-slides FirstVisualChange	windows10-64-shippable-qr	fission warm webrender	402.75 -> 267.00
28%	google-slides LastVisualChange	macosx1015-64-shippable-qr	fission warm webrender	2,953.33 -> 2,136.67
25%	google-slides LastVisualChange	windows10-64-shippable-qr	fission warm webrender	3,172.42 -> 2,367.00
25%	google-slides SpeedIndex	windows10-64-shippable-qr	fission warm webrender	766.00 -> 576.83
25%	google-slides PerceptualSpeedIndex	windows10-64-shippable-qr	fission warm webrender	799.67 -> 602.58
...	...	...	...	...
4%	google-slides dcf	linux1804-64-shippable-qr	fission warm webrender	1,360.25 -> 1,306.25

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=33286

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9fa6f71111d0
Part 1 - Improve how WR handles bounding rects for off-screen surfaces r=gfx-reviewers
https://hg.mozilla.org/integration/autoland/rev/2b42abbdb0df
Part 2 - Performance and quality fixes for part 1. r=gfx-reviewers,nical

Comment 13

[Glenn Watson [:gw]]

Attachment: Bug 1749380 - Adjust fuzziness expectation on android

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1762adb371d4
Adjust fuzziness expectation on android. r=reftest-fix

Comment 15

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/9fa6f71111d0
https://hg.mozilla.org/mozilla-central/rev/2b42abbdb0df
https://hg.mozilla.org/mozilla-central/rev/1762adb371d4

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1749380 - Fix drag coordinates on X11 after D139243. r=stransky

This gives proper window-relative, CSS-scaled window coordinates on X11.

Before D139243 the coordinates were only correct in non-HiDPI
environments.

Depends on D139244

Comment 17

[Phabricator Automation]

Comment on attachment 9264772 [details]
Bug 1749380 - Fix drag coordinates on X11 after D139243. r=stransky

Revision D139245 was moved to bug 1756241. Setting attachment 9264772 [details] to obsolete.

Comment 18

[Norisz Fay [:noriszfay]]

Backed out from central per devs request for causing webrender crashes

Backout link

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c70bc92ad36
Part 1 - Improve how WR handles bounding rects for off-screen surfaces r=gfx-reviewers
https://hg.mozilla.org/integration/autoland/rev/fc2b3d6448bc
Part 2 - Performance and quality fixes for part 1. r=gfx-reviewers,nical

Comment 20

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/8c70bc92ad36
https://hg.mozilla.org/mozilla-central/rev/fc2b3d6448bc