Re: [E-devel] Badly broken genlist item rel handling

2013-11-29 Thread Gustavo Sverzut Barbieri
On Thu, Nov 28, 2013 at 10:17 PM, Carsten Haitzler ras...@rasterman.com wrote:
 On Thu, 28 Nov 2013 20:22:21 -0200 Gustavo Sverzut Barbieri
 barbi...@gmail.com said:

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

 i remember putting in rel ONLY for the purpose of the queue handling. ie the
 item is queued but since it is relative TO something, rel is filled in with
 before true/false. after the item is no longer in the queue - rel should be
 unused and ignored. we could happily set it to NULL if we wanted.

Good to know, reseting these flags would make lots of sense, avoid
bugs (as the one I fixed with _item_del_pre_hook, or at least make it
harder to show as it would still happen if the item wasn't realized
yet) and save memory (you'd not keep the list in memory).

While at that, I'd say it's better to remove the rel and before
from items and make it a separate structure that is allocated once the
item is relative to something (you'd only have a single pointer wasted
in the main structure for the item, not a pointer + bitflag, that if
padding is wrong may spawn over one pointer in size!). Similarly we
could have rel_revs to not be a list, but a structure with two
pointers... if we fix this bug keeping the rel, then we'll always
have at most one before and another after, so 2 ptrs.


 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.

 but then you'd be forced to evaluate all queued items immediately. either that
 or re-architect how the queue works. ie insert items immediately into the
 actual list AND keep a queue entry, but this will have all sorts of nasty
 knock-on effects when a user scrolls as they will encounter entire block or
 sets of items not yet sized and identified.

well, the current approach is also wrong, so we need to find a solution.

Can't we insert in the sd-items list and somehow flag before/after
item blocks as gone, then we just go and regenerate them? (that may
not make sense, I never looked into the block handling of genlist)


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

[EGIT] [core/efl] master 02/04: ecore/wayland: Get the touch up event position from the down_info.

2013-11-29 Thread Rafael Antognolli
antognolli pushed a commit to branch master.

http://git.enlightenment.org/core/efl.git/commit/?id=2c95c5ee1d470650480781fe72518ef67263010d

commit 2c95c5ee1d470650480781fe72518ef67263010d
Author: Rafael Antognolli rafael.antogno...@intel.com
Date:   Thu Nov 28 17:53:42 2013 -0200

ecore/wayland: Get the touch up event position from the down_info.

down_info is a struct that stores some information about the current
pressed touch events. It should be used for that specific touch point,
instead of the generic input info, when sending a mouse_up event.
---
 src/lib/ecore_wayland/ecore_wl_input.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/lib/ecore_wayland/ecore_wl_input.c 
b/src/lib/ecore_wayland/ecore_wl_input.c
index a9b7ac6..4661891 100644
--- a/src/lib/ecore_wayland/ecore_wl_input.c
+++ b/src/lib/ecore_wayland/ecore_wl_input.c
@@ -1357,8 +1357,6 @@ _ecore_wl_input_mouse_up_send(Ecore_Wl_Input *input, 
Ecore_Wl_Window *win, int d
  ev-buttons = button;
 
ev-timestamp = timestamp;
-   ev-x = input-sx;
-   ev-y = input-sy;
ev-root.x = input-sx;
ev-root.y = input-sy;
ev-modifiers = input-modifiers;
@@ -1372,11 +1370,15 @@ _ecore_wl_input_mouse_up_send(Ecore_Wl_Input *input, 
Ecore_Wl_Window *win, int d
   ev-double_click = 1;
 if (down_info-did_triple)
   ev-triple_click = 1;
+ev-x = down_info-sx;
+ev-y = down_info-sy;
 ev-multi.x = down_info-sx;
 ev-multi.y = down_info-sy;
  }
else
  {
+ev-x = input-sx;
+ev-y = input-sy;
 ev-multi.x = input-sx;
 ev-multi.y = input-sy;
  }

-- 




[EGIT] [core/elementary] master 01/02: elm_widget.c: fixed formatting while reading the code.

2013-11-29 Thread Daniel Juyung Seo
seoz pushed a commit to branch master.

http://git.enlightenment.org/core/elementary.git/commit/?id=f4baea0ebde0d3048a3e8f7bd1a149a00278

commit f4baea0ebde0d3048a3e8f7bd1a149a00278
Author: Daniel Juyung Seo juyung@samsung.com
Date:   Sat Nov 30 15:28:44 2013 +0900

elm_widget.c: fixed formatting while reading the code.
---
 src/lib/elm_widget.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/lib/elm_widget.c b/src/lib/elm_widget.c
index d06a6b6..d22399e 100644
--- a/src/lib/elm_widget.c
+++ b/src/lib/elm_widget.c
@@ -567,28 +567,28 @@ _propagate_event(void *data,
switch (type)
  {
   case EVAS_CALLBACK_KEY_DOWN:
-  {
- Evas_Event_Key_Down *ev = event_info;
- event_flags = (ev-event_flags);
-  }
-  break;
+   {
+  Evas_Event_Key_Down *ev = event_info;
+  event_flags = (ev-event_flags);
+   }
+ break;
 
   case EVAS_CALLBACK_KEY_UP:
-  {
- Evas_Event_Key_Up *ev = event_info;
- event_flags = (ev-event_flags);
-  }
-  break;
+   {
+  Evas_Event_Key_Up *ev = event_info;
+  event_flags = (ev-event_flags);
+   }
+ break;
 
   case EVAS_CALLBACK_MOUSE_WHEEL:
-  {
- Evas_Event_Mouse_Wheel *ev = event_info;
- event_flags = (ev-event_flags);
-  }
-  break;
+   {
+  Evas_Event_Mouse_Wheel *ev = event_info;
+  event_flags = (ev-event_flags);
+   }
+ break;
 
   default:
-break;
+ break;
  }
 
elm_widget_event_propagate(obj, type, event_info, event_flags);

--