Bug 43754

Summary: Use Chromium OpenGL bindings rather than GLEW in WebGraphicsContextDefaultImpl
Product: WebKit Reporter: Kenneth Russell <kbr>
Component: WebGLAssignee: Kenneth Russell <kbr>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, eric, gman, vangelis, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 43811    
Attachments:
Description Flags
Patch dglazkov: review+, kbr: commit-queue-

Description Kenneth Russell 2010-08-09 15:56:33 PDT
In the Chromium WebGL port, the "default" implementation of WebGraphicsContext3D calls OpenGL directly, currently via GLEW. We need to switch this to use Chromium's OpenGL bindings rather than GLEW so that Chromium's test shell can use the OSMesa renderer. This will allow WebGL tests to be run on the build bots all of the time.
Comment 1 Kenneth Russell 2010-08-10 11:14:00 PDT
Created attachment 64032 [details]
Patch

From the ChangeLog:

Deleted per-platform OpenGL context management code, now abstracted via GLContext. Built and tested in Chromium with --in-process-webgl flag.

Thanks to Al Patrick for the initial patch.
Comment 2 Kenneth Russell 2010-08-10 11:14:01 PDT
Created attachment 64033


From the ChangeLog:

Deleted per-platform OpenGL context management code, now abstracted via GLContext. Built and tested in Chromium with --in-process-webgl flag.

Thanks to Al Patrick for the initial patch.
Comment 3 WebKit Review Bot 2010-08-10 11:18:39 PDT
Attachment 64032 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:35:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:72:  Use 0 instead of NULL.  [readability/null] [5]
WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:107:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 3 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Dimitri Glazkov (Google) 2010-08-10 11:23:46 PDT
Comment on attachment 64032 [details]
Patch

r=me, after style fixes:

WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:72
 +      , m_glContext(NULL)
(0)

WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:98
 +          delete m_glContext;
Maybe make OwnPtr?

WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:107
 +      m_glContext = gfx::GLContext::CreateOffscreenGLContext(NULL);
(0)

WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:427
 +      // TODO(apatrick): OpenGL ES 2 does not support GL_BGRA so this fails when
FIXME:
Comment 5 Kenneth Russell 2010-08-10 14:24:41 PDT
(In reply to comment #4)
> (From update of attachment 64032 [details])
> r=me, after style fixes:
> 
> WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:72
>  +      , m_glContext(NULL)
> (0)
> 
> WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:98
>  +          delete m_glContext;
> Maybe make OwnPtr?
> 
> WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:107
>  +      m_glContext = gfx::GLContext::CreateOffscreenGLContext(NULL);
> (0)
> 
> WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:427
>  +      // TODO(apatrick): OpenGL ES 2 does not support GL_BGRA so this fails when
> FIXME:

Will fix these issues before commit.
Comment 6 Kenneth Russell 2010-08-10 14:27:44 PDT
Committed r65093: <http://trac.webkit.org/changeset/65093>
Comment 7 WebKit Review Bot 2010-08-10 14:41:12 PDT
http://trac.webkit.org/changeset/65093 might have broken Chromium Linux Release