Good !
I added you to evas.dox.in as well.
You'll see your name on below page later. (It takes time changes to be
applied to the web page.)
http://docs.enlightenment.org/auto/evas/

Thanks.
Daniel Juyung Seo (SeoZ)

On Thu, Mar 17, 2011 at 2:39 AM, Cedric BAIL <cedric.b...@free.fr> wrote:
> Hi again,
>
> On Wed, Mar 16, 2011 at 5:02 PM, Sung W. Park <sung...@gmail.com> wrote:
>> Hi Cedric,
>> Thanks for the review and the comments.  I'm glad to hear that it worked =)
>>
>> On Wed, Mar 16, 2011 at 11:35 PM, Cedric BAIL <cedric.b...@free.fr> wrote:
>>> On Wed, Mar 16, 2011 at 8:26 AM, Sung W. Park <sung...@gmail.com> wrote:
>>> > Based on the discussions and the comments that I've been getting, I'm
>>> > attaching
>>> > patches for the community to review and test out the code. The patches
>>> > implement
>>> > the code for the gl_x11 backend.  Other backends can be taken care of
>>> > later
>>> > once this has been reviewed.
>>> >
>>> > I apologize if this patch seems a bit big.  I felt that we've had enough
>>> > discussions
>>> > on this topic already in increments and now we're at a point where we
>>> > can
>>> > see this
>>> > prototype.
>>>
>>> I don't think them to big, they are really easy to understand and
>>> follow. I really like that !
>>>
>>> > Here are the description of the patches:
>>> > (by the way, apply the patches by giving -p0 from evas directory)
>>> >
>>> > 1. Evas_Engine_GL_Context.patch
>>> >  - Basically, this patch replaces all the instances of Evas_GL_Context
>>> > used
>>> >    by the gl engines to Evas_Engine_GL_Context.  This was changed
>>> > because
>>> >    Evas_GL_Context is exposed to the users by Evas_GL.h and this was
>>> > ok'ed
>>> >    in the previous discussion.
>>>
>>> Ok, just reading it. Why do you need to add :
>>> +#if defined (GLES_VARIETY_S3C6410) || defined (GLES_VARIETY_SGX)
>>> +#include <EGL/egl.h>
>>> +#else
>>> +#include <GL/glx.h>
>>> +#endif
>>
>> This part of the code should not be there!  It was from the previous hack
>> that I did to expose the Evas' GL context.  It was a foolish was but
>> it did help me get my feet wet.  I thought I deleted it but it managed to
>> hide in there somehow.  You can just delete it and it works fine.
>
> So that one is in svn. I also added you in the AUTHORS file, if you
> prefer another mail address, please let me know.
>
>>> in src/modules/engines/gl_x11/Evas_Engine_GL_X11.h . What is the
>>> purpose of this patch ? And why do you use evas private configure
>>> define in a public header ?
>>>
>>> By the way the rest of the patch is ok to go in as it is.
>>>
>>> > 2. evas_gl.patch
>>> >  - This patch includes the image_object_native_surface_set() patch
>>> >    since it's required to run evas_gl and this patch hasn't been
>>> > reflected
>>> >    in the svn yet.
>>> >  - And, it has the rest of the code that allows evas_gl to run for
>>> > gl_x11
>>> > engine.
>>>
>>> It seems you are currently destroying a surface using evas context
>>> instead of the right context, because it could be destroyed. Maybe I
>>> am wrong, but couldn't you implement a refcount attached to the user
>>> context to prevent destruction as long as some surface are attached to
>>> it. The rest of the patch looks good to me. Now we should wait for the
>>> software version :-) Hum, maybe it would be good Vincent if you could
>>> review it to, so we are sure nothing goes wrong when you try to port
>>> this feature on Windows (I don't see anything obvious, but more eyes
>>> is always better).
>>>
>>
>> Essentially, Evas_GL_Surface is implemented using an FBO and a
>> texture and the texture can be seen both by the Evas' GL context and
>> then user created context.  the reason why I'm using Evas' context in
>> the surface_destroy code instead of using the user context is because
>> the user context can be destroyed first.  I could potentially keep a ref
>> count on the context and destroy it together but I didn't see a point in
>> doing that here.  it would work just fine using Evas' GL context.
>> that's why i make sure that i unbind the context once i use it.  maybe
>> there is an angle that i'm not seeing here though.  i'm definitely open
>> to suggestions. =)
>
> Well, it works, it's just it would be cleaner imho, but as you say not
> that much necessary. Not a blocker in my opinion.
>
>>> > 3. Evas_GL.h
>>> >  - Instead of putting it as part of the patch, I've included them
>>> > separately so it
>>> >    can be viewed easily.
>>> >  - This files goes in
>>> >    src/lib/
>>>
>>> > 4. evas_gl.c
>>> >  - Instead of putting it as part of the patch, I've included them
>>> > separately so it
>>> >    can be viewed easily.
>>> >  - This files goes in
>>> >    src/lib/canvas/
>>>
>>> Maybe in evas_gl_new you should check the validity of the Evas object.
>>> And before calling any evas_gl->evas->engine.func, you should check if
>>> they are correctly set. That's for the obvious, don't see much more
>>> than that.
>>>
>> Good point!  I'll add that =)
>>
>>>
>>> > 5. examples/
>>> >  - I've included 3 sample programs + Makefile
>>> >    a. evasglsample1.c (a simple triangle using GL)
>>> >    b. evasglgears.c (famous gears using Evas_GL)
>>> >    c. evasglessample1.c (a simple triangle using GLES)
>>> >  - You can simply do a make in the directory and it'll compile the
>>> > evaglsampl1
>>> >    and evasglgears by default.
>>>
>>> Hehe, looks nice :-) Examples are easy to understand and make this API
>>> sounds good.
>>>
>>> > I just tested with a fresh co from SVN and have applied the patches.  I
>>> > hope
>>> > it works.
>>>
>>> I confirm it run well on my computer :-)
>>>
>>> > thanks in advance for your comments!
>>>
>>> That's good ! Could have been my only comment ;-)
>>
>> Let me know if you guys have other comments/suggestions.
>
> Yeah, stop sleeping and review that patch ! :-)
> --
> Cedric BAIL
>
> ------------------------------------------------------------------------------
> Colocation vs. Managed Hosting
> A question and answer guide to determining the best fit
> for your organization - today and in the future.
> http://p.sf.net/sfu/internap-sfd2d
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>

------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to