1322504 - Use TextureClient in WebRenderPaintedLayer

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Comment 1

[Ethan Lin[:ethlin]]

Per discussion with sotaro, I'll use ContentClient to handle TextureClient in WebRenderPaintedLayer.

Comment 2

[Sotaro Ikeda [:sotaro]]

Bug 1325227 is going to make transaction of ContentClient to async.

Comment 3

[Sotaro Ikeda [:sotaro]]

Using SingleTiledContentClient might make the bug simpler.

Comment 4

[Ethan Lin[:ethlin]]

(In reply to Sotaro Ikeda [:sotaro PTO 28/Dec - 4/Jan] from comment #3)
> Using SingleTiledContentClient might make the bug simpler.

I imported ContentClient first, and it's also simple after I disabled RotationBuffer. I'll upload related patch for review. And I will start to work on SingleTiledContentClient.

Comment 5

[Ethan Lin[:ethlin]]

Attachment: Part1. Use ContentClient in WebRenderPaintedLayer

Comment 6

[Ethan Lin[:ethlin]]

Attachment: Part2. Disable RotationBuffer for WR and fix bound/format problem after using ContentClient

We may need to change WR interface to support RotationBuffer. So I disable it for now. And I also fix some format and boundary problem in this patch after using ContentClient.

Comment 7

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8822362 [details] [diff] [review]
Part2. Disable RotationBuffer for WR and fix bound/format problem after using ContentClient

The patch caused the following error on linux. Review+ if the following problem is addressed.

----------------------------------- 0:04.64 /home/sotaro/quantum_gfx_src_6/graphics/gfx/layers/wr/WebRenderBridgeParent.cpp:305:22: error: variable ‘size’ set but not used [-Werror=unused-but-set-variable]
 0:04.64          gfx::IntSize size = dSurf->GetSize();

Comment 8

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8822360 [details] [diff] [review]
Part1. Use ContentClient in WebRenderPaintedLayer

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

ethlin, can you address the comment?

::: gfx/layers/wr/WebRenderLayerManager.cpp
@@ +325,5 @@
>  
> +  // this may result in Layers being deleted, which results in
> +  // PLayer::Send__delete__() and DeallocShmem()
> +  mKeepAlive.Clear();
> +

It is called only during snapshot. The mKeepAlive.Clear() needs to be called alwasys after transaction end.

::: gfx/layers/wr/WebRenderPaintedLayer.h
@@ +61,5 @@
> +    }
> +  }
> +
> +  bool UsedForReadback() { return mUsedForReadback; }
> +  void SetUsedForReadback(bool aUsed) { mUsedForReadback = aUsed; }

UsedForReadback() and SetUsedForReadback() are duplicate of the following. Is there a reason for it?
https://dxr.mozilla.org/mozilla-central/source/gfx/layers/Layers.h#2044

Comment 9

[Ethan Lin[:ethlin]]

(In reply to Sotaro Ikeda [:sotaro PTO 28/Dec - 4/Jan] from comment #8)
> Comment on attachment 8822360 [details] [diff] [review]
> Part1. Use ContentClient in WebRenderPaintedLayer
> 
> Review of attachment 8822360 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ethlin, can you address the comment?
> 
> ::: gfx/layers/wr/WebRenderLayerManager.cpp
> @@ +325,5 @@
> >  
> > +  // this may result in Layers being deleted, which results in
> > +  // PLayer::Send__delete__() and DeallocShmem()
> > +  mKeepAlive.Clear();
> > +
> 
> It is called only during snapshot. The mKeepAlive.Clear() needs to be called
> alwasys after transaction end.
> 

Okay, I should move it to the end of EndTransaction.

> ::: gfx/layers/wr/WebRenderPaintedLayer.h
> @@ +61,5 @@
> > +    }
> > +  }
> > +
> > +  bool UsedForReadback() { return mUsedForReadback; }
> > +  void SetUsedForReadback(bool aUsed) { mUsedForReadback = aUsed; }
> 
> UsedForReadback() and SetUsedForReadback() are duplicate of the following.
> Is there a reason for it?
> https://dxr.mozilla.org/mozilla-central/source/gfx/layers/Layers.h#2044

Nope, I'll remove them.

Comment 10

[Ethan Lin[:ethlin]]

Attachment: Part1. Use ContentClient in WebRenderPaintedLayer

Address sotaro's comments.

Comment 11

[Ethan Lin[:ethlin]]

Attachment: Part2. Disable RotationBuffer for WR and fix bound/format problem after using ContentClient. r=sotaro

Fix build break.

Comment 12

[Ethan Lin[:ethlin]]

Attachment: Part2. fix bound/format problem after using ContentClient. r=sotaro

