Page MenuHomePhabricator

Efl.Gfx.Vg.Value_Provider: Introduce property change feature of Efl.Ui.Animation_View
ClosedPublic

Authored by jsuya on Sep 9 2019, 5:49 AM.

Details

Summary

Efl.Gfx.Vg.Value_Provider is an object for integrating and managing the properties of vector objects.
These values are dependent on the keypath.
Keypath is the target a specific content or a set of contents that will be updated.
It can include the specific name of the contents, wildcard(*) or Globstar(**).

The valueProvider is borrowed from another library that uses a vector object of type json, such as Efl.Ui.Animation_View
(https://github.com/airbnb/lottie-ios/blob/5fc0e59e0cb85d3586b1d0d1cf4a2c9669b91d15/lottie-swift/src/Public/iOS/AnimatedControl.swift#L50)

This feature should be used with some patches that apply to the vg json loader and Efl.Canvas.Vg.Object.

Test Plan

N/A

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13209
Build 9375: arc lint + arc unit
jsuya created this revision.Sep 9 2019, 5:49 AM
jsuya requested review of this revision.Sep 9 2019, 5:49 AM
jsuya updated this revision to Diff 24878.Sep 10 2019, 4:20 AM

code update

jsuya updated this revision to Diff 24879.Sep 10 2019, 4:22 AM

remove unnecessary code

jsuya updated this revision to Diff 24886.Sep 10 2019, 5:42 AM
jsuya retitled this revision from [WIP] Efl.Gfx.Vg.Value_Provider: Introduce property change feature of Efl.Ui.Animation_View to Efl.Gfx.Vg.Value_Provider: Introduce property change feature of Efl.Ui.Animation_View.

update commit message

Hermet requested changes to this revision.Thu, Sep 19, 12:19 AM

Please check comments.

src/lib/elementary/efl_ui_animation_view.c
239

before it remove the list, it must derefence the items to free them properly.
ex)
EINA_LIST_FREE(pd->vp_list, vp)

efl_unref(vp);
740

++reference.
efl_ref(value_provider);

748

I think ex vp can be just removed from the list and replaced with value_provider. thats' it. don't need to copy properties.

src/lib/evas/canvas/efl_gfx_vg_value_provider.c
38

Seems this method doesn't necessary.

src/lib/evas/canvas/efl_gfx_vg_value_provider.eo
2

This shouldn't be exposed.

12

Can we say this Value_Provider is a type of Color or Shape?
Please check shape and path whether this value_provider is satisfied with their spec.

I'd rather consider clear interfaces for overridable value list....

class value_provider {
@property keypath:
@property transform:
@property stroke_color:
..

Done.

};

This revision now requires changes to proceed.Thu, Sep 19, 12:19 AM
jsuya marked 5 inline comments as done.Thu, Sep 19, 7:02 PM
jsuya added inline comments.
src/lib/elementary/efl_ui_animation_view.c
748

I think, we have to consider this case.

ValueProvider1 "shape1.*" FillColor 0 0 0 255

StrokeColor 0 255 0 255

override( Animation_view, ValueProvider1);
ValueProvider2 "shape1.*" FillColor 255 255 255 255
override( Animation_view, ValueProvider2);
and
When user want to remove overrided FillColor value of "shape1.*" path.

When change color black to white, we have to keep Strokecolor value.
Then I think that it is better to manage valueprovider based on path.
At the very least, I think the way to delete an existing valueprovider when an override occurs cannot handle this case.
what do you think?

src/lib/evas/canvas/efl_gfx_vg_value_provider.c
38

This is necessary if we keep the logic of efl_ui_animation_view.c +748.

jsuya updated this revision to Diff 25234.Fri, Sep 20, 1:09 AM
jsuya marked an inline comment as done.

update commit
Add ref,unref for vp
Remove copy property
Change enum and get function to internal api

jsuya updated this revision to Diff 25320.Sun, Sep 22, 7:18 PM

update commit

jsuya updated this revision to Diff 25326.Sun, Sep 22, 11:14 PM

remove unused function

jsuya updated this revision to Diff 25328.Sun, Sep 22, 11:16 PM

rebase commit

Hermet requested changes to this revision.Mon, Sep 23, 10:05 PM

Please check one more comments.

src/lib/elementary/efl_ui_animation_view.c
719

I think we need a secure code if user missed adding the keypath.
if (!keypath) ERR("...") return;

src/lib/evas/canvas/efl_gfx_vg_value_provider.c
21

do we need to free eina_stringshare_del(pd->keypath); ??

38

how about just "transform_set" ?

68

Let's remove this exceptional handling, user can easily figure out as the result color would look wrong.

//Exception Handling.
if (r < 0) r = 0;
if (g < 0) g = 0;
if (b < 0) b = 0;
if (a > 255) a = 255;
else if (a < 0) a = 0;

if (r > a)
  {
     r = a;
     perr = EINA_TRUE;
  }
if (g > a)
  {
     g = a;
     perr = EINA_TRUE;
  }
if (b > a)
  {
     b = a;
     perr = EINA_TRUE;
  }

if (perr) ERR("Evas only handles pre-multiplied color!");
115

Remove here exception as well.

src/lib/evas/canvas/efl_gfx_vg_value_provider.h
14

Eina_Stringshare *

This revision now requires changes to proceed.Mon, Sep 23, 10:05 PM
jsuya updated this revision to Diff 25433.Tue, Sep 24, 12:12 AM
jsuya marked 6 inline comments as done.

update commit

  • Change keypath value type
  • Change property name transfomation to transform.
  • Remove pre-multiplied color check.
Hermet accepted this revision.Thu, Sep 26, 3:46 AM

Looks fine to me please apply this after 1.23 release.

This revision is now accepted and ready to land.Thu, Sep 26, 3:46 AM
jsuya updated this revision to Diff 25854.Tue, Oct 1, 10:20 PM

enhance docs

This revision was automatically updated to reflect the committed changes.