1893088 - Split coord space mapping for stacking context coord / external scroll offsets

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, task)

Comment 1

[Glenn Watson [:gw]]

Attachment: Bug 1893088 - Split coord space mapping for stacking context coord / external scroll offsets

During display list and scene building, there are two coordinate
remapping steps that occur:

1. From stacking context coords -> reference frame relative coords
2. From pre-scrolled coords -> removed external scrolling offsets

These were previously handled in one place, however we want to
split these up so that we can apply snapping after step 1 but
prior to step 2. This will allow us to have fractional external
scroll offsets that don't affect snapping. These will be snapped
later on during frame build after applying any (possibly fractional
APZ scroll offsets). This is a cheap operation during frame building
as we only need to snap and modify the transform matrices, not
individual primitives.

This patch should have no functional changes, it's prep work for
the changes referenced above. It does move all of step 1 to be
done during DL building in the content process, and all of step
2 to be done during scene building in the GPU process. In future,
if/when we resolve the issues we have with reliance on cross-iframe
knowledge for fractional snapping, we can move step 2 (including
snapping) in to the content process as well.

Further, as part of the DL bypass work, we will need to remap coord
spaces during DL building in a differeny way, which this simplifies.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f9a83173704
Split coord space mapping for stacking context coord / external scroll offsets r=gfx-reviewers,nical

Comment 3

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/9f9a83173704

Comment 4

[Serban Stanca [:SerbanS]]

Backed out for causing talos related process-crashes.

• Backout link
• Push with failures - 1
• Failure Log

---

• Push with failures - 2
• Failure Log

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: graphical glitch that was caused by the landing in comment 2-3

I noticed a regression in Firefox Nightly that bisects to being caused by this bug's commit (though it looks like this has already been backed out, so presumably the regression will be gone in the next Nightly):
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d6fdf720a9b7d0fd0bd443fa07564093d18a24f9&tochange=9f9a831737047da5c6b52e522fef4b82755b7d41

I'm attaching a screenshot of the regression.

STR to trigger this issue:

1. Click the v dropdown-menu button at the right edge of the tab-strip.
2. Click "New Container Tab"
3. Look at the menu items (Personal|Work|Banking|Shopping)

EXPECTED RESULTS:
The hotkey underlines for the menu items' first letters should be properly positioned below those letters (and consistently-sized).

ACTUAL RESULTS:
The hotkey underlines are mispositioned (they're too high up, intersecting the letters), and their sizes are also surprisingly variable. (For "P", "B", and "S", they're quite skinny. For "W" it looks like it's roughly the right size.)

Comment 6

[Daniel Holbert [:dholbert]]

(Ideally we should address comment 5 before re-landing, to avoid (re)introducing that graphical regression.)

Comment 7

[Daniel Holbert [:dholbert]]

Comment 5 is from Nightly 127.0a1 (2024-04-30) on Ubuntu 22.04 on my ThinkStation desktop. I see the same bug on the same Nightly in Windows 11 as well. (On macOS it looks like these underlines aren't present, maybe, so this might not be testable there.)

Comment 8

[Glenn Watson [:gw]]

I will return to looking at this in a week or two, and the regression, before landing, but working on some higher priority things at the moment.

Comment 9

[Mayank Bansal]

Almost certain that patches from this bug caused perf regressions on tscrollx : https://treeherder.mozilla.org/perfherder/alerts?id=59 , and that the subsequent backout of the patches fixed the regression : https://treeherder.mozilla.org/perfherder/alerts?id=124&hideDwnToInv=0

Comment 10

[Andrej (:aglavic)]

(In reply to Mayank Bansal from comment #9)

Almost certain that patches from this bug caused perf regressions on tscrollx : https://treeherder.mozilla.org/perfherder/alerts?id=59 , and that the subsequent backout of the patches fixed the regression : https://treeherder.mozilla.org/perfherder/alerts?id=124&hideDwnToInv=0

Perfherder has detected a talos performance change from push 476a989f5352b5aa87bc5c2932415f2cb64471ad.

Improvements we see from backing out the patch:

Ratio	Test	Platform	Options	Absolute values (old vs new)
31%	tscrollx	macosx1015-64-shippable-qr	e10s fission stylo webrender	1.04 -> 0.72
30%	tscrollx	macosx1015-64-shippable-qr	e10s fission stylo webrender	1.04 -> 0.72
24%	tscrollx	linux1804-64-shippable-qr	e10s fission stylo webrender	1.78 -> 1.35

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 124

For more information on performance sheriffing please see our FAQ.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5d01eb677b9
Split coord space mapping for stacking context coord / external scroll offsets r=gfx-reviewers,nical

Comment 12

[Serban Stanca [:SerbanS]]

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