Bug 111759 – [EFL][WebGL] Implement a common GraphicsSurface Implementation.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Kalyan]

Implement common GraphicsSurface implementation which can be used by both GLX and GLES. This should handle the core logic and leave any platform specific bits to be handled separately.

[Kalyan]

Created attachment 192365 [details]
patch

[Kalyan]

Created attachment 192368 [details]
patchv2

[Kenneth Rohde Christiansen]

Comment on attachment 192368 [details]
patchv2

View in context: https://bugs.webkit.org/attachment.cgi?id=192368&action=review

> Source/WebCore/ChangeLog:25
> +        * platform/graphics/efl/GraphicsContext3DPrivate.h:
> +        (GraphicsContext3DPrivate):
> +        TransportSurface related code is moved to GraphicsSurfaceCommon.

I personally prefer a newline before the comment... makes it easier to read on bugzilla

> Source/WebCore/ChangeLog:57
> +        (WebCore::GLTransportSurfaceClient::texture):
> +        (WebCore::GLTransportSurfaceClient::hasAlpha):
> +        Moved TransportSurface to EFL folder. Added a base class
> +        for TransportSurfaceClient.

Reasoning is always welcome!

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:196
> +    // Dont release current graphics surface until we have

Don't or Do not

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:198
> +    // prepared surface with requested size. This is to
> +    // avoid flashing during resize.

Lines are allowed to be longer, like 80-100 chars depending on current indent

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:207
> +    m_targetRect = IntRect(0, 0, m_size.width(), m_size.height());

Why not IntRect(IntPoint(), m_size)

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:211
> +
> +    if (m_size.isEmpty()) {
> +
> +        if (m_graphicsSurface) {

unneeded newline

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.h:63
> +    enum PendingOpeartion {

Spelling! OPERATION

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.h:64
> +        Default = 0x00, // No Pending Operation.

None = 0x00

> Source/WebCore/platform/graphics/surfaces/efl/GLTransportSurface.cpp:66
> +    m_rect = IntRect(0, 0, size.width(), size.height());

IntRect(IntPoint(), size)

> Source/WebCore/platform/graphics/surfaces/efl/GLTransportSurface.h:43
> +    // Creates a GL surface whose results can be transported
> +    // to the UI process for display.
> +    static PassOwnPtr<GLTransportSurface> createTransportSurface(const IntSize&, SurfaceAttributes = GLPlatformSurface::Default);

The third line is long, so no need to wrap that comment

[Kalyan]

Created attachment 193571 [details]
patchv3

[Kalyan]

(In reply to comment #3)
> (From update of attachment 192368 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192368&action=review
> > Source/WebCore/platform/graphics/surfaces/efl/GLTransportSurface.h:43
> > +    // Creates a GL surface whose results can be transported
> > +    // to the UI process for display.
> > +    static PassOwnPtr<GLTransportSurface> createTransportSurface(const IntSize&, SurfaceAttributes = GLPlatformSurface::Default);
> 
> The third line is long, so no need to wrap that comment

Done.

[Kenneth Rohde Christiansen]

Are you not a committer? then you can set cq+ and you should add yourself to committers.py

[Kalyan]

(In reply to comment #6)
> Are you not a committer? then you can set cq+ and you should add yourself to committers.py

Not yet. Still waiting to finish some of the formalities.

[WebKit Review Bot]

Comment on attachment 193571 [details]
patchv3

Clearing flags on attachment: 193571

Committed r146458: <http://trac.webkit.org/changeset/146458>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.