Hi, 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 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). > 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. > 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 ;-) -- 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
