Wrench replay doesn't seem to work with Gecko

RESOLVED FIXED in Firefox 55

Status

()

Core
Graphics: WebRender
RESOLVED FIXED
8 months ago
6 months ago

People

(Reporter: rhunt, Assigned: rhunt)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment)

(Assignee)

Description

8 months ago
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)
(Assignee)

Comment 1

8 months ago
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
(Assignee)

Comment 2

8 months ago
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
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.
(Assignee)

Comment 4

8 months ago
Created attachment 8854167 [details] [diff] [review]
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.
Assignee: nobody → rhunt
Attachment #8854167 - Flags: review?(jmuizelaar)
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?
Attachment #8854167 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 6

8 months ago
Yes. I definitely did not. Thanks!

Comment 7

7 months ago
Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/projects/graphics/rev/3198a84c53f4
Add a comment about webrender binary recording r=jrmuizel
Status: NEW → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → FIXED
https://hg.mozilla.org/mozilla-central/rev/3198a84c53f4a9e9c67dbb8ee6504ae09994d8ad
status-firefox55: --- → fixed
Target Milestone: --- → mozilla55

Updated

6 months ago
See Also: → bug 1366502
See Also: → bug 1367689
You need to log in before you can comment on or make changes to this bug.