Hi,
I was debugging the crash with elm_genlist_clear() in 1.8 and I'm
wondering the rationale of "rel". Currently it is badly broken given
that you use anything other than append/prepend without parent.
The current code keeps a list of items in the smart data (sd->items).
Although these are ordered as they would be displayed, the code will
replicate that information in the "rel" (it->items->rel), that means
the item the current one is relative to. Then it keeps a boolean
"before" flag and a reverse relative list (who is relative to it).
Bad things happens if you insert or move before, after or sorted as
the siblings are not updated. Take _item_move_before(it, before):
sd->items =
eina_inlist_remove(sd->items, EINA_INLIST_GET(it));
if (it->item->block) _item_block_del(it);
sd->items = eina_inlist_prepend_relative
(sd->items, EINA_INLIST_GET(it), EINA_INLIST_GET(before));
So far, so good. remove from old position and place it where it should
be. Delete the block so it would be recreated when needed.
if (it->item->rel)
it->item->rel->item->rel_revs =
eina_list_remove(it->item->rel->item->rel_revs, it);
It was relative to another item, then remove the current item from the
reverse relative list of that other item as it's not relative to it
anymore. Good.
it->item->rel = before;
before->item->rel_revs = eina_list_append(before->item->rel_revs, it);
it->item->before = EINA_TRUE;
Now make the item relative to the given parameter "before", also
appending itself to its reverse relative list. Flag as "before". Just
that?
Did you notice the problem? If not take the given list as input:
- a (rel = , before=False, rel_revs=[c])
- c (rel = a, before=False, rel_revs=[d])
- d (rel = c, before=False, rel_revs=[b])
- b (rel = d, before=False, rel_revs=[])
Now move "b" before "c". The result will be:
- a (rel = , before=False, rel_revs=[c])
- b (rel = c, before=True, rel_revs=[])
- c (rel = a, before=False, rel_revs=[d, b])
- d (rel = c, before=False, rel_revs=[]).
Wait, "c" is still relative to "a" even if should be after "b"... To
fix this is quite cumbersome as other elements would be relative given
they can be before=True or False. Would need to walk the reverse
relative's list of "c" and see if "b" got in the way of them, changing
their relative members.
Looking at the code it seems the user of that is _item_block_add(),
does it need all of that complexity to do its job? I didn't review
such function, but if we could remove all those pointer+boolean+list
we would save memory AND complexity.
Would someone with more experience in that code review the usage of
"rel" and either remove that or fix its handling in the code?
PS: I'm also not sure but this can be related to the problem of
keyboard navigation where you expand a genlist item, then move down it
would go to the next top item, then down again and it goes to the
first child of the expanded. Very easy to notice with espionage (d-bus
inspector in python-efl).
--
Gustavo Sverzut Barbieri
--------------------------------------
Mobile: +55 (19) 9225-2202
Contact: http://www.gustavobarbieri.com.br/contact
------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT
organizations don't have a clear picture of how application performance
affects their revenue. With AppDynamics, you get 100% visibility into your
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
