Hi Daniel, Can you elaborate on what you mean by making this as a patch?
I've sent two patches for the files that need to be updated, one of which has been applied already. The other one has been reviewed a little but hasn't been applied yet. As for the other files, they need to be freshly added to the svn and since i don't have commit access, i've just attached them separately. Let me know what you mean by it. cheers, Sung On Thu, Mar 24, 2011 at 12:32 AM, Daniel Juyung Seo <seojuyu...@gmail.com>wrote: > Can you make this as a patch? > > Thanks. > Daniel Juyung Seo (SeoZ) > > On Thu, Mar 17, 2011 at 11:34 PM, Sung W. Park <sung...@gmail.com> wrote: > > Hi again, > > > > 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. > >> > > > > Great! That email address is fine. wow, thanks. > > > > > >> > >> >> 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. > >> > > > > It would be cleaner. Maybe I'll revisit it at a later time. I'm taking > > tomorrow > > off to get some rest finally =) > > > > > >> > >> >> > 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 ! :-) > >> > > > > LOL. yeah, it was only a little past midnight when I sent out that > email. > > > > By the way, I have added > > > > MAGIC_CHECK(e, Evas, MAGIC_EVAS); > > return NULL; > > MAGIC_CHECK_END(); > > > > in evas_gl_new() as you've suggested. It's probably not necessary but > I'll > > just attach the file anyway. > > > > cheers, > > Sung > > > > -- > >> 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 > > > > > ------------------------------------------------------------------------------ Enable your software for Intel(R) Active Management Technology to meet the growing manageability and security demands of your customers. Businesses are taking advantage of Intel(R) vPro (TM) technology - will your software be a part of the solution? Download the Intel(R) Manageability Checker today! http://p.sf.net/sfu/intel-dev2devmar _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
