If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Support colorlayer for generic HWComposer, like MDP

NEW
Assigned to

Status

Firefox OS
General
4 years ago
4 years ago

People

(Reporter: pchang, Assigned: jerry)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

4 years ago
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.
(Reporter)

Updated

4 years ago
Blocks: 900827, 901978

Updated

4 years ago
Assignee: nobody → hshih
(Assignee)

Comment 1

4 years ago
Created attachment 804439 [details] [diff] [review]
0001-add-jb-hwc-support
(Assignee)

Comment 2

4 years ago
Created attachment 804440 [details] [diff] [review]
0002-add-jb-hwc-support
(Assignee)

Comment 3

4 years ago
Created attachment 804442 [details] [diff] [review]
0003-add-jb-hwc-support
(Assignee)

Comment 4

4 years ago
Created attachment 804444 [details] [diff] [review]
[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.
(Assignee)

Comment 5

4 years ago
Created attachment 805182 [details] [diff] [review]
[WIP] color layer support for non-color-fill device v2

Base on bug 896765.
Get a real GraphicBuffer in HwcComposer2D for color layer composition.
Attachment #804439 - Attachment is obsolete: true
Attachment #804440 - Attachment is obsolete: true
Attachment #804442 - Attachment is obsolete: true
Attachment #804444 - Attachment is obsolete: true
(Assignee)

Comment 6

4 years ago
Created attachment 805185 [details] [diff] [review]
force enable hwcomposer
(Assignee)

Comment 7

4 years ago
Created attachment 805774 [details] [diff] [review]
[WIP] color layer support for non-color-fill device v2.1

consider buffer stride when fill graphic buffer
Attachment #805182 - Attachment is obsolete: true
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.
(Assignee)

Comment 9

4 years ago
query colorlayer support:bug 901978

Updated

4 years ago
No longer blocks: 900827
You need to log in before you can comment on or make changes to this bug.