Page MenuHomePhabricator

ector: Fix precomp layer rendering issue when it has alpha value
ClosedPublic

Authored by jsuya on Jun 5 2019, 12:15 AM.

Details

Summary

When the precomp layer(parent layer) has alpha transparency and has more than 1 child layer
and they overlap each other if vg object just propagate the alpha to child layer
it will be applied twice in overlapped area.
Even if the child layer does not have alpha transparency, parent alpha is applied to each child.

Test Plan

N/A

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jsuya created this revision.Jun 5 2019, 12:15 AM
jsuya requested review of this revision.Jun 5 2019, 12:15 AM
jsuya planned changes to this revision.Jun 5 2019, 12:15 AM
jsuya updated this revision to Diff 22688.Jun 11 2019, 12:05 AM

update patch

jsuya updated this revision to Diff 22689.Jun 11 2019, 12:06 AM
jsuya edited the summary of this revision. (Show Details)

update comments

Hermet requested changes to this revision.Jun 11 2019, 3:21 AM

Please check comments.

src/lib/evas/canvas/efl_canvas_vg_object.c
441

We can use Ector_Buffer to pass to ector side.
See How Mask Buffer is used with Ector_Buffer
Ector_Buffer buffer;
...
ector_surface_blend_with_buffer(ector, buffer);

453

Here alpha multiplying could be performed in ector_surface_blend_with_buffer() instead.
It would be better in operation wise.
i.e) ector_surface_blend_with_buffer(ector, buffer, ba /*buffer alpha*/ );

464

how about naming this?
ector_surface_draw_image (ector, buffer /* image */, ba);

src/lib/evas/canvas/evas_vg_private.h
5

No, please don't include it.

This revision now requires changes to proceed.Jun 11 2019, 3:21 AM
jsuya updated this revision to Diff 22697.Jun 11 2019, 11:53 PM

update code
change function name and use Ector.Buffer

jsuya marked 3 inline comments as done.Jun 11 2019, 11:53 PM

Please check a comment and give me your opinion.

src/lib/evas/canvas/efl_canvas_vg_object.c
441

How about using container size instead of full ector buffer size?
Eina_Rect r;
efl_path_bounds_get(container, &r);

Hermet requested changes to this revision.Jun 12 2019, 7:30 PM
This revision now requires changes to proceed.Jun 12 2019, 7:30 PM
jsuya updated this revision to Diff 22715.Jun 13 2019, 12:32 AM

update code
-modify coordinate calculation in draw_image

jsuya added inline comments.Jun 13 2019, 12:45 AM
src/lib/evas/canvas/efl_canvas_vg_object.c
441
if (alpha < 255)
  {
     Eina_Rect r =  EINA_RECT( -1, -1, 0, 0 );
     efl_gfx_path_bounds_get(node, &r);
     if (r.x < 0 || r.y < 0 || r.w <= 0 || r.h <= 0 || r.w + r.x > w || r.h + r.x > h)
        return ;                              
                                              
     // Make empty buffer
     void *pixels = calloc(r.w * r.h, sizeof(uint32_t*));
     Ector_Buffer *buffer = ENFN->ector_buffer_new(ENC, obj->layer->evas->evas,
                                              r.w, r.h,
                                              EFL_GFX_COLORSPACE_ARGB8888,
                                              ECTOR_BUFFER_FLAG_DRAWABLE |
                                              ECTOR_BUFFER_FLAG_CPU_READABLE |
                                              ECTOR_BUFFER_FLAG_CPU_WRITABLE);
     ector_buffer_pixels_set(buffer, pixels,
                             r.w, r.h, 0,
                             EFL_GFX_COLORSPACE_ARGB8888, EINA_TRUE);

     // Buffer change
     ector_buffer_pixels_set(ector, pixels,
                             r.w, r.h, 0,
                             EFL_GFX_COLORSPACE_ARGB8888, EINA_TRUE);
     ector_surface_reference_point_set(ector, -r.x, -r.y);

     // Draw child node to changed buffer
     EINA_LIST_FOREACH(cd->children, l, child)
        _evas_vg_render(obj, pd, engine, output, context, child, clips, r.w, r.h, ector, do_async);
     
     // Re-set original surface
     ENFN->ector_begin(engine, output, context, ector, 0, 0, EINA_FALSE, do_async);

     // Draw buffer to original surface.(Ector_Surface)
     ector_surface_draw_image(ector, buffer, r.x, r.y, alpha);

     if (pixels) free(pixels);
     if (buffer) efl_unref(buffer);
  }

@Hermet
I modified the code and tested it. With your comments
But i found some problems and it will take time to analyze the reasons.

  • some sample are not displayed
  • some sample are displayed and with 1 pixel line at left side

So I think it would be better to do this optimization in the next task.

jsuya updated this revision to Diff 22721.Jun 13 2019, 11:07 PM

add TODO comments

Hermet requested changes to this revision.Jun 13 2019, 11:09 PM

Please check a comment.

src/lib/evas/canvas/efl_canvas_vg_object.c
444

This buffer is to dangling and not managed.
Container need to keep this buffer and reuse, destroy it.
Please refer how masking buffer is managed.

This revision now requires changes to proceed.Jun 13 2019, 11:09 PM
jsuya updated this revision to Diff 22722.Jun 13 2019, 11:58 PM

Modify the buffer to reuse when the layer has transparency.

Hermet requested changes to this revision.Jun 14 2019, 12:54 AM

Please check a comment.

src/lib/evas/canvas/efl_canvas_vg_object.c
441

This condition is logically wrong.
It must be this like...
if (!cd->alphablending_pixels) create pixels
if (!cd->alphablending_buffer)
create buffer

450

this could be skipped.

src/lib/evas/canvas/evas_vg_private.h
100

Naming is unnecessarily long,
How about blend_buffer, blend_pixels ?

This revision now requires changes to proceed.Jun 14 2019, 12:54 AM
jsuya updated this revision to Diff 22851.Jun 19 2019, 9:42 PM

Fix wrong condition and change member name.

jsuya added inline comments.Jun 19 2019, 9:48 PM
src/lib/evas/canvas/efl_canvas_vg_object.c
450

As i as know, buffer_pixels_set is must be needed after that create the buffer.
pixel_buffer is shared to blend_buffer and ector(original surface).

Hermet accepted this revision.Jun 20 2019, 12:06 AM
This revision is now accepted and ready to land.Jun 20 2019, 12:06 AM
This revision was automatically updated to reflect the committed changes.