Move display item caching logic to DisplayListBuilder
Categories
(Core :: Graphics: WebRender, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox76 | --- | fixed |
People
(Reporter: mikokm, Assigned: mikokm)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
This improves the separation of concerns of the implementation. The actual result of caching should be the same. The change was needed to allow caching decision to be moved inside nsDisplayItem::CreateWebRenderCommands()
(or anywhere with DisplayItemBuilder
in scope), instead of doing it by per-item basis beforehand.
Old design when caching an item:
WebRenderCommandBuilder::CreateWebRenderCommands()
notifiedDisplayItemCache
thatnsDisplayItem::CreateWebRenderCommands()
was about to be called.DisplayItemCache
assigned the cache slots to the item and notifiedDisplayItemBuilder
if the item supported caching.WebRenderCommandBuilder::CreateWebRenderCommands()
notifiedDisplayItemCache
thatnsDisplayItem::CreateWebRenderCommands()
was finished.DisplayItemCache
usedDisplayItemBuilder
to push additional items, if the item was cached.
New design:
DisplayItemBuilder
now has methods:
void StartGroup(nsPaintedDisplayItem* aItem);
void CancelGroup();
void FinishGroup();
bool ReuseItem(nsPaintedDisplayItem* aItem);
Any WR display items created between calls to StartGroup()
and FinishGroup()
will be reused by a call to ReuseItem()
, which will push a DisplayItem::ReuseItem(key)
to WR display list. This call is (still) inside WebRenderCommandBuilder::CreateWebRenderCommands()
. A call to CancelGroup()
will discard display items pushed after the call to StartGroup()
.
For example, inside nsDisplayBackgroundColor::CreateWebRenderCommands()
:
aBuilder.StartGroup(this);
aBuilder.PushRect(r, r, !BackfaceIsHidden(),
wr::ToColorF(ToDeviceColor(mColor)));
aBuilder.FinishGroup();
DisplayItemCache
is now only responsible for assigning the cache slots and ensuring that the cache state is valid. This means handling some awkward edge-cases like changing pipeline/clip/spatial ids, which require cache invalidation. Additionally it is an optimization to avoid bloating gecko display items with additional state data.
Assignee | ||
Comment 1•5 years ago
|
||
Pushed by mikokm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/91760460f914 Refactor WebRender display item caching r=jrmuizel
Comment 3•5 years ago
|
||
Backed out for build bustages
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=91760460f91496fb2262242dcdf7c0deb311978c&selectedJob=292717043
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=292717043&repo=autoland&lineNumber=13889
Backout: https://hg.mozilla.org/integration/autoland/rev/b3d8484a2aaa21f511f43d1b1e42178e5efb2298
Assignee | ||
Updated•5 years ago
|
Pushed by mikokm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f792d895cdf8 Refactor WebRender display item caching r=jrmuizel
Comment 5•5 years ago
|
||
bugherder |
Comment 6•4 years ago
|
||
Could this have caused a performance improvement? I see a big step down on the displaylist_mutate test when this landed: https://treeherder.mozilla.org/perf.html?selectMulti=displaylist_mutate+opt+e10s+stylo#/graphs?highlightAlerts=1&selected=1937901,1073835236&series=mozilla-central,1937901,1,1&series=mozilla-central,1934058,1,1&timerange=7776000&zoom=1583907509944,1584031730802,1273.26814920112,1625.1326319897125
Assignee | ||
Comment 7•4 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #6)
Could this have caused a performance improvement? I see a big step down on the displaylist_mutate test when this landed: https://treeherder.mozilla.org/perf.html?selectMulti=displaylist_mutate+opt+e10s+stylo#/graphs?highlightAlerts=1&selected=1937901,1073835236&series=mozilla-central,1937901,1,1&series=mozilla-central,1934058,1,1&timerange=7776000&zoom=1583907509944,1584031730802,1273.26814920112,1625.1326319897125
I think that bump was caused by bug 1619461.
The improvements from this feature were similar though (for WR display list bound benchmarks), but reported in bug 1616412, where the pref was flipped on.
Description
•