patch will be added.
Created attachment 97589 [details] Patch
(In reply to comment #1) > Created an attachment (id=97589) [details] > Patch This patch adds the GraphicsContext3DInternal class using Evas_GL APIs. Evas_GL APIs allow to use OpenGL to render to specially set up evas_image_objects which act as render target surfaces. GraphicsContext3D in Efl port will be used for accelerated compositing as well as WebGL.
Comment on attachment 97589 [details] Patch Patch is *huge*; could you please split it into smaller, preferably less than 20kB parts?
Created attachment 97745 [details] Patch
(In reply to comment #4) > Created an attachment (id=97745) [details] > Patch The patch has split into three bugs. This patch contains only GraphicsContext3DEfl.cpp. - GraphicsContext3D.h : Bug 62959 - GraphicsContext3DInternal.h/cpp : Bug 62961 I'm looking forward to the review.
Reviewing this depends on changes made to bug 62961, so there's not much to say about this one. Is there going to be another patch plugging these new files into the build system?
(In reply to comment #6) > Reviewing this depends on changes made to bug 62961, so there's not much to say about this one. > > Is there going to be another patch plugging these new files into the build system? Yes, I will upload another patch for adding new files to build-script files. Thanks.
Created attachment 98116 [details] Patch
It looks mostly OK, it just needs to be updated when the final version of GraphicsContext3DInternal is approved. > Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:29 > +#include <wtf/text/CString.h> I don't think you need this header after you pass a String directly to Graphics
Comment on attachment 98116 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98116&action=review This looks like a huge wrapper class; isn't it possible to share most of this code with other ports, as there are few references to Evas and such? > Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:35 > + bool renderDirectlyToEvasGLObject = (renderStyle == RenderDirectlyToHostWindow) ? true : false; No need to use a ternary operator here; comparisons already returns a boolean. > Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:142 > + m_internal->bufferData(target, size, /* data */ 0, usage); Is this comment really needed? > Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:167 > + m_internal->clearColor(r, g, b, a); I'd use more descriptive names here. > Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:177 > + m_internal->clearStencil(s); Ditto.
Created attachment 98298 [details] Patch
(In reply to comment #11) > Created an attachment (id=98298) [details] > Patch I updated this patch to reflect the comment #9 and #10.
(In reply to comment #10) > (From update of attachment 98116 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=98116&action=review > > This looks like a huge wrapper class; isn't it possible to share most of this code with other ports, as there are few references to Evas and such? Yes, most of this code are sharable. Source/WebCore/platform/graphics/GraphicsContext3D.h:934 // FIXME: ideally this would be used on all platforms. #if PLATFORM(CHROMIUM) || PLATFORM(QT) || PLATFORM(GTK) friend class GraphicsContext3DInternal; OwnPtr<GraphicsContext3DInternal> m_internal; #endif Chromium & QT already have their own GraphicsContext3DInternal class. How can you make it common? I have no concrete plans yet. Thank you for your comment :)
Informal r+ from my side now that GraphicsContext3DInternal looks OK too.
Comment on attachment 98298 [details] Patch rs=me.
Comment on attachment 98298 [details] Patch Clearing flags on attachment: 98298 Committed r95002: <http://trac.webkit.org/changeset/95002>
All reviewed patches have been landed. Closing bug.