810361 - Handle ColorLayers with hardware composer

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Comment 1

[Diego Wilson [:diego]]

Attachment: Handle ColorLayers with HwcComposer2D

Comment 2

[Diego Wilson [:diego]]

Attachment: Enhance display libs to support colorlayers

Comment 3

[Diego Wilson [:diego]]

Attachment: Enhance kernel display driver to support ColorLayers

Comment 4

[Diego Wilson [:diego]]

All patches attached are already provided through CAF

https://www.codeaurora.org/gitweb/quic/b2g/?p=build.git;a=commit;h=fe5f26354a965d25188a0756b99120ba83f09093

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 690213 [details] [diff] [review]
Handle ColorLayers with HwcComposer2D

>diff --git a/widget/gonk/HwcComposer2D.cpp b/widget/gonk/HwcComposer2D.cpp

>+// HWC layer flags
>+enum {
>+    HWC_COLOR_FILL = 0x8 // RGBA color-fill using copybit
>+};

We also need a new propdb value like ro.moz.hwc.have_color_layers=true
that we can use for runtime feature detection.

>@@ -319,43 +334,42 @@ HwcComposer2D::PrepareLayerList(Layer* aLayer,
>         return true;
>     }
> 
>-    buffer_handle_t handle = buffer->getNativeBuffer()->handle;
>+    buffer_handle_t handle = fillColor ? NULL : buffer->getNativeBuffer()->handle;

Nit: s/NULL/nullptr.

>+    } else {
>+        hwcLayer.flags |= HWC_COLOR_FILL;
>+        ColorLayer* colorLayer = static_cast<ColorLayer*>(layerGL->GetLayer());
>+        hwcLayer.transform = colorLayer->GetColor().Packed();

Cute hack :).  Please document this above where we define the
COLOR_FILL flag.

r=me with that

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 690214 [details] [diff] [review]
Enhance display libs to support colorlayers

Looks good to me but this code is a bit out of my front yard :).

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 690215 [details] [diff] [review]
Enhance kernel display driver to support ColorLayers

A bit further still outside my front yard, but again looks ok to me.

/me jealous he doesn't get to hack kernel anymore ;)

Comment 8

[Diego Wilson [:diego]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #5)
> Comment on attachment 690213 [details] [diff] [review]
> Handle ColorLayers with HwcComposer2D
> 
> >diff --git a/widget/gonk/HwcComposer2D.cpp b/widget/gonk/HwcComposer2D.cpp
> 
> >+// HWC layer flags
> >+enum {
> >+    HWC_COLOR_FILL = 0x8 // RGBA color-fill using copybit
> >+};
> 
> We also need a new propdb value like ro.moz.hwc.have_color_layers=true
> that we can use for runtime feature detection.

Agreed. Working on it

> 
> >@@ -319,43 +334,42 @@ HwcComposer2D::PrepareLayerList(Layer* aLayer,
> >         return true;
> >     }
> > 
> >-    buffer_handle_t handle = buffer->getNativeBuffer()->handle;
> >+    buffer_handle_t handle = fillColor ? NULL : buffer->getNativeBuffer()->handle;
> 
> Nit: s/NULL/nullptr.
> 
> >+    } else {
> >+        hwcLayer.flags |= HWC_COLOR_FILL;
> >+        ColorLayer* colorLayer = static_cast<ColorLayer*>(layerGL->GetLayer());
> >+        hwcLayer.transform = colorLayer->GetColor().Packed();
> 
> Cute hack :).  Please document this above where we define the
> COLOR_FILL flag.
> 
> r=me with that

I'll share an updated patch ASAP

Comment 9

[Diego Wilson [:diego]]

Attachment: Handle ColorLayers with HwcComposer2D

Addresses all review comments on the previous patch

Comment 10

[Andrew Overholt [:overholt]]

Do we need to block on this or could we just uplift it?  What happens if we don't take it?

Comment 11

[Michael Vines [:m1] [:evilmachines]]

We've landed this patch on CAF already.  So the goal of this bug is to get this patch landed in Gecko proper so that we don't essentially have a minor Gecko fork for v1 commercial.   I'm neutral on this, don't like the fork but it's not the end of the world either IMO.

Comment 12

[Andrew Overholt [:overholt]]

(In reply to Michael Vines [:m1] from comment #11)
> We've landed this patch on CAF already.

That sounds like it's important enough to block on.

Comment 13

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Can we directly land the patch here or has the caf version accreted some differences?

Comment 14

[Diego Wilson [:diego]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #13)
> Can we directly land the patch here or has the caf version accreted some
> differences?

The r+'ed patch in this bug is the latest and greatest. If you can help me land it that'd be great

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Will land tomorrow.

Comment 16

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: For landing

Comment 17

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

https://hg.mozilla.org/integration/mozilla-inbound/rev/632213b8cbc6
https://hg.mozilla.org/releases/mozilla-b2g18/rev/07574bef1b04

Comment 18

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

This appears to have turned the Windows B2G build red:
https://tbpl.mozilla.org/?tree=Mozilla-B2g18&jobname=b2g_mozilla-b2g18_win32_gecko%20build

Comment 19

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Er, that's probably a change in another repo (ugh!) that happened to occur at the same time.

Comment 20

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/632213b8cbc6