1205713 - Merge ImageHostOverlay/ImageClientOverlay to ImageHost/ImageClient

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Sotaro Ikeda [:sotaro]]

ImageHostOverlay/ImageClientOverlay try to support only OverlayImage(sideband on gonk). It causes video rendering uses totally different code path between TextureClient based video and OverlayImage(sideband on gonk). It is not good. They should share code as much as possible.

From this point of view ImageHostOverlay/ImageClientOverlay should be merged to ImageHost/ImageClient. If it is done, we could test some code without using actual TV stream(sideband on gonk) on specific hardware.

On android, sideband stream and normal gralloc buffer rendering is handled by one android Layer. If sideband handle exists, sideband stream is used for rendering. It it does not exist, normal rendering is done.

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: patch - - Merge ImageHostOverlay/ImageClientOverlay to ImageHost/ImageClien

Comment 2

[Sotaro Ikeda [:sotaro]]

By attachment 8701722 [details] [diff] [review], ImageHost renders OverlayImage. With it, smooth transition between non OverlayImage and video image becomes possible.

OverlaySource rendering has priority than TextureHost rendering. Its is similar to android SidebandStream handling.
  http://androidxref.com/6.0.0_r1/xref/frameworks/native/services/surfaceflinger/Layer.cpp#556

Comment 3

[Nicolas Silva [:nical]]

Comment on attachment 8701722 [details] [diff] [review]
patch - - Merge ImageHostOverlay/ImageClientOverlay to ImageHost/ImageClien

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

Nice! I would rename ImageHostOverlay into something that doesn't sound too much like a compositable (OverlayRenderer?) or even merge it with ImageHost since it doesn't represent a lot of code (I guess most of it would turn into a simple function (ImageHost::CompositeOverlay maybe?). Anyway, looks very good.

::: gfx/layers/composite/ImageHost.h
@@ +147,3 @@
>  
>  /**
>   * ImageHostOverlay works with ImageClientOverlay

nit: ImageClientOverlay is gone now

@@ +154,3 @@
>  
> +public:
> +  NS_INLINE_DECL_REFCOUNTING(CompositableHost)

I think you meant NS_INLINE_DECL_REFCOUNTING(ImageHostOverlay) ?

Comment 4

[Sotaro Ikeda [:sotaro]]

(In reply to Nicolas Silva [:nical] from comment #3)
> Comment on attachment 8701722 [details] [diff] [review]
> patch - - Merge ImageHostOverlay/ImageClientOverlay to ImageHost/ImageClien
> 
> Review of attachment 8701722 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nice! I would rename ImageHostOverlay into something that doesn't sound too
> much like a compositable (OverlayRenderer?) or even merge it with ImageHost
> since it doesn't represent a lot of code (I guess most of it would turn into
> a simple function (ImageHost::CompositeOverlay maybe?). Anyway, looks very
> good.
> 

At first, I thought to merged to ImageHost. But for the time being, just keep it seems simple.

Comment 5

[Sotaro Ikeda [:sotaro]]

(In reply to Nicolas Silva [:nical] from comment #3)
> Comment on attachment 8701722 [details] [diff] [review]
> 
> ::: gfx/layers/composite/ImageHost.h
> @@ +147,3 @@
> >  
> >  /**
> >   * ImageHostOverlay works with ImageClientOverlay
> 
> nit: ImageClientOverlay is gone now

I'll update the comment in a next patch.

> 
> @@ +154,3 @@
> >  
> > +public:
> > +  NS_INLINE_DECL_REFCOUNTING(CompositableHost)
> 
> I think you meant NS_INLINE_DECL_REFCOUNTING(ImageHostOverlay) ?

Yes, I'll update it in a next patch.

Comment 6

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Merge ImageHostOverlay/ImageClientOverlay to ImageHost/ImageClient

Comment 7

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Merge ImageHostOverlay/ImageClientOverlay to ImageHost/ImageClient

correct patch.

Comment 8

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Merge ImageHostOverlay/ImageClientOverlay to ImageHost/ImageClient

Comment 9

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Merge ImageHostOverlay/ImageClientOverlay to ImageHost/ImageClient

Comment 10

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Merge ImageHostOverlay/ImageClientOverlay to ImageHost/ImageClient

Comment 11

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=41f446ad4896
tryserver seems unstable.

Comment 12

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Merge ImageHostOverlay/ImageClientOverlay to ImageHost/ImageClient

Fix gonk debug build failure.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad889ee5ed7e

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ad889ee5ed7e