1436020 - During SanityTest, verifyLayersRendering() always failed with WebRender

Status: RESOLVED FIXED

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

Description

[Sotaro Ikeda [:sotaro]]

Created based on Bug 1435693 comment 6.

Comment 1

[Sotaro Ikeda [:sotaro]]

The verifyLayersRendering() was failed on my PC, because I enabled WebRender Profiler UI.

Comment 2

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #1)
> The verifyLayersRendering() was failed on my PC, because I enabled WebRender Profiler UI.

When WebRender Profiler UI was disabled, the verifyLayersRendering() was always passed.

Comment 3

[Sotaro Ikeda [:sotaro]]

From Comment 2, it seems better to disable WebRender Profiler during verifyLayersRendering().

Comment 4

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #3)
> From Comment 2, it seems better to disable WebRender Profiler during
> verifyLayersRendering().

Correction:
it seems better to disable WebRender Profiler during takeWindowSnapshot().

Comment 5

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Disable WebRender profiler during taking snap shot of sanity test

Comment 6

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8948922 [details] [diff] [review]
patch - Disable WebRender profiler during taking snap shot of sanity test

Review of attachment 8948922 [details] [diff] [review]:
-----------------------------------------------------------------

Do we ever want to show the WebRender profiler UI when taking a snapshot?

I think we should just disable the profiler overlay for all snapshots (in the C++ code), so that callers don't have to deal with prefs.

Comment 7

[Sotaro Ikeda [:sotaro]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #6)
> Comment on attachment 8948922 [details] [diff] [review]
> patch - Disable WebRender profiler during taking snap shot of sanity test
> 
> I think we should just disable the profiler overlay for all snapshots (in
> the C++ code), so that callers don't have to deal with prefs.

Yea, it seems like a reasonable approach.

Comment 8

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Disable WebRender profiler UI during readback

Comment 9

[Sotaro Ikeda [:sotaro]]

attachment 8949307 [details] [diff] [review] locally defines WebRender's DebugFlags. It should be defined in webrender_ffi_generated.h. I tried to update webrender_ffi_generated.h as to include DebugFlags types. But even with default gecko source, `rustup run nightly cbindgen toolkit/library/rust/ --crate webrender_bindings -o gfx/webrender_bindings/webrender_ffi_generated.h` failed with error on Linux and on Windows :(

DebugFlags was update in WebRender and is going to be imported to gecko.
 https://github.com/servo/webrender/pull/2382

By the way WebRenderDebugPrefChangeCallback() also defines DebugFlags types locally.
https://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxPlatform.cpp#598

Comment 10

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #9)
> But even with default gecko source, `rustup run nightly cbindgen
> toolkit/library/rust/ --crate webrender_bindings -o
> gfx/webrender_bindings/webrender_ffi_generated.h` failed with error on Linux
> and on Windows :(

I got following error with default m-c source.

> rustup run nightly cbindgen toolkit/library/rust/ --crate webrender_bindings -o gfx/webrender_bindings/webrender_ffi_generated.h
ERROR: Parsing crate `webrender`:``:
unparsed tokens after crate: "
ERROR: Couldn't generate bindings for toolkit/library/rust/.

Comment 11

[Sotaro Ikeda [:sotaro]]

:rhunt, is there a tip for updating webrender_ffi_generated.h?

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Apply this patch on your tree temporarily: https://hg.mozilla.org/try/rev/e3f85ef18dca6d6bfc842bb1dbaa9bb188c1d0a5 then run cbindgen, and then undo the patch.

This has been fixed in upstream cbindgen by using the latest version of syn, but there are other problems so it hasn't been published yet.

Comment 13

[Sotaro Ikeda [:sotaro]]

Thanks!

Comment 14

[Nicolas Silva [:nical]]

Comment on attachment 8949307 [details] [diff] [review]
patch - Disable WebRender profiler UI during readback

Review of attachment 8949307 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, but maybe we should just disable all debug flags when taking the screenshot? Just setting them all to zero and restore them after the call. That way you don't even need to worry about maintaining the constants by hand.

Comment 15

[Sotaro Ikeda [:sotaro]]

(In reply to Nicolas Silva [:nical] from comment #14)
> 
> Looks good to me, but maybe we should just disable all debug flags when
> taking the screenshot? Just setting them all to zero and restore them after
> the call. That way you don't even need to worry about maintaining the
> constants by hand.

Thanks for the advice, it is simpler!

Comment 16

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Disable WebRender profiler UI during readback

Comment 17

[Sotaro Ikeda [:sotaro]]

Bug 1437949 is going to re-add RendererOGL::Update(). Then it seems better to update the patch as to restore DebugFlag.

Comment 18

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Disable WebRender profiler UI during readback

Comment 19

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #17)
> Bug 1437949 is going to re-add RendererOGL::Update(). Then it seems better
> to update the patch as to restore DebugFlag.

I change my mind. I am going to handle Bug 1437949 related side effect as a follow up.

Comment 20

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d69ca2d5cda5cbddf4593bed0045c0b592707444
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b89e0625775b278e8ffe8881cd5a572534bf38e3

Comment 21

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Disable WebRender profiler UI during readback

Rebased.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1647b6d2973f
Disable WebRender profiler UI during readback r=nical

Comment 23

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1647b6d2973f