2010/9/22 Otávio Pontes <[email protected]>:
> 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.

  - 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
for double equality will raise some warnings for compilers with
-Wextra AFAIR.

  - 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.


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.

Good work so far!


-- 
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
--------------------------------------
MSN: [email protected]
Skype: gsbarbieri
Mobile: +55 (19) 9225-2202

------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to