Here we go !

2011/9/28 Jaehwan Kim <jae.hwan....@samsung.com>:
>>>>> 7. Do not increase the size of runtime structure, especially Edje_Part
>>>>> and also Edje_program as much as possible. The reorder is maybe doable
>>>>> when you find the keyword, that would avoid completly the reorder
>>>>> structure inside Edje_Part.
>>>>
>>>> If I increase the size of runtime structure, previously built edj files may
>>>> have
>>>> a problem. You concerned about it, right? I didn't think about that. then 
>>>> how
>>>> can I change the code? Do you have any idea? I saw the code you change
>>>> in 62086. Should I change like that?
>>>
>>>> The issue is that with your change, we now have pointer that are not
>>>> used at runtime, but are allocated. This is a waste of memory in my
>>>> opinion. It will not break previous edje file, thanks to eet, but
>>>> still that's not a good thing in my opinion. Maybe instead of adding
>>>> this structure, you could try to put the part/program at the right
>>>> place when you find the keyword that tell you where to put it.
>>>>    As for can_override, I think it should be part of a more generic
>>>> infrastructure to detect when setting a value override an existing
>>>> property. So lets forget about that one for the moment. You don't need
>>>> to change the way you handle it in your patch. When we improve edje
>>>> error, we could improve that at that time.
>>>
>>> I separated the structure of reorder from Edje_Part. And after reordering,
>>> the structure is deleted. Becuase Edje_Part just has the reorder pointer,
>>> I think there isn't the waste of the memory.
>>> Please check attached patch file.
>>
>>> This change sounds good to me, just one question why didn't you put
>>> the "can_override" member inside the reorder member ?
>>
>> "can_override" is not related to reorder feature. So I think it is not 
>> proper.
>> And "can_override" is in three structure but reorder is just in _Edje_Part
>> structure. I think it's not proper in unity aspect, too.
>
>> Yes, but can_override is a data only used by edje_cc. So puting it in
>> Edje_Part make it increase the size of Edje at runtime. In fact, in my
>> opinion, it would make sense to create an Edje_Part_Parser or
>> something like that, that would hold this can_override and the reorder
>> field to.
>
> I changed the structure of three struct (Edje_Program, Edje_Pack_Element,
> Edje_Part) as you said. I made three struct (Edje_Program_Parser,
> Edje_Pack_Element_Parser, Edje_Part_Parser). can_override and reorder
> are used in that struct. So Edje_Program, Edje_Pack_Element and Edje_Part
> are not changed. We don't worry about the increasing the size of Edje at 
> runtime.
> I tested it and it works well. Please check my patch.

I did just move the various _Parser structure inside edje_cc.h instead
of edje_private.h and fixed some obvious source of bug. As the rest
looked fine, I pushed it in svn, so every one can now try and use it.
Thanks for your work.

Have fun !
-- 
Cedric BAIL

------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2dcopy1
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to