Hi Chris, Thanks for the fast review. :)
On Tue, Sep 18, 2012 at 4:17 PM, Christopher Michael <cp.mich...@samsung.com > wrote: > Hi Sung, > > Initial review of this: > > Short version, it looks good :) but found some problems in you're patch: > > In the evgl_eng_window_surface_create for the GLES_VARIETY_SGX path: > > + surface = eglCreateWindowSurface(re->**win->egl_disp, > + re->win->egl_config, > + (EGLNativeWindowType)**DefaultRootWindow(re->info->**info.display), > + NULL); > > DefaultRootWindow is an X11 function. This will fail for wayland_egl. > Better to use re->win->win here IMO. > I can fix that easily. I can't remember why I did it this way to tell you the truth. > > Also, I noticed in some places that we are still using > glsym_eglQueryString and glXQueryString. Wouldn't it be better if Evas_GL > provided a QueryString function ? This way the various backends could call > which ever querystring function they use ? (Same for GetProcAddress). > Basically, the EVGL_Interface could provide 2 more methods. QueryString and > GetProcAddress. > What do you mean by some places? In evas_engine? or evas_gl_core? I actually tried to remove all the EGL/GLX dependencies from the Evas_GL part of the code. That way, evas_engine can handle with the glue layer stuff and evas_gl_core would only need to handle with GL. That's the reason why evas_engine has to provide these two functions to evas_gl_core. It's part of the engine interface. Maybe I misunderstood you? > > Other than that, I Like It :) It removes a lot of duplicated code and is > good (in general). > > I agree. I like it a lot better as well (in general) ;-) > Cheers, > dh > > I'll wait for Carsten to take a look at it as well and then push it up. Thanks! Sung > > On 17/09/12 10:52, Sung W. Park wrote: > >> Hi all, >> >> I'd like to ask you devs for review on Evas_GL that I've recently >> refactored >> before I push it upstream. >> >> evas_gl was introduced last year and it has gotten really messy over the >> year >> with a lot of little tweaks here and there. >> >> Also, I've noticed not too long ago that the same code was pretty much >> being >> copied over to wayland_egl engine and so I've decided that it's about time >> to >> rewrite the ugly code. >> >> I've commonized the GL part of the code and made an interface that >> each engine has to implement to get the evas_gl running. In the overall >> scheme of things, one can argue whether this was the best design but with >> what we have currently, i thought it was reasonable. >> >> I'm attaching a patch that modifies the following files... >> >> src/modules/engines/gl_common/**Makefile.am >> src/modules/engines/gl_x11/**evas_engine.c >> >> and I've added 7 new files in gl_common. >> >> src/modules/engines/gl_common/**evas_gl_core_private.h >> src/modules/engines/gl_common/**evas_gl_core.h >> src/modules/engines/gl_common/**evas_gl_core.c >> src/modules/engines/gl_common/**evas_gl_api.c >> src/modules/engines/gl_common/**evas_gl_api_ext_def.h >> src/modules/engines/gl_common/**evas_gl_api_ext.h >> src/modules/engines/gl_common/**evas_gl_api_ext.c >> >> I'm also including two samples files as well as a PPT slides I've >> made for some people here. >> >> Your comments would be greatly appreciated. >> >> cheers, >> Sung >> >> > ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
