1826452 - Consider replacing LayoutDevicePoint::FromAppUnitsRounded in ClipManager::DefineScrollLayers with LayoutDevicePoint::FromAppUnits

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

This FromAppUnitsRounded call is one of the sources of bug 1801098.

Given that tscrollx and tp5o_scroll could catch a perf regression (bug 1825394) caused by removing snapped text base line stuff (bug 1824531), I hoped those test also can catch performance regressions caused by using the non-rounded version of FromAppUnits.

So I just pushed a try run to run those perf test; https://treeherder.mozilla.org/jobs?repo=try&revision=9378b9c86786ccd537873a32dcb46fda966b8c74

The result is interesting, it looks like using the non-rounded version improves tscrollx. Here is a comparison; https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=mozilla-central&newProject=try&newRevision=9378b9c86786ccd537873a32dcb46fda966b8c74&originalSignature=3188480&newSignature=3168146&framework=1&application=&selectedTimeRange=172800&page=1&showOnlyComparable=1 , it's not significant though.

I supposed those tests will be slightly regressed because of the original intention of using the rounded version of the function (bug 1669865). But it doesn't regress at lease those scroll tests.

Is it now worth dropping the rounded version of the function?

Comment 1

[Hiroyuki Ikezoe (:hiro)]

The result might not be able to catch bug 1669865 since as per bug 1669865 comment 0 it requires full-zoom to reproduce the issue. So I tried with layout.css.devPixelsPerPx=1.5 (I suppose it has the same characteristic with full-zoom) (note that our Macs on CI are not HiDPI at all).

Here is a comparison rounded/non-rounded FromAppUnits (both with layout.css.devPixelsPerPx=1.5), it's improving!
https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=fe9827dc552b05329da2b594b134bfccc68a6500&newProject=try&newRevision=e3a44dc050c61e2c99e30e8b281dcd1d314d40e5&framework=1&page=1&showOnlyComparable=1

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1826452 - Use non-rounded external scroll offsets behind a pref. r?botond!,gw!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/95470214c6cb
Use non-rounded external scroll offsets behind a pref. r=botond,gw

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/95470214c6cb