Bug 109988 - [WebGL][EFL] Refactor GraphicsContext3DPrivate to add support for SharedContext.
Summary: [WebGL][EFL] Refactor GraphicsContext3DPrivate to add support for SharedContext.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Kalyan
URL:
Keywords:
Depends on:
Blocks: 106508 109435
  Show dependency treegraph
 
Reported: 2013-02-15 16:57 PST by Kalyan
Modified: 2013-02-18 08:17 PST (History)
9 users (show)

See Also:


Attachments
patch (39.40 KB, patch)
2013-02-16 12:30 PST, Kalyan
no flags Details | Formatted Diff | Diff
patchv2 (39.84 KB, patch)
2013-02-18 06:12 PST, Kalyan
no flags Details | Formatted Diff | Diff
patchv3 (39.83 KB, patch)
2013-02-18 06:50 PST, Kalyan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kalyan 2013-02-15 16:57:51 PST
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.
Comment 1 Kalyan 2013-02-16 12:30:59 PST
Created attachment 188722 [details]
patch
Comment 2 Kalyan 2013-02-16 12:40:27 PST
Still, one pending change is to move TransportSurface management from GraphicsContext3DPrivate to GraphicsSurface. This will be done in next patch.
Comment 3 Kenneth Rohde Christiansen 2013-02-18 02:58:59 PST
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
Comment 4 Mikhail Pozdnyakov 2013-02-18 04:37:12 PST
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).
Comment 5 Kalyan 2013-02-18 06:12:09 PST
Created attachment 188869 [details]
patchv2
Comment 6 Kalyan 2013-02-18 06:15:16 PST
(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.
Comment 7 Kalyan 2013-02-18 06:16:15 PST
Comment on attachment 188869 [details]
patchv2

marking patch as obsolete, as it missed some of the review comments.
Comment 8 Kalyan 2013-02-18 06:16:48 PST
(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
Comment 9 Kalyan 2013-02-18 06:50:17 PST
Created attachment 188875 [details]
patchv3
Comment 10 Kalyan 2013-02-18 06:51:07 PST
(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.
Comment 11 WebKit Review Bot 2013-02-18 08:17:36 PST
Comment on attachment 188875 [details]
patchv3

Clearing flags on attachment: 188875

Committed r143220: <http://trac.webkit.org/changeset/143220>
Comment 12 WebKit Review Bot 2013-02-18 08:17:40 PST
All reviewed patches have been landed.  Closing bug.