Bug 109988 – [WebGL][EFL] Refactor GraphicsContext3DPrivate to add support for SharedContext.

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Kalyan]

Currently we don't have any support for shared context. This would help with sharing GL objects without having to worry about the context state.

[Kalyan]

Created attachment 188722 [details]
patch

[Kalyan]

Still, one pending change is to move TransportSurface management from GraphicsContext3DPrivate to GraphicsSurface. This will be done in next patch.

[Kenneth Rohde Christiansen]

Comment on attachment 188722 [details]
patch

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

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:214
> +
> +        success = false;
> +    }

I think return false is more clear

> Source/WebCore/platform/graphics/opengl/GLPlatformContext.h:66
> +    virtual bool initialize(GLPlatformSurface*, PlatformContext = 0);

What is PlatformContext here? Why not a pointer? (I guess it is hiding that fact)

contextToReuse?

> Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h:56
> +    // The handle will be null if surface doesnot support

spelling

[Mikhail Pozdnyakov]

Comment on attachment 188722 [details]
patch

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

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:66
> +            goto initializationFailed;

looks like a bad idea to use 'goto' in C++ code.. Same effect can be reached using do { some breaks; } while(false).

[Kalyan]

Created attachment 188869 [details]
patchv2

[Kalyan]

(In reply to comment #3)
> (From update of attachment 188722 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=188722&action=review
> 
> I think return false is more clear

oops missed this one, will update a new patch taking this into account.

> What is PlatformContext here? Why not a pointer? (I guess it is hiding that fact)
> 
> contextToReuse?

PlatformContext is typedefed  to the respective GL context(glx, egl etc) in GLdefs.h

> > Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h:56
> > +    // The handle will be null if surface doesnot support
> spelling

fixed.

[Kalyan]

Comment on attachment 188869 [details]
patchv2

marking patch as obsolete, as it missed some of the review comments.

[Kalyan]

(In reply to comment #4)
> (From update of attachment 188722 [details])
> looks like a bad idea to use 'goto' in C++ code.. Same effect can be reached using do { some breaks; } while(false).

fixed

[Kalyan]

Created attachment 188875 [details]
patchv3

[Kalyan]

(In reply to comment #6)
> (In reply to comment #3)
> > (From update of attachment 188722 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=188722&action=review
> > 
> > I think return false is more clear
> 
> oops missed this one, will update a new patch taking this into account.
> 

done.

[WebKit Review Bot]

Comment on attachment 188875 [details]
patchv3

Clearing flags on attachment: 188875

Committed r143220: <http://trac.webkit.org/changeset/143220>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.