We recently started using smart pointers in WK2 EFL port. However, the accelerated compositing code is still using raw pointers for Evas_GL* objects. We should fix this.
Created attachment 171433 [details] Patch
Any feedback on this patch please?
Comment on attachment 171433 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171433&action=review > Source/WebKit2/UIProcess/API/efl/EvasGLSurface.cpp:33 > +EvasGLSurface::EvasGLSurface(Evas_GL* evasGl, Evas_GL_Surface* surface) Maybe call this passSurface as you pass ownership? > Source/WebKit2/UIProcess/API/efl/EvasGLSurface.h:46 > + return adoptPtr(new EvasGLSurface(evasGl, surface)); // Sets m_evasGL. I suppose we should use GL and not Gl in accordance with style guide > Source/WebKit2/UIProcess/API/efl/EvasGLSurface.h:55 > + Evas_GL* m_evasGl; What are you using this for? Ah it is being set by the ctor. > Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:254 > + evas_gl_make_current(m_evasGl.get(), m_evasGlSurface->surface(), m_evasGlContext->context()); wouldn't ->get() not be more consistent? hmm I see that won't work as it already exists.
Created attachment 171633 [details] Patch Take Kenneth's feedback into consideration.
Comment on attachment 171633 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171633&action=review > Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:471 > + evas_gl_native_surface_get(m_evasGL.get(), m_evasGLSurface->surface(), &nativeSurface); > evas_object_image_native_surface_set(sd->image, &nativeSurface); > > - evas_gl_make_current(m_evasGl, m_evasGlSurface, m_evasGlContext); > + evas_gl_make_current(m_evasGL.get(), m_evasGLSurface->surface(), m_evasGLContext->context()); why not call evasGL() instead of m_evasGL.get(). > Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:473 > + Evas_GL_API* gl = evas_gl_api_get(evasGL()); like here we are doing so
Created attachment 171645 [details] Patch Take Kenneth's feedback into consideration. Good point, it looks nicer and more consistent.
Comment on attachment 171645 [details] Patch Clearing flags on attachment: 171645 Committed r133041: <http://trac.webkit.org/changeset/133041>
All reviewed patches have been landed. Closing bug.
Comment on attachment 171645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171645&action=review Sorry for late comments. But, it seems to me there are some nits. > Source/WebKit2/PlatformEfl.cmake:45 > + UIProcess/API/efl/EvasGLContext.cpp Wrong alphabetical order. > Source/WebKit2/PlatformEfl.cmake:46 > + UIProcess/API/efl/EvasGLSurface.cpp ditto. > Source/WebKit2/UIProcess/API/efl/EvasGLSurface.cpp:35 > + , m_surface(passSurface) passSurface ? I think surface is just enough. > Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:493 > + EINA_LOG_DOM_WARN(_ewk_log_dom, "Failed to create GLContext."); We already define WARN macro for this. > Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:499 > + EINA_LOG_DOM_WARN(_ewk_log_dom, "Failed to create GLSurface."); ditto.