Hi Jaehwan,

2011/9/24 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.

>> And could you please split your patch in two part, one with just the
>> change to use "current_part" and another with the inherit change.
>
> I will upload again ,after split the patch code.

I will look at them.

Thanks,
-- 
Cedric BAIL

------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2dcopy2
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to