Closed Bug 810361 Opened 12 years ago Closed 12 years ago

Handle ColorLayers with hardware composer

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-basecamp:+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed)

RESOLVED FIXED
B2G C4 (2jan on)
blocking-basecamp +
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 --- fixed

People

(Reporter: diego, Assigned: diego)

Details

Attachments

(4 files, 1 obsolete file)

      No description provided.
Group: qualcomm-confidential
Attachment #690213 - Flags: review?(jones.chris.g)
Attachment #690214 - Flags: review?(jones.chris.g)
Attachment #690215 - Flags: review?(jones.chris.g)
Attachment #690214 - Attachment is patch: true
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
Attachment #690213 - Flags: review?(jones.chris.g) → review+
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 :).
Attachment #690214 - Flags: review?(jones.chris.g) → feedback+
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 ;)
Attachment #690215 - Flags: review?(jones.chris.g) → feedback+
(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
Addresses all review comments on the previous patch
Attachment #690213 - Attachment is obsolete: true
Attachment #694884 - Flags: review?(jones.chris.g)
Attachment #694884 - Flags: review?(jones.chris.g) → review+
blocking-basecamp: --- → ?
Do we need to block on this or could we just uplift it?  What happens if we don't take it?
Assignee: nobody → dwilson
Flags: needinfo?(dwilson)
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.
Flags: needinfo?(dwilson)
(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.
blocking-basecamp: ? → +
Keywords: checkin-needed
Can we directly land the patch here or has the caf version accreted some differences?
Flags: needinfo?(dwilson)
(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
Flags: needinfo?(dwilson)
Er, that's probably a change in another repo (ugh!) that happened to occur at the same time.
Keywords: checkin-needed
Target Milestone: --- → B2G C4 (2jan on)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: