1318100 - Add support for reftests in webrender

Status: RESOLVED FIXED

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

Description

[Mason Chang [Inactive] [:mchang]]

Add support for reftests with the webrender branch. Depends on updating webrender to https://github.com/servo/webrender/pull/560.

Comment 1

[Mason Chang [Inactive] [:mchang]]

Attachment: Add support for reftests in webrender

If we have a EndTransactionWithTarget, we'll send a sync request to the parent side to do a readback of the pixels. This ends up as a readback via glReadPixels in bindings.rs. The data gets copied back out over IPC and drawn to the requested target.

Comment 2

[Mason Chang [Inactive] [:mchang]]

Also, I'm going to look at replicating how reftests are done in Servo, which is it uses an Epoch to determine when a frame has finished painting and waits for that. We don't really do anything useful with Epoch right now, but I hope to clean up some of this stuff once I take a look at that.

Comment 3

[Markus Stange [:mstange]]

Comment on attachment 8811452 [details] [diff] [review]
Add support for reftests in webrender

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

This probably conflicts a lot with https://github.com/jrmuizel/gecko-dev/pull/53 .

The duplicated code in bindings.rs is unfortunate and the order of operations at the end of it looks strange to me, but since this is all temporary anyway, go ahead.

::: gfx/layers/wr/WebRenderLayerManager.cpp
@@ +268,5 @@
> +  Rect dst(bounds.x, bounds.y, bounds.width, bounds.height);
> +  Rect src(0, 0, bounds.width, bounds.height);
> +
> +  // The data we get from webrender is upside down. So flip and translate up so the image is rightside up.
> +  SurfacePattern pattern(surf, ExtendMode::CLAMP, Matrix::Scaling(1.0, -1.0).PostTranslate(0.0, bounds.height));

Setting the matrix on the DrawTarget and using DrawSurface would probably be faster. Doesn't need to be fixed now though.

::: gfx/webrender/src/bindings.rs
@@ +214,5 @@
>  impl webrender_traits::RenderNotifier for Notifier {
>      fn new_frame_ready(&mut self) {
> +        let &(ref lock, ref cvar) = &*self.render_thread_notifier;
> +        let mut finished = lock.lock().unwrap();
> +        *finished = true;

I won't pretend to know what's going on here.

Comment 4

[Markus Stange [:mstange]]

Comment on attachment 8811452 [details] [diff] [review]
Add support for reftests in webrender

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

::: gfx/layers/wr/WebRenderLayerManager.h
@@ +130,5 @@
> +  // When we're doing a transaction in order to draw to a non-default
> +  // target, the layers transaction is only performed in order to send
> +  // a PLayers:Update.  We save the original non-default target to
> +  // mShadowTarget, and then perform the transaction using
> +  // mDummyTarget as the render target.  After the transaction ends,

I don't see mDummyTarget in this patch. Is this comment still accurate?

@@ +134,5 @@
> +  // mDummyTarget as the render target.  After the transaction ends,
> +  // we send a message to our remote side to capture the actual pixels
> +  // being drawn to the default target, and then copy those pixels
> +  // back to mShadowTarget.
> +  RefPtr<gfxContext> mShadowTarget;

Why not call this just mTarget?

Comment 5

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

(In reply to Mason Chang [:mchang] from comment #0)
> Add support for reftests with the webrender branch. Depends on updating
> webrender to https://github.com/servo/webrender/pull/560.

Presumably we'll get this update as part of bug 1318440.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/f5bea7e99fdc
Add support for reftests in webrender. r=mstange

Comment 7

[Mason Chang [Inactive] [:mchang]]

> ::: gfx/webrender/src/bindings.rs
> @@ +214,5 @@
> >  impl webrender_traits::RenderNotifier for Notifier {
> >      fn new_frame_ready(&mut self) {
> > +        let &(ref lock, ref cvar) = &*self.render_thread_notifier;
> > +        let mut finished = lock.lock().unwrap();
> > +        *finished = true;
> 
> I won't pretend to know what's going on here.

I cleaned up a lot of the horribleness :). Just have the flush notifier now.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/9af4655d0edc
Follow up clean up of WebrenderBridgeChild

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/0fc195181db2
Minor cosmetic cleanup. r=mchang?