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
