Bug 79169

Summary: [Qt][WK2] Enable WebGL for WebKit2-Qt Linux/GLX
Product: WebKit Reporter: Noam Rosenthal <noam>
Component: WebGLAssignee: Zeno Albisser <zeno>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: hausmann, jesus, lauro.neto, rafael.lobo, zeno
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 84030, 86214, 90881, 93252    
Bug Blocks:    
Attachments:
Description Flags
patch for review. noam: review-

Description Noam Rosenthal 2012-02-21 17:30:07 PST
see subject.
Comment 1 Zeno Albisser 2012-02-22 01:58:30 PST
noam: i'm reassigning this one to myself, i guess that is fine with you.
i already started to look into why webgl stopped working with QtTestBrowser. - As a starting point.
Comment 2 Noam Rosenthal 2012-02-22 06:00:07 PST
(In reply to comment #1)
> noam: i'm reassigning this one to myself, i guess that is fine with you.
> i already started to look into why webgl stopped working with QtTestBrowser. - As a starting point.

Great!
Comment 3 Zeno Albisser 2012-07-13 03:16:25 PDT
Created attachment 152206 [details]
patch for review.
Comment 4 Simon Hausmann 2012-07-13 04:30:32 PDT
Comment on attachment 152206 [details]
patch for review.

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

> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:88
> +#if OS(DARWIN)
>      RefPtr<GraphicsSurface> m_frontBufferGraphicsSurface;
>      RefPtr<GraphicsSurface> m_backBufferGraphicsSurface;
> +#else
> +    RefPtr<GraphicsSurface> m_graphicsSurface;
> +#endif

I supposed that there are two users of GraphicsSurface: Those who need double-buffering and those who don't (they write to the surface only once and then are done with it).

I think should be encapsulated in GraphicsSurface itself (for example having a front and a back PlatformGraphicsSurface) and be a parameter of the creating API. Requiring OS(DARWIN) on the
_caller_ side feels wrong.

Is that the direction you plan to take after this patch or in another iteration?

> Source/WebCore/platform/graphics/texmap/TextureMapperBackingStore.cpp:71
> +    static_cast<TextureMapperGL*>(textureMapper)->drawTexture(m_textureID, 0, m_graphicsSurfaceSize, targetRect, adjustedTransform, opacity, mask);

Similarly here, perhaps drawTexture should be a function of GraphicsSurface that depending on the platform implementation uses drawTextureRectangleARGB or drawTexture of a TextureMapperGL parameter.
Comment 5 Zeno Albisser 2012-07-13 04:59:43 PDT
(In reply to comment #4)
> I supposed that there are two users of GraphicsSurface: Those who need double-buffering and those who don't (they write to the surface only once and then are done with it).
> 
> I think should be encapsulated in GraphicsSurface itself (for example having a front and a back PlatformGraphicsSurface) and be a parameter of the creating API. Requiring OS(DARWIN) on the
> _caller_ side feels wrong.
> 
> Is that the direction you plan to take after this patch or in another iteration?
That's exactly what am working on right now.
Comment 6 Noam Rosenthal 2012-07-13 08:10:25 PDT
Comment on attachment 152206 [details]
patch for review.

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

A bit too #ifdeffy... i'd rather it would be capability based and not OS based.

>> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:88
>> +#endif
> 
> I supposed that there are two users of GraphicsSurface: Those who need double-buffering and those who don't (they write to the surface only once and then are done with it).
> 
> I think should be encapsulated in GraphicsSurface itself (for example having a front and a back PlatformGraphicsSurface) and be a parameter of the creating API. Requiring OS(DARWIN) on the
> _caller_ side feels wrong.
> 
> Is that the direction you plan to take after this patch or in another iteration?

I'd rather this be done with a runtime flag like GraphicsSurface::isDoubleBufferred, even now. Then we can keep m_front... and initialize the back one only if double buffered is not supported.
Comment 7 Zeno Albisser 2012-10-05 11:17:04 PDT

*** This bug has been marked as a duplicate of bug 90881 ***