[Webkit-unassigned] [Bug 96627] [EFL] Use the shareable GraphicsContext3DOpenGL* implementation.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=96627





--- Comment #7 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2012-09-26 03:11:48 PST ---
(From update of attachment 164343)
View in context: https://bugs.webkit.org/attachment.cgi?id=164343&action=review

Did you compare this new code with what Qt is doing?

> Source/WebCore/PlatformEfl.cmake:295
> -    platform/graphics/cairo/GLContext.cpp
> -    platform/graphics/cairo/GraphicsContext3DCairo.cpp
> -    platform/graphics/cairo/GraphicsContext3DPrivate.cpp
> +    platform/graphics/efl/GraphicsContext3DEfl.cpp
> +    platform/graphics/efl/GraphicsContext3DPrivate.cpp

So any reason why GTK doesnt want to use the same? I guess this is still based on cairo right

> Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:48
> +    , m_attrs(attrs)

attributes?

> Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:92
> +        glGenTextures(1, &m_texture);
> +        glBindTexture(GraphicsContext3D::TEXTURE_2D, m_texture);
> +        glTexParameterf(GraphicsContext3D::TEXTURE_2D, GraphicsContext3D::TEXTURE_MAG_FILTER, GraphicsContext3D::LINEAR);
> +        glTexParameterf(GraphicsContext3D::TEXTURE_2D, GraphicsContext3D::TEXTURE_MIN_FILTER, GraphicsContext3D::LINEAR);
> +        glTexParameteri(GraphicsContext3D::TEXTURE_2D, GraphicsContext3D::TEXTURE_WRAP_S, GraphicsContext3D::CLAMP_TO_EDGE);
> +        glTexParameteri(GraphicsContext3D::TEXTURE_2D, GraphicsContext3D::TEXTURE_WRAP_T, GraphicsContext3D::CLAMP_TO_EDGE);

Maybe a small comment here makes sense, so that people dont have to look up the functions

> Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:105
> +            glBindFramebuffer(GraphicsContext3D::FRAMEBUFFER, m_fbo);

Maybe m_fbo should be called soemthing like m_nonMultisampleFBO to make the difference clear

> Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:120
> +    // ANGLE initialization.
> +    ShBuiltInResources ANGLEResources;
> +    ShInitBuiltInResources(&ANGLEResources);

Shouldnt this be per PLATFORM(WIN) only?

> Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:139
> +    glClearColor(0.0, 0.0, 0.0, 0.0);

why not white?

> Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:165
> +    if (!m_private)
> +        return false;

enwline after return

> Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:167
> +    if (m_renderStyle == RenderToCurrentGLContext)
> +        return true;

same here

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:60
> +    // For WebKit2, we need to create a dummy ecoreEvas object for the WebProcess in order to use EvasGL APIs.
> +    ecore_evas_init();
> +    Ecore_Evas* ecoreEvas = ecore_evas_gl_x11_new(0, 0, 0, 0, 1, 1);
> +    if (!ecoreEvas)
> +        return;
> +    evas = ecore_evas_get(ecoreEvas);
> +    if (!evas)
> +        return;

As this is X11 dependent we might want to abstract that out. Or at least add some ifdefs

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:91
> +    if (m_surface)
> +        evas_gl_surface_destroy(m_evasGL, m_surface);

OwnPtr works for this, look at RenderThemeEfl for examples

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:96
> +
> +    evas_gl_free(m_evasGL);

where is the ecoreEvas freed?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.