1352657 - Wrench replay doesn't seem to work with Gecko

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Ryan Hunt [:rhunt]]

I've been unable to successfully use wrench replay recently with graphics head.

I've been enabling recording in bindings.rs, and running wrench from a webrender repo with the current webrender version (retrieved from README.webrender).

The error:

$ RUST_BACKTRACE=1 cargo run -- replay ../../firefox/wr-record-1.bin
    Finished debug [optimized + debuginfo] target(s) in 0.0 secs
     Running `c:\src\webrender\target\debug\wrench.exe replay ../../firefox/wr-record-1.bin`
OpenGL version 3.2.0 NVIDIA 378.49, GeForce GTX 1080/PCIe/SSE2
hidpi factor: 1 (native 1)
Shader override path: None
Warning: binary file ApiMsg type mismatch: expected 0xee14ce53f7c80a02, found 0xc01ee62c41c3f423
Frame 0 offset: 16
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Custom("invalid value: integer `201326592`, expected variant index 0 <= i < 26")', C:\bot\slave\stable-dist-rustc-win-msvc-64\build\src\libcore\result.rs:837
stack backtrace:
   9:     0x7ff6bf885d72 - wrench::binary_frame_reader::{{impl}}::do_frame
                        at C:\src\webrender\wrench\src\binary_frame_reader.rs:136
\wrench.exe replay ../../firefox/wr-record-1.bin` (exit code: 3221225501)

Comment 1

[Ryan Hunt [:rhunt]]

I noticed that the gk-rust crate specifies specific profile properties for dev and release that might affect the layout of ApiMsg and TypeId. [1]

So I modified my webrender repo to have the same values. The ApiMsg type mismatch changed, but it didn't fix the issue.

[1] http://searchfox.org/mozilla-central/rev/f668d2b2c1413452d536210b3ea2999eb81824f3/toolkit/library/rust/Cargo.toml

Comment 2

[Ryan Hunt [:rhunt]]

cc'ing Jeff to see if it works for him or if he has any thoughts
cc'ing Jamie because he was having issues and may have resolved them

Comment 3

[Jeff Muizelaar [:jrmuizel]]

I had this working earlier this week. I needed to patch gecko and webrender to get it to work.

First make sure you're running with a webrender that matches the version in gecko. (It seems like you are)

Here's the gecko patch. The WebRenderBridgeParent part just disables external images, so that they work like regular images.

diff --git a/gfx/layers/wr/WebRenderBridgeParent.cpp b/gfx/layers/wr/WebRenderBridgeParent.cpp
index a2a12a7b..a470ffe 100644
--- a/gfx/layers/wr/WebRenderBridgeParent.cpp
+++ b/gfx/layers/wr/WebRenderBridgeParent.cpp
@@ -338,17 +338,17 @@ WebRenderBridgeParent::ProcessWebRenderCommands(const gfx::IntSize &aSize,
         }
         // XXX select Texture for video in CompositeToTarget().
         TextureHost* texture = host->GetAsTextureHost();
         if (!texture) {
           NS_ERROR("TextureHost does not exist");
           break;
         }
         WebRenderTextureHost* wrTexture = texture->AsWebRenderTextureHost();
-        if (wrTexture) {
+        if (false && wrTexture) {
           // XXX handling YUV
           gfx::SurfaceFormat format =
             wrTexture->GetFormat() == SurfaceFormat::YUV ? SurfaceFormat::B8G8R8A8 : wrTexture->GetFormat();
           wr::ImageDescriptor descriptor(wrTexture->GetSize(), wrTexture->GetRGBStride(), format);
           mApi->AddExternalImageBuffer(key,
                                        descriptor,
                                        wrTexture->GetExternalImageKey());
           mCompositableHolder->HoldExternalImage(mPipelineId, aEpoch, texture->AsWebRenderTextureHost());
diff --git a/gfx/webrender_bindings/src/bindings.rs b/gfx/webrender_bindings/src/bindings.rs
index c92e04e..8c4f3fc 100644
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -21,17 +21,17 @@ use webrender_traits::RepeatMode;
 use webrender::renderer::{Renderer, RendererOptions};
 use webrender::renderer::{ExternalImage, ExternalImageHandler, ExternalImageSource};
 use webrender::{ApiRecordingReceiver, BinaryRecorder};
 use app_units::Au;
 use euclid::{TypedPoint2D, SideOffsets2D};
 
 extern crate webrender_traits;
 
-static ENABLE_RECORDING: bool = false;
+static ENABLE_RECORDING: bool = true;
 
 // This macro adds some checks to make sure we notice when the memory representation of
 // types change.
 macro_rules! check_ffi_type {
     ($check_sizes_match:ident struct $TypeName:ident as ($T1:ident, $T2:ident)) => (
         fn $check_sizes_match() {
             #[repr(C)] struct TestType($T1, $T2);
             let _ = mem::transmute::<$TypeName, TestType>;

In webrender I got a panic when during a map lookup related to clip ids or something like that. I just commented the line out.

The ApiMsg mismatch messages are somewhat expected. i.e. I've successfully used it while getting that message.

In your case it looks like perhaps the buffer that it's reading from is too short. You may need to do some investigation into why this is. Otherwise, I'll take a look on Monday.

Comment 4

[Ryan Hunt [:rhunt]]

Attachment: comment.patch

From a discussion on irc, this has been resolved by using this patch and running 'cargo update -p bincode' in wrench. The bincode update has been upstreamed already.

The only changes that should be needed to record for wrench are to flip ENABLE_RECORDING and disable external images. It could be nice to have external images automatically disabled when ENABLE_RECORDING is flipped on. But for now, at the least I'd like to add a comment.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8854167 [details] [diff] [review]
comment.patch

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

The comment is good. But I assume you didn't mean to include the other changes?

Comment 6

[Ryan Hunt [:rhunt]]

Yes. I definitely did not. Thanks!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/3198a84c53f4
Add a comment about webrender binary recording r=jrmuizel

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/3198a84c53f4a9e9c67dbb8ee6504ae09994d8ad