2010/9/28 Gustavo Sverzut Barbieri <barbieri@...>:
> 2010/9/23 Otávio Pontes <otavio@...>:
>> 2010/9/22 Gustavo Sverzut Barbieri <barbieri@...>:
>>> 2010/9/22 Otávio Pontes <otavio@...>:
>>>> I have added the transition layout into edje too. It displays an
>>>> animation when changing box state with different layouts.
>>>> I am sending the patch for edje and an edc sample file.
>>> Awesome results, few comments:
>>> - edje users don't have direct access to items, so you shouldn't
>>> require to listen to child,added/child,removed... but it doesn't
>>> hurt... however:
>>> - edje already knows about the items, as they are managed through
>>> edje_object_part_box_* methods. You should reuse that. This will avoid
>>> reload children list to recreate all items again.
>> Done. And it was a better solution.
>>> - at _edje_box_layout_calculate_coords() you don't revert the box
>>> properties (padding, align) to the original once you measure the final
>>> state. Okay, at layout you have a "if (progress == 0.0)" and then do
>>> that, but it is better to do it in the other function. Also, comparing
>> Setting box properties in layout function when progress == 0 is
>> necessary because sometimes layout is used without any animation and
>> _edje_box_layout_calculate_coords is called only when an animation
>> will start. After running _edje_box_layout_calculate_coords the layout
>> will place objects without looking at box properties. These properties
>> will be needed only when progress == 0.0, so setting them on
>> _edje_box_layout_calculate_coords is not necessary.
>>> for double equality will raise some warnings for compilers with
>>> -Wextra AFAIR.
>> Checking now if progress < 0.01 to avoid floating point problems.
>>> - double check if anim->start_progress < 1 before calling _exec() to
>>> avoid division by 0.0 and an FPU exception? While it is hard to happen
>>> in tests, it may happen one day in platforms that crash the
>>> application :-(
>>> - edje coding style require all structs with Edje prefix, even if
>>> they are private.
>>> Last but not least, few questions that I cannot remember from Edje and
>>> from your patch it is not obvious to me:
>>> - does it still allocates the animation even if the transition is
>>> immediate (ie: zero timed "transition:" in your program)? Seems so as
>>> _edje_box_recalc_apply() always create it.
>> _edje_box_recalc_apply aways allocate the anim struct, that is used
>> for all animations with this box. I could allocate the struct only if
>> necessary, but it wont be possible to avoid reloading children in
>> _edje_box_layout_load_children_list. In this new patch anim struct is
>> created when the first child is added to the box.
>>> I'd move this to part box constructor:
>>> evas_object_box_layout_set(ep->object, _edje_box_layout, ep->anim,
>>> _edje_box_layout_free_data), always have it (you may even call it just
>>> "box" and remove the "anim" reference. Then you free it (your
>>> _edje_box_layout_free_data) at part delete. You keep this as the
>>> single layout function ever set to ep->object. Then move all your
>>> _edje_box_layout_calculate_coords() to _edje_box_recalc_apply() as
>>> there you know for sure what happened and do not have to set a flag to
>>> be checked later... I guess these things will make code simpler and
>>> less error prone, also easier for others to review and maintain it.
>> Calling functions to allocate and destroy data in edje_load.c. Restart
>> flag was removed and _edje_box_layout_calculate_coords is called in
>> in _edje_box_recalc_apply. The recalculate flag is still necessary.
>> Sending new patch attached.
> sorry taking so long! in svn as r52871.
> Gustavo Sverzut Barbieri
> http://profusion.mobi embedded systems
> MSN: barbieri@...
> Skype: gsbarbieri
> Mobile: +55 (19) 9225-2202