1553571 - MotionMark 1.1 - test "Suits shape only": visualization errors with Webrender activated.

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Rival]

Attachment: webrender1.jpg

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0

Steps to reproduce:

With the landing of Webrender on Firefox stable, in my opinion, the most used benchmark to highlight the performance improvements obtained thanks to Webrender, will be MotionMark.
Of course it is desirable that Webrender works in the best way with this benchmark.

With my configuration, with tests run with Webrender enabled and not, I noticed some problems.

MotionMark ver1.1 and ver1.0
Firefox 67.0 64bit
Windows 10 build 1803 (display: 2560x1440 60hz)
GPU GTX1600 - 6GB (driver: 430.64)
CPU i5 6600K - 16GB

• Test "Suits: shape only"
  https://browserbench.org/MotionMark1.1/developer.html?test-interval=30&display=minimal&tiles=big&controller=ramp&frame-rate=50&kalman-process-error=1&kalman-measurement-error=4&time-measurement=performance&suite-name=Suitssuite&test-name=Suitsshapeonly

Actual results:

By activating Webrender the scores are excellent, but the "shape only" figures are horribly visualized, they appear to be displayed at a much lower resolution and then enlarged and blurred, or as if they have suffered extreme compression.

You can see attached image.

Instead, no problem with the "clip only" figures of the same type of subtest:
https://browserbench.org/MotionMark1.1/developer.html?test-interval=30&display=minimal&tiles=big&controller=ramp&frame-rate=50&kalman-process-error=1&kalman-measurement-error=4&time-measurement=performance&suite-name=Suitssuite&test-name=Suitscliponly

The same problem also in the 1.0 version of MotionMark, in which the "Suits" test simultaneously uses figures "shape" and "clip", in fact only SOME figures (those "shape") are not correctly displayed:
https://browserbench.org/MotionMark/developer.html?test-interval=30&display=minimal&tiles=big&controller=ramp&frame-rate=50&kalman-process-error=1&kalman-measurement-error=4&time-measurement=performance&suite-name=Animometer&test-name=Suits

Comment 1

[Jessie [:jbonisteel] pls NI]

Nical, could you take a peak at this and let me know what you think?

Comment 2

[Nicolas Silva [:nical]]

On this test we have a number of svg paths with a translatlation and a scale. It's hard to see if the scale is animated but the translation obviously is. We do a good job at layerizing a blob per moving element, but the blobs are obviously rasterized at the pre-scale resolution which makes them look like a blurry mess.

Jeff, you better know how the blob layerization works, any idea/pointer?

Comment 3

[Jeff Muizelaar [:jrmuizel]]

I believe this is now fixed in Nightly. Rival, can you confirm?

Comment 4

[Rival]

Sorry Jeff,
but with the latest build available to me on the channel, (70.0a1 build id: 20190826094255),
the problem is still unchanged.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Timothy, this seems a bit like bug 1569215. Do you want to take a look when you get a chance?

Comment 6

[Timothy Nikkel (:tnikkel)]

Regressed by bug 1519444. But I'm assuming that just exposed a regression from bug 1415987 by applying that bug to smaller layers. Checking that now.

Comment 7

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1553571. Pass the size of the display item to the StackingContextHelper constructor in nsDisplayTransform::CreateWebRenderCommands. r=jrmuizel

In FrameLayerBuilder::ChooseScale we hit this line https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/layout/painting/FrameLayerBuilder.cpp#6124 and aVisibleRect is empty (because every call site except for nsDisplayMasksAndClipPaths::CreateWebRenderCommands (which doesn't need to) passes empty for the size of the bounds) and so the maxScale stays at 4, and we clamp to 4 instead of something 30-50.

The call to ChooseScale in StackingContextHelper::StackingContextHelper is the only place the size of aBounds is ever looked at. And we only ever call ChooseScale if we are passed a mBoundTransform which only nsDisplayTransform does. So this change should be quite safe.

Comment 8

[Timothy Nikkel (:tnikkel)]

(In reply to Timothy Nikkel (:tnikkel) from comment #6)

Regressed by bug 1519444. But I'm assuming that just exposed a regression from bug 1415987 by applying that bug to smaller layers. Checking that now.

Indeed, if you revert bug 1415987 but leave bug 1519444 then the bug doesn't appear.

Comment 9

[Rival]

I don't know, if you can use this information to investigate the cause of the bug,
but the graphic problem does not occur, setting in firefox 68:

gfx.webrender.blob.invalidation = false

Naturally, with this setting, the performances (scores) fall by 70%, even lower than the scores
obtained with a completely disabled webrender. ;)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5395a79f7a0b
Pass the size of the display item to the StackingContextHelper constructor in nsDisplayTransform::CreateWebRenderCommands. r=jrmuizel

Comment 11

[Noemi Erli[:noemi_erli]]

Backed out changeset 5395a79f7a0b (Bug 1553571) for causing reftest failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=264122154&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&classifiedState=unclassified&searchStr=0f56530b69e1f742c9f75e13c5a4151e7845074e&revision=5395a79f7a0b00cc89d3a4d567d542789808900e

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=264122154&repo=autoland&lineNumber=26537

Backout: https://hg.mozilla.org/integration/autoland/rev/b1248ff798589712c53e7d5de49eebf596ee67ed

Comment 12

[Timothy Nikkel (:tnikkel)]

Oh yeah, I figured it might need some fuzz to pass but it passed locally on my machine without. I'll add fuzz and reland as we don't need pixel perfect rendering here, just testing to make sure it's not a vastly lower resolution image upscaled larger.

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a39bef5e2ad
Backed out changeset 5395a79f7a0b for causing reftest failures

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/934307ae011e
Pass the size of the display item to the StackingContextHelper constructor in nsDisplayTransform::CreateWebRenderCommands. r=jrmuizel

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/934307ae011e

Comment 16

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-beta/rev/934307ae011e