912373 - Support colorlayer for generic HWComposer, like MDP

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect)

Description

[Peter Chang[:pchang]]

For generic HWComposer, like MDP, it requires the graphicbuffer for composition.
But we don't have the graphicbuffer for colorlayer.


We have two possible ways to solve this problem.

a. fallback colorlayer to thebeslayer
   redundant solid color drawing cost
b. create static graphic buffer inside HWcomposer2D and send that buffer if we have color layer.
   we can create a small buffer and then ask MDP to scale up it.
   And there is another special colorlayer (page indexer) size is (80,3). I think we can have another buffer to handle it.

Comment 1

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: 0001-add-jb-hwc-support

Comment 2

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: 0002-add-jb-hwc-support

Comment 3

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: 0003-add-jb-hwc-support

Comment 4

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: [WIP] color layer support for non-color-fill device

Apply 0001~0003 patch for JB hwc device testing.

Treat the color layer as a normal layer and create a real GraphicBuffer for composition.

Comment 5

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: [WIP] color layer support for non-color-fill device v2

Base on bug 896765.
Get a real GraphicBuffer in HwcComposer2D for color layer composition.

Comment 6

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: force enable hwcomposer

Comment 7

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: [WIP] color layer support for non-color-fill device v2.1

consider buffer stride when fill graphic buffer

Comment 8

[u459114]

Comment on attachment 805774 [details] [diff] [review]
[WIP] color layer support for non-color-fill device v2.1

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

::: widget/gonk/HwcComposer2D.cpp
@@ +416,5 @@
> +            uint32_t buffer_height=hwcLayer.displayFrame.bottom-hwcLayer.displayFrame.top;
> +
> +            printf_stderr("hwc:acqure color buffer (%u,%u)",buffer_width,buffer_height);
> +
> +            GraphicBuffer *color_buffer=ColorBufferFactory::GetSingleton()->GetColorBuffer(buffer_width,buffer_height,colorLayer->GetColor().Packed());

With ColorBufferFactory::Instance()->GenColorBuffer(hwcLayer)
You can hide all this implementation inside ColorBufferFactory

Andy, why not just fill color in this function and remove ColorBufferFactory::FillAllColorBuffer function?

@@ +542,5 @@
>  {
> +    char propValue[PROPERTY_VALUE_MAX];
> +    property_get("debug.colorfill.use_buffer", propValue, "1");
> +    mColorFillWithGraphicBuffer = (atoi(propValue) == 1) ? true : false;
> +

Don't read this value in TryRender.

@@ +556,5 @@
>  
>      // XXX: The clear() below means all rect vectors will be have to be
>      // reallocated. We may want to avoid this if possible
>      mVisibleRegions.clear();
> +    

trailer space

@@ +567,5 @@
>                            gfxMatrix(),
>                            aGLWorldTransform))
>      {
>          LOGD("Render aborted. Nothing was drawn to the screen");
> +	    

trailer space

::: widget/gonk/HwcUtils.cpp
@@ +212,5 @@
> +    uint32_t buffer_stride=mBuffer->getStride();
> +    void *locked_buffer=nullptr;
> +    uint8_t *byte_buffer=nullptr;
> +    uint32_t *pixel_buffer=nullptr;
> +

Declare and define local data at the place that you are going to use it

@@ +224,5 @@
> +    }
> +
> +    //lock buffer
> +    android::Rect rect(fill_width,fill_height);
> +    if((buffer_status=mBuffer->lock(android::GraphicBuffer::USAGE_SW_WRITE_OFTEN|android::GraphicBuffer::USAGE_SW_READ_NEVER,&locked_buffer))!=0){

if(mBuffer->lock(android::GraphicBuffer::USAGE_SW_WRITE_OFTEN|android::GraphicBuffer::USAGE_SW_READ_NEVER,&locked_buffer)!=0)

@@ +229,5 @@
> +      return false;
> +    }
> +
> +    //fill buffer
> +    byte_buffer=reinterpret_cast<uint8_t*>(locked_buffer);

uint8_t *byte_buffer=reinterpret_cast<uint8_t*>(locked_buffer);

@@ +231,5 @@
> +
> +    //fill buffer
> +    byte_buffer=reinterpret_cast<uint8_t*>(locked_buffer);
> +    for(uint32_t i=0;i<fill_height;++i){
> +      pixel_buffer=reinterpret_cast<uint32_t*>(byte_buffer+buffer_stride*i);

uint32_t *pixel_buffer = pixel_buffer=reinterpret_cast<uint32_t*>(byte_buffer+buffer_stride*i)

@@ +241,5 @@
> +    //unlock buffer
> +    if((buffer_status=mBuffer->unlock())!=0){
> +      return false;
> +    }
> +

Is there auto lock template for GraphicBuffer? If there is, don't manually call lock and unlock

@@ +252,5 @@
> +
> +  return true;
> +}
> +
> +bool ColorBuffer::BufferSizeFit(uint32_t width,uint32_t height)

Rename to CompareSize?

@@ +272,5 @@
> +  mPendingFillWidth=width;
> +  mPendingFillHeight=height;
> +}
> +
> +bool ColorBuffer::NeedFill(void)

bool IsNeedFill() const

@@ +276,5 @@
> +bool ColorBuffer::NeedFill(void)
> +{
> +  if(mPendingFillWidth==0 || mPendingFillHeight==0){
> +    return false;
> +  }

This statement should be an assertion.

@@ +298,5 @@
> +}
> +
> +
> +static StaticRefPtr<ColorBufferFactory> sColorBufferFactoryInstance;
> +static Mutex sColorBufferFactoryMutex("ColorBufferFactory mutex");

Mutex can be a object data member instead of static object. Since ColodBufferFactory itself has already a singleton object

::: widget/gonk/HwcUtils.h
@@ +119,5 @@
>  };
>  
> +
> +class ColorBuffer;
> +class ColorBufferFactory;

class ColorBufferFactory;
Remove this line

@@ +131,5 @@
> +    FILL_ALL,
> +    FILL_PARTIAL,
> +  };
> +
> +  static ColorBufferFactory* GetSingleton(void);

Rename to Instance()

@@ +134,5 @@
> +
> +  static ColorBufferFactory* GetSingleton(void);
> +
> +  ColorBufferFactory();
> +  virtual ~ColorBufferFactory();

Make constructor and destructor private

@@ +136,5 @@
> +
> +  ColorBufferFactory();
> +  virtual ~ColorBufferFactory();
> +
> +  void Reset(void);

Comment need.

Comment 9

[Jerry Shih[:jerry] (UTC+8) (inactive)]

query colorlayer support:bug 901978

Comment 10

[BMO Automation]

Firefox OS is not being worked on