It looks like we need visible rect info for creating WR image or we may get wrong image region. So I pass the visible rect in TOpDPPushExternalImageId. I also separate the change of preference from this patch.

Comment 13

[Ethan Lin[:ethlin]]

Attachment: Part3. Disable RotationBuffer when using WR. r=sotaro

Comment 14

[Sotaro Ikeda [:sotaro]]

The patches seem to cause the regression. During scrolling http://www.yahoo.co.jp/, I saw a bit of rendering problem.

Comment 15

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8823201 [details] [diff] [review]
Part2. fix bound/format problem after using ContentClient. r=sotaro

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

Can you explain why the patch added "IntRect visibleRect"? Since "WRRect bounds" of OpDPPushExternalImageId delivers almost same information.

Comment 16

[Ethan Lin[:ethlin]]

(In reply to Sotaro Ikeda [:sotaro PTO 28/Dec - 4/Jan] from comment #15)
> Comment on attachment 8823201 [details] [diff] [review]
> Part2. fix bound/format problem after using ContentClient. r=sotaro
> 
> Review of attachment 8823201 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can you explain why the patch added "IntRect visibleRect"? Since "WRRect
> bounds" of OpDPPushExternalImageId delivers almost same information.

For WebRenderCanvasLayer, the 'bounds' is a transformed rect, so the size may be different from visible rect. Why we need to do the transform for the 'bounds' is because the unit of buffer data in CanvasLayer is not device pixel. Actually, I think we should do the transform for PaintedLayer and ImageLayer too, though normally the transform is identify matrix (I'm not sure when is not).

I sent the result to try server and I found that I should pass visible region to parent side for clipping. I'll change the visibleRect to visibleRegion.

Comment 17

[Ethan Lin[:ethlin]]

I got many unexpected-pass and some unexpected-fail from try server. I'll mark these tests since unexpected-pass tests are more than unexpected-fail.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f32c540b37e11fd616c9bfac824e7bbebbc98a2&selectedJob=66989832

Comment 18

[Ethan Lin[:ethlin]]

Attachment: Part2. fix bound/format problem after using ContentClient.

We should send visible region to parent for culling. For now I do the culling by Moz2D. We should do that in WebRender in the future.

Comment 19

[Ethan Lin[:ethlin]]

Attachment: Part1. Use ContentClient in WebRenderPaintedLayer. r=sotaro

Rebase the patch.

Comment 20

[Ethan Lin[:ethlin]]

Attachment: Part4. Set reftest flag.

Change some flags for reftests for passing try server tests.

Comment 21

[Ethan Lin[:ethlin]]

Attachment: Part4. Set reftest flag.

Correct some flags.

Comment 22

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

Comment on attachment 8824913 [details] [diff] [review]
Part4. Set reftest flag.

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

::: 0002-mask.patch
@@ +4,5 @@
> +mask
> +
> +---
> + gfx/layers/wr/WebRenderBorderLayer.cpp  | 1 +
> + gfx/layers/wr/WebRenderCanvasLayer.cpp  | 1 +

Spurious file in this patch

::: layout/reftests/bugs/reftest.list
@@ +1931,5 @@
>  skip-if(!Android) == 1133905-4-vh-rtl.html 1133905-ref-vh-rtl.html
>  skip-if(!Android) fails-if(Android) == 1133905-5-vh-rtl.html 1133905-ref-vh-rtl.html
>  skip-if(!Android) fails-if(Android) == 1133905-6-vh-rtl.html 1133905-ref-vh-rtl.html
>  == 1150021-1.xul 1150021-1-ref.xul
> +fuzzy-if(webrender,139,10) == 1151145-1.html 1151145-1-ref.html

I would prefer to mark this as fails-if(webrender) rather than fuzzy, at least until we can investigate it fully. From the try push it looks like the caps of the scrollthumb are a little off, but it's not obvious to me that it's not fixable.

::: layout/reftests/webm-video/reftest.list
@@ +41,5 @@
>  # particular (non-spec-mandated) video codec.
>  default-preferences test-pref(layout.css.object-fit-and-position.enabled,true) test-pref(gfx.ycbcr.accurate-conversion,true)
> +fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-contain-webm-001.html object-fit-contain-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
> +fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-contain-webm-002.html object-fit-contain-webm-002-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
> +fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-cover-webm-001.html object-fit-cover-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures

Are these actually random-if? From the try push you linked it seems like these pass in non-e10s and continue to fail in e10s. Can we annotate them accordingly? random-if is "worse" because then if starts failing or passing consistently we won't automatically find out.

Comment 23

[Ethan Lin[:ethlin]]

Attachment: Part4. Set reftest flag.

Basing on try server result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc25a108a0c82bbb6cb67778d2d29606f739f530&selectedJob=67189781, update the flags.

Comment 24

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #14)
> The patches seem to cause the regression. During scrolling
> http://www.yahoo.co.jp/, I saw a bit of rendering problem.

I still see some small artifacts during scrolling http://www.yahoo.co.jp/ on my linux pc with patches. It might be related to RotatedBuffer.

:ethlin, don't you see the problem on your side?

Comment 25

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #24)
> (In reply to Sotaro Ikeda [:sotaro] from comment #14)
> > The patches seem to cause the regression. During scrolling
> > http://www.yahoo.co.jp/, I saw a bit of rendering problem.
> 
> I still see some small artifacts during scrolling http://www.yahoo.co.jp/ on
> my linux pc with patches. It might be related to RotatedBuffer.
> 
> :ethlin, don't you see the problem on your side?

I scrolled the page with mouse.

Comment 26

[Ethan Lin[:ethlin]]

(In reply to Sotaro Ikeda [:sotaro] from comment #25)
> (In reply to Sotaro Ikeda [:sotaro] from comment #24)
> > (In reply to Sotaro Ikeda [:sotaro] from comment #14)
> > > The patches seem to cause the regression. During scrolling
> > > http://www.yahoo.co.jp/, I saw a bit of rendering problem.
> > 
> > I still see some small artifacts during scrolling http://www.yahoo.co.jp/ on
> > my linux pc with patches. It might be related to RotatedBuffer.
> > 
> > :ethlin, don't you see the problem on your side?
> 
> I scrolled the page with mouse.

Oh..I see the problem now. I'll fix the problem first.

Comment 27

[Ethan Lin[:ethlin]]

(In reply to Ethan Lin[:ethlin] from comment #26)
> (In reply to Sotaro Ikeda [:sotaro] from comment #25)
> > (In reply to Sotaro Ikeda [:sotaro] from comment #24)
> > > (In reply to Sotaro Ikeda [:sotaro] from comment #14)
> > > > The patches seem to cause the regression. During scrolling
> > > > http://www.yahoo.co.jp/, I saw a bit of rendering problem.
> > > 
> > > I still see some small artifacts during scrolling http://www.yahoo.co.jp/ on
> > > my linux pc with patches. It might be related to RotatedBuffer.
> > > 
> > > :ethlin, don't you see the problem on your side?
> > 
> > I scrolled the page with mouse.
> 
> Oh..I see the problem now. I'll fix the problem first.

Well, I should consider BufferRect for ContentHost. I'll send to try server first to see the result.

Comment 28

[Ethan Lin[:ethlin]]

Attachment: Part2. fix bound/format problem after using ContentClient.

I didn't count BufferRect offset for visible region. I add the offset in this patch. The try result are all unexpected-pass now.
I also checked the scrolling action on www.yahoo.co.jp, and it looked also good.

Comment 29

[Ethan Lin[:ethlin]]

Attachment: Part4. Remove some fails-if

Based on the try server result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=83fd679fbdc7&selectedJob=67877736, I remove some failed-if flag.

Comment 30

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8825652 [details] [diff] [review]
Part2. fix bound/format problem after using ContentClient.

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

::: gfx/layers/wr/WebRenderLayerManager.h
@@ +175,5 @@
>  
>    DrawPaintedLayerCallback GetPaintedLayerCallback() const
>    { return mPaintedLayerCallback; }
>  
> +  virtual bool IsCompositingCheap() override { return false; }

Can you explain why is it necessary? It could cause the bad effect to video playback.

Comment 31

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8825652 [details] [diff] [review]
Part2. fix bound/format problem after using ContentClient.

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

::: gfx/layers/wr/WebRenderBridgeParent.cpp
@@ +307,5 @@
> +          visibleRegion.MoveBy(-offset);
> +          visibleRegion.AndWith(IntRect(IntPoint(0,0), dSurf->GetSize()));
> +          visibleRect = visibleRegion.GetBounds().ToUnknownRect();
> +
> +          // XXX Remove it when we can put subimage in WebRender.

Can you create a bug for it?

Comment 32

[Ethan Lin[:ethlin]]

(In reply to Sotaro Ikeda [:sotaro] from comment #30)
> Comment on attachment 8825652 [details] [diff] [review]
> Part2. fix bound/format problem after using ContentClient.
> 
> Review of attachment 8825652 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/wr/WebRenderLayerManager.h
> @@ +175,5 @@
> >  
> >    DrawPaintedLayerCallback GetPaintedLayerCallback() const
> >    { return mPaintedLayerCallback; }
> >  
> > +  virtual bool IsCompositingCheap() override { return false; }
> 
> Can you explain why is it necessary? It could cause the bad effect to video
> playback.

Opps...I should remove it! Sorry for that.

Comment 33

[Ethan Lin[:ethlin]]

I'll re-send try to make sure the reftest flags.

Comment 34

[Ethan Lin[:ethlin]]

(In reply to Sotaro Ikeda [:sotaro] from comment #31)
> Comment on attachment 8825652 [details] [diff] [review]
> Part2. fix bound/format problem after using ContentClient.
> 
> Review of attachment 8825652 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/wr/WebRenderBridgeParent.cpp
> @@ +307,5 @@
> > +          visibleRegion.MoveBy(-offset);
> > +          visibleRegion.AndWith(IntRect(IntPoint(0,0), dSurf->GetSize()));
> > +          visibleRect = visibleRegion.GetBounds().ToUnknownRect();
> > +
> > +          // XXX Remove it when we can put subimage in WebRender.
> 
> Can you create a bug for it?

Okay! I just created Bug 1330261 for it.

Comment 35

[Ethan Lin[:ethlin]]

Comment on attachment 8825653 [details] [diff] [review]
Part4. Remove some fails-if

># HG changeset patch
># User Ethan Lin <[e-mail]>
>
>Bug 1322504 - Part4. Remove some fails-if for reftests after using ContentClient in WebRenderPaintedLayer. r=sotaro
>
>---
> dom/html/reftests/reftest.list                     |  2 +-
> layout/reftests/bugs/reftest.list                  |  4 ++--
> layout/reftests/ogg-video/reftest.list             |  2 +-
> layout/reftests/svg/reftest.list                   |  4 ++--
> .../w3c-css/submitted/masking/reftest.list         |  2 +-
> layout/reftests/webm-video/reftest.list            | 22 +++++++++++-----------
> 6 files changed, 18 insertions(+), 18 deletions(-)
>
>diff --git a/dom/html/reftests/reftest.list b/dom/html/reftests/reftest.list
>index 2daf55e..155b118 100644
>--- a/dom/html/reftests/reftest.list
>+++ b/dom/html/reftests/reftest.list
>@@ -57,9 +57,9 @@ fuzzy(3,640) == bug917595-exif-rotated.jpg bug917595-pixel-rotated.jpg # bug 106
> == table-border-2.html table-border-2-ref.html
> != table-border-2.html table-border-2-notref.html
> 
> # Test imageset is using permissions.default.image
> pref(permissions.default.image,1) HTTP == bug1196784-with-srcset.html bug1196784-no-srcset.html
> pref(permissions.default.image,2) HTTP == bug1196784-with-srcset.html bug1196784-no-srcset.html
> 
> # Test video with rotation information can be rotated.
>-fails-if(webrender) == bug1228601-video-rotation-90.html bug1228601-video-rotated-ref.html
>+== bug1228601-video-rotation-90.html bug1228601-video-rotated-ref.html
>diff --git a/layout/reftests/bugs/reftest.list b/layout/reftests/bugs/reftest.list
>index 1bc8dc7..42cdfb5 100644
>--- a/layout/reftests/bugs/reftest.list
>+++ b/layout/reftests/bugs/reftest.list
>@@ -1667,17 +1667,17 @@ fuzzy-if(Android,8,500) == 637852-1.html 637852-1-ref.html
> fuzzy-if(Android,8,500) == 637852-2.html 637852-2-ref.html
> fuzzy-if(Android,8,500) == 637852-3.html 637852-3-ref.html
> == 641770-1.html 641770-1-ref.html
> == 641856-1.html 641856-1-ref.html
> == 645491-1.html 645491-1-ref.html
> == 645647-1.html 645647-1-ref.html
> == 645647-2.html 645647-2-ref.html
> == 645768-1.html 645768-1-ref.html
>-fails-if(layersGPUAccelerated&&cocoaWidget) fuzzy-if(!layersGPUAccelerated,41,260) fuzzy-if(skiaContent,57,11000) fails-if(webrender) == 650228-1.html 650228-1-ref.html # Quartz alpha blending doesn't match GL alpha blending
>+fails-if(layersGPUAccelerated&&cocoaWidget) fuzzy-if(!layersGPUAccelerated,41,260) fuzzy-if(skiaContent,57,11000) random-if(webrender) == 650228-1.html 650228-1-ref.html # Quartz alpha blending doesn't match GL alpha blending
> needs-focus == 652301-1a.html 652301-1-ref.html
> needs-focus == 652301-1b.html 652301-1-ref.html
> fuzzy-if(skiaContent,2,5) == 652775-1.html 652775-1-ref.html
> HTTP == 652991-1a.html 652991-1-ref.html
> HTTP == 652991-1b.html 652991-1-ref.html
> HTTP == 652991-2.html 652991-2-ref.html
> HTTP == 652991-3.html 652991-3-ref.html
> HTTP == 652991-4.html 652991-4-ref.html
>@@ -1797,17 +1797,17 @@ fuzzy-if(Android,8,400) fails-if(webrender) == 906199-1.html 906199-1-ref.html
> fuzzy-if(cocoaWidget,1,40) == 928607-1.html 928607-1-ref.html
> fuzzy-if(skiaContent,1,3) == 931464-1.html 931464-1-ref.html
> == 931853.html 931853-ref.html
> == 931853-quirks.html 931853-quirks-ref.html
> fuzzy-if(OSX==1006,2,30) == 933264-1.html 933264-1-ref.html
> == 936670-1.svg 936670-1-ref.svg
> == 941940-1.html 941940-1-ref.html
> fails == 942017.html 942017-ref.html # bug 942017
>-fuzzy-if(Android,1,1) fuzzy-if(skiaContent,1,160000) fails-if(webrender) == 942672-1.html 942672-1-ref.html
>+fuzzy-if(Android,1,1) fuzzy-if(skiaContent,1,160000) == 942672-1.html 942672-1-ref.html
> == 953334-win32-clipping.html 953334-win32-clipping-ref.html
> fuzzy-if(skiaContent,1,5) == 956513-1.svg 956513-1-ref.svg
> == 944291-1.html 944291-1-ref.html
> == 950436-1.html 950436-1-ref.html
> == 957770-1.svg 957770-1-ref.svg
> == 960277-1.html 960277-1-ref.html
> fuzzy-if(skiaContent,1,80) == 961887-1.html 961887-1-ref.html
> fails-if(webrender) == 961887-2.html 961887-2-ref.html
>diff --git a/layout/reftests/ogg-video/reftest.list b/layout/reftests/ogg-video/reftest.list
>index 87a84ac..cbe110f 100644
>--- a/layout/reftests/ogg-video/reftest.list
>+++ b/layout/reftests/ogg-video/reftest.list
>@@ -14,17 +14,17 @@ fails-if(Android) HTTP(..) == canvas-1b.xhtml basic-1-ref.html
> == empty-1a.html empty-1-ref.html
> == empty-1b.html empty-1-ref.html
> #these is skipped because we hang on the htmlparser tests when this is ran
> random skip-if(Android) HTTP(..) == object-aspect-ratio-1a.xhtml aspect-ratio-1-ref.html
> random skip-if(Android) HTTP(..) == object-aspect-ratio-1b.xhtml aspect-ratio-1-ref.html
> skip-if(Android) HTTP(..) == offset-1.xhtml offset-1-ref.html
> random skip-if(Android) HTTP(..) == object-aspect-ratio-2a.xhtml aspect-ratio-2-ref.html
> random skip-if(Android) HTTP(..) == object-aspect-ratio-2b.xhtml aspect-ratio-2-ref.html
>-skip-if(Android) fuzzy-if(winWidget,1,56000) fuzzy-if(cocoaWidget,1,56000) fails-if(webrender&&!browserIsRemote) HTTP(..) == zoomed-1.xhtml zoomed-1-ref.html # bug 778995 for fuzzy
>+skip-if(Android) fuzzy-if(winWidget,1,56000) fuzzy-if(cocoaWidget,1,56000) HTTP(..) == zoomed-1.xhtml zoomed-1-ref.html # bug 778995 for fuzzy
> skip-if(Android) == poster-1.html poster-ref-blue250x200.html
> skip-if(Android) == poster-2.html poster-ref-black140x100.html
> skip-if(Android) == poster-3.html poster-ref-black140x100.html
> skip-if(Android) == poster-4.html poster-ref-black140x100.html
> skip-if(Android) == poster-5.html poster-ref-black140x100.html
> skip-if(Android) == poster-6.html poster-ref-black140x100.html
> skip-if(Android) == poster-7.html poster-ref-red140x100.html
> skip-if(Android) == poster-8.html poster-ref-black140x100.html
>diff --git a/layout/reftests/svg/reftest.list b/layout/reftests/svg/reftest.list
>index 71098bb..0728654 100644
>--- a/layout/reftests/svg/reftest.list
>+++ b/layout/reftests/svg/reftest.list
>@@ -458,10 +458,10 @@ default-preferences
> 
> == clipPath-and-mask-on-outflowElement-01a.html clipPath-on-outflowElement-01-ref.html
> == clipPath-and-mask-on-outflowElement-01b.html clipPath-on-outflowElement-01-ref.html
> 
> != nested-mask-mode.svg about:blank
> == mask-empty-size.svg about:blank
> 
> == paint-on-maskLayer-1a.html paint-on-maskLayer-1-ref.html
>-fails-if(webrender) == paint-on-maskLayer-1b.html paint-on-maskLayer-1-ref.html
>-fails-if(webrender) == paint-on-maskLayer-1c.html paint-on-maskLayer-1-ref.html
>+== paint-on-maskLayer-1b.html paint-on-maskLayer-1-ref.html
>+== paint-on-maskLayer-1c.html paint-on-maskLayer-1-ref.html
>diff --git a/layout/reftests/w3c-css/submitted/masking/reftest.list b/layout/reftests/w3c-css/submitted/masking/reftest.list
>index a35318d..66cdb8f 100644
>--- a/layout/reftests/w3c-css/submitted/masking/reftest.list
>+++ b/layout/reftests/w3c-css/submitted/masking/reftest.list
>@@ -107,11 +107,11 @@ fuzzy-if(winWidget,9,98) == clip-path-geometryBox-2.html clip-path-geometryBox-2
> 
> default-preferences
> 
> # mask with opacity test cases
> fuzzy(1,5000) == mask-opacity-1a.html mask-opacity-1-ref.html
> fuzzy(1,5000) == mask-opacity-1b.html mask-opacity-1-ref.html
> fuzzy(1,5000) == mask-opacity-1c.html mask-opacity-1-ref.html
> fuzzy(1,5000) == mask-opacity-1d.html mask-opacity-1-ref.html
>-fuzzy(1,5000) fails-if(webrender) == mask-opacity-1e.html mask-opacity-1-ref.html
>+fuzzy(1,5000) == mask-opacity-1e.html mask-opacity-1-ref.html
> 
> fails-if(webrender) == clip-path-mix-blend-mode-1.html clip-path-mix-blend-mode-1-ref.html
>diff --git a/layout/reftests/webm-video/reftest.list b/layout/reftests/webm-video/reftest.list
>index aacddfc..73f9fde 100644
>--- a/layout/reftests/webm-video/reftest.list
>+++ b/layout/reftests/webm-video/reftest.list
>@@ -13,17 +13,17 @@ fails-if(Android) HTTP(..) == canvas-1b.xhtml basic-1-ref.html
> == empty-1a.html empty-1-ref.html
> == empty-1b.html empty-1-ref.html
> #these is skipped because we hang on the htmlparser tests when this is ran
> random skip-if(Android) HTTP(..) == object-aspect-ratio-1a.xhtml aspect-ratio-1-ref.html
> random skip-if(Android) HTTP(..) == object-aspect-ratio-1b.xhtml aspect-ratio-1-ref.html
> skip-if(Android) HTTP(..) == offset-1.xhtml offset-1-ref.html
> random skip-if(Android) HTTP(..) == object-aspect-ratio-2a.xhtml aspect-ratio-2-ref.html
> random skip-if(Android) HTTP(..) == object-aspect-ratio-2b.xhtml aspect-ratio-2-ref.html
>-skip-if(Android) fuzzy-if(winWidget,1,56000) fuzzy-if(cocoaWidget,1,56000) fails-if(webrender&&!browserIsRemote) HTTP(..) == zoomed-1.xhtml zoomed-1-ref.html # bug 778995 for fuzzy
>+skip-if(Android) fuzzy-if(winWidget,1,56000) fuzzy-if(cocoaWidget,1,56000) HTTP(..) == zoomed-1.xhtml zoomed-1-ref.html # bug 778995 for fuzzy
> skip-if(Android) == poster-1.html poster-ref-blue250x200.html
> skip-if(Android) == poster-2.html poster-ref-black140x100.html
> skip-if(Android) == poster-3.html poster-ref-black140x100.html
> skip-if(Android) == poster-4.html poster-ref-black140x100.html
> skip-if(Android) == poster-5.html poster-ref-black140x100.html
> skip-if(Android) == poster-6.html poster-ref-black140x100.html
> skip-if(Android) == poster-7.html poster-ref-red140x100.html
> skip-if(Android) == poster-8.html poster-ref-black140x100.html
>@@ -35,20 +35,20 @@ skip-if(Android) == poster-15.html poster-ref-green70x30.html
> random-if(winWidget) random-if(cocoaWidget) skip-if(Android) == bug686957.html bug686957-ref.html # bug 922951 for OS X
> 
> # Tests for <video src> with 'object-fit' & 'object-position':
> # These tests should be very similar to tests in our w3c-css/submitted/images3
> # reftest directory. They live here because they use WebM video (VP9), and it
> # wouldn't be fair of us to make a W3C testsuite implicitly depend on any
> # particular (non-spec-mandated) video codec.
> default-preferences test-pref(layout.css.object-fit-and-position.enabled,true) test-pref(gfx.ycbcr.accurate-conversion,true)
>-fails-if(layersGPUAccelerated) skip-if(Android) == object-fit-contain-webm-001.html object-fit-contain-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>-fails-if(layersGPUAccelerated) skip-if(Android) == object-fit-contain-webm-002.html object-fit-contain-webm-002-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>-fails-if(layersGPUAccelerated) skip-if(Android) == object-fit-cover-webm-001.html object-fit-cover-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>-fails-if(layersGPUAccelerated) skip-if(Android) == object-fit-cover-webm-002.html object-fit-cover-webm-002-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>-fails-if(layersGPUAccelerated) skip-if(Android) == object-fit-fill-webm-001.html object-fit-fill-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>-fails-if(layersGPUAccelerated) skip-if(Android) == object-fit-fill-webm-002.html object-fit-fill-webm-002-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>-fails-if(layersGPUAccelerated) skip-if(Android) == object-fit-none-webm-001.html object-fit-none-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>-fails-if(layersGPUAccelerated) skip-if(Android) == object-fit-none-webm-002.html object-fit-none-webm-002-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>-fails-if(layersGPUAccelerated) skip-if(Android) == object-fit-scale-down-webm-001.html object-fit-scale-down-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>-fails-if(layersGPUAccelerated) skip-if(Android) == object-fit-scale-down-webm-002.html object-fit-scale-down-webm-002-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>+fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-contain-webm-001.html object-fit-contain-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>+fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-contain-webm-002.html object-fit-contain-webm-002-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>+fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-cover-webm-001.html object-fit-cover-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>+fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-cover-webm-002.html object-fit-cover-webm-002-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>+fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-fill-webm-001.html object-fit-fill-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>+fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-fill-webm-002.html object-fit-fill-webm-002-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>+fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-none-webm-001.html object-fit-none-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>+fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-none-webm-002.html object-fit-none-webm-002-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>+fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-scale-down-webm-001.html object-fit-scale-down-webm-001-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>+fails-if(layersGPUAccelerated) skip-if(Android) random-if(webrender) == object-fit-scale-down-webm-002.html object-fit-scale-down-webm-002-ref.html # Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
> fails-if(layersGPUAccelerated) skip-if(Android) fails == object-position-webm-001.html object-position-webm-001-ref.html # Bug 1098417 for across-the-board failure, Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
> fails-if(layersGPUAccelerated) skip-if(Android) fails == object-position-webm-002.html object-position-webm-002-ref.html # Bug 1098417 for across-the-board failure, Bug 1083516 for layersGPUAccelerated failures, Bug 1084564 for Android failures
>

Comment 36

[Ethan Lin[:ethlin]]

Attachment: Part1. Use ContentClient in WebRenderPaintedLayer. r=sotaro

Rebase the patch.

Comment 37

[Ethan Lin[:ethlin]]

Attachment: Part2. fix bound/format problem after using ContentClient.

Address sotaro's comment. I removed the override of IsCompositingCheap in WebRenderLayerManager.

Comment 38

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8825715 [details] [diff] [review]
Part2. fix bound/format problem after using ContentClient.

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

::: gfx/layers/ipc/WebRenderMessages.ipdlh
@@ +63,5 @@
>    WRImageKey key;
>  };
>  
>  struct OpDPPushExternalImageId {
> +  LayerIntRegion visibleRegion;

name of visibleRegion seems a bit confusing. It actually seems to mean valid buffer region.

Comment 39

[Ethan Lin[:ethlin]]

try server result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8385b840f7fc76521170b4a5b7076e88a1de0ced

There is only one unexpected-pass. I'll remove the 'fails-if' flag.

Comment 40

[Ethan Lin[:ethlin]]

(In reply to Sotaro Ikeda [:sotaro] from comment #38)
> Comment on attachment 8825715 [details] [diff] [review]
> Part2. fix bound/format problem after using ContentClient.
> 
> Review of attachment 8825715 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/ipc/WebRenderMessages.ipdlh
> @@ +63,5 @@
> >    WRImageKey key;
> >  };
> >  
> >  struct OpDPPushExternalImageId {
> > +  LayerIntRegion visibleRegion;
> 
> name of visibleRegion seems a bit confusing. It actually seems to mean valid
> buffer region.

Okay, I'll rename to validBufferRegion.

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/4c9d48bb926c
Part1. Use ContentClient in WebRenderPaintedLayer. r=sotaro
https://hg.mozilla.org/projects/graphics/rev/693ed93b56db
Part2. Fix bound/format problem after using ContentClient. r=sotaro
https://hg.mozilla.org/projects/graphics/rev/81e3e207072b
Part3. Disable RotationBuffer when enabling WR. r=sotaro
https://hg.mozilla.org/projects/graphics/rev/11b1c9cffb0a
Part4. Remove a fails-if flag after using ContentClient in WebRenderPaintedLayer. r=gfx?

Comment 42

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

These patches may have introduced or exposed some nondeterministic behaviour. There are a couple more "unexpected pass" tests showing up, but intermittently. See https://treeherder.mozilla.org/#/jobs?repo=graphics&revision=11b1c9cffb0ac0b13c35e96f92238ddee06370f8&filter-searchStr=quantum%20reftest-e10s-4&group_state=expanded

Comment 43

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

I ended up backing this out because of the intermittent failure in https://hg.mozilla.org/projects/graphics/rev/571286200177ae7ddfa1893c6b42853b60f2e81e. I really hate to do this but I think in the long run it's better to investigate this now rather than just marking it random-if. There might be other tests that are affected and will only be discovered later and cause much frustration to other people.

Comment 44

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

(In reply to Ethan Lin[:ethlin] from comment #39)
> try server result:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=8385b840f7fc76521170b4a5b7076e88a1de0ced
> 
> There is only one unexpected-pass. I'll remove the 'fails-if' flag.

I retriggered these a bunch, and the other failures are showing up in this try push as well. Seems to be restricted to debug-e10s reftests, both accelerated and unaccelerated.

Comment 45

[Ethan Lin[:ethlin]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #42)
> These patches may have introduced or exposed some nondeterministic
> behaviour. There are a couple more "unexpected pass" tests showing up, but
> intermittently. See
> https://treeherder.mozilla.org/#/
> jobs?repo=graphics&revision=11b1c9cffb0ac0b13c35e96f92238ddee06370f8&filter-
> searchStr=quantum%20reftest-e10s-4&group_state=expanded

Looks like there are two tests become intermittent unexpected-pass:
layout/reftests/bugs/581317-1.html , layout/reftests/bugs/650228-1.html. I guess it's because the timing of getting snapshot. Some active layer changes to inactive one. And this patch may fix either active or inactive case.

Comment 46

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

The Ru4 jobs [1] also has layout/reftests/bugs/728983-1.html failing a few times - from the reftest analyzer it looks like the shade of green is slightly different. I don't know how that's happening and if that's a regression from your patch or not. I don't think I've seen it any previous push.

[1] https://treeherder.mozilla.org/#/jobs?repo=graphics&revision=11b1c9cffb0ac0b13c35e96f92238ddee06370f8&filter-searchStr=quantum%20reftest%20e10s&group_state=expanded&selectedJob=68484937

Comment 47

[Ethan Lin[:ethlin]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #46)
> The Ru4 jobs [1] also has layout/reftests/bugs/728983-1.html failing a few
> times - from the reftest analyzer it looks like the shade of green is
> slightly different. I don't know how that's happening and if that's a
> regression from your patch or not. I don't think I've seen it any previous
> push.
> 
> [1]
> https://treeherder.mozilla.org/#/
> jobs?repo=graphics&revision=11b1c9cffb0ac0b13c35e96f92238ddee06370f8&filter-
> searchStr=quantum%20reftest%20e10s&group_state=expanded&selectedJob=68484937

Right, these three tests are all about opacity changing. I think these failures happen now is because the patches in this bug change the timing. I'll keep investigating.

Comment 48

[Ethan Lin[:ethlin]]

I think I found the problem. At least for 581317-1.html, 650228-1.html, we don't handle color layer correctly, so basically the testcase will fail. But after the testcase stops changing the opacity of the color layer, the color layer will be merged to a painted layer (because it's an inactive layer). So it's all about the timing of snapshot. I think 728983-1.html should be the similar problem, just reverse.
For now color layer has two problems: position and opacity. I'll create bugs to fix color layer. kats, can I mark these three testcase to 'random-if' first? And I will remove the 'random-if' in the following bugs.

Comment 49

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

(In reply to Ethan Lin[:ethlin] from comment #48)
> For now color layer has two problems: position and opacity. I'll create bugs
> to fix color layer. kats, can I mark these three testcase to 'random-if'
> first? And I will remove the 'random-if' in the following bugs.

Ok, as long as we understand the problem I think it's ok to mark them random-if. Thanks for looking into it!

Comment 50

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/7c573aa43807
Part1. Use ContentClient in WebRenderPaintedLayer. r=sotaro
https://hg.mozilla.org/projects/graphics/rev/d6424c3c807d
Part2. Fix bound/format problem after using ContentClient. r=sotaro
https://hg.mozilla.org/projects/graphics/rev/903355a64373
Part3. Disable RotationBuffer when enabling WR. r=sotaro
https://hg.mozilla.org/projects/graphics/rev/7b90b2b7960e
Part4. Remove a fails-if flag and set some random-if after using ContentClient in WebRenderPaintedLayer. r=gfx?

Comment 51

[Ethan Lin[:ethlin]]

This is the try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a41768346e6f11c1c2887b10c947399f9127be67