[Webkit-unassigned] [Bug 101291] [EFL] Refactor GraphicsContext3DEFL

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2012-11-14 13:54:20 PST ---
(From update of attachment 174209)
View in context: https://bugs.webkit.org/attachment.cgi?id=174209&action=review

> Source/WebCore/platform/graphics/efl/GLNativeContext.cpp:38
> +static PFNGLGETGRAPHICSRESETSTATUSARBPROC  glGetGraphicsResetStatusARB = 0;

remove double space before gl

> Source/WebCore/platform/graphics/efl/GLNativeContext.cpp:44
> +    static bool initializedShims = false;
> +    static bool didInitializeShims = true;

why both?

> Source/WebCore/platform/graphics/efl/GLNativeContext.cpp:155
> +bool GLNativeContext::isContextLost() const
> +{
> +    return m_contextLost;
> +}

isValid ?

> Source/WebCore/platform/graphics/efl/GLNativeContext.cpp:166
> +PlatformNativeContext GLNativeContext::handle() const

It is confusing that the class is called GL*NativeContext* but this handle() returns the PlatformNativeContext.

Should the class just be called GLContext? Better name?

> Source/WebCore/platform/graphics/efl/GLNativeContext.h:72
> +    // Destroys any gl resources associated with this context.

GL

> Source/WebCore/platform/graphics/efl/GLNativeSurface.cpp:47
> +#if HAVE(GLX)
> +    OwnPtr<GLNativeSurface> surface =  adoptPtr(new GLXPBuffer());
> +
> +    if (surface->handle().isValid())
> +        return surface.release();
> +#endif

Later this will handle EGL?

> Source/WebCore/platform/graphics/efl/GLNativeSurface.cpp:92
> +PlatformSurfaceConfig GLNativeSurface::surfaceConfig()

::configuration()

> Source/WebCore/platform/graphics/efl/GLNativeSurface.h:43
> +public:
> +

I would move public: down one line and remove the next newline

> Source/WebCore/platform/graphics/efl/GLNativeSurface.h:47
> +    // Create a GL surface used for offsreen rendering. The results can be transported

*screen* (spelling)

> Source/WebCore/platform/graphics/efl/GLNativeSurface.h:79
> +    bool m_RestoreNeeded;

non capital r!

> Source/WebCore/platform/graphics/efl/GLNativeSurface.h:85
> +    PlatformNativeSurface m_drawable;
> +
> +};

remove newline

> Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:170
> +    m_private->freeResources();

releaseResources() ?

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:46
> +    , m_PendingSurfaceResize(false)

non capital P!

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:71
> +            freeResources();

releaseResources()

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:119
> +        m_contextLostCallback->onContextLost();

We dont use on_ something... didLooseContext

> Source/WebCore/platform/graphics/surfaces/glx/GLXSurface.h:39
> +static long X11Redirect = 1L << 9;

Where does this come from? comment?

> Source/WebCore/platform/graphics/surfaces/glx/GLXSurface.h:41
> +class SharedResources : public WTF::RefCountedBase {

SharedX11Resources?

> Source/WebCore/platform/graphics/surfaces/glx/GLXSurface.h:48
> +            m_staticSharedResource =  new SharedResources();

no double space before new

> Source/WebCore/platform/graphics/surfaces/glx/GLXSurface.h:125
> +        // Didnt find any visual supporting alpha, select the first available config.

Did not *

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