Bug 225867

Summary: [GTK] [2.33.1] Fails to build when HAVE_OPENGL_ES_3 is on
Product: WebKit Reporter: Alberto Garcia <berto>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, bugs-noreply, dino, ews-watchlist, graouts, kondapallykalyan, mcatanzaro
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch v2 mcatanzaro: review+, berto: commit-queue-

Description Alberto Garcia 2021-05-17 06:41:10 PDT
After r276368 WebKitGTK fails to build if HAVE_OPENGL_ES_3 is on:

FAILED: Source/WebCore/CMakeFiles/WebCore.dir/platform/graphics/opengl/GraphicsContextGLOpenGLCommon.cpp.o 
In file included from ../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGLCommon.cpp:61:
../Source/WebCore/platform/graphics/OpenGLESShims.h:48: warning: "GL_COLOR_ATTACHMENT0_EXT" redefined
   48 | #define GL_COLOR_ATTACHMENT0_EXT GL_COLOR_ATTACHMENT0
      | 
In file included from ../Source/WebCore/platform/graphics/opengl/ExtensionsGLOpenGLES.h:37,
                 from ../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGLCommon.cpp:35:
/usr/include/GLES2/gl2ext.h:1336: note: this is the location of the previous definition
 1336 | #define GL_COLOR_ATTACHMENT0_EXT          0x8CE0
      | 
../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGLCommon.cpp: In member function ‘virtual void WebCore::GraphicsContextGLOpenGL::copyBufferSubData(GCGLenum, GCGLenum, GCGLintptr, GCGLintptr, GCGLsizeiptr)’:
../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGLCommon.cpp:2206:7: error: ‘::glCopyBufferSubData’ has not been declared; did you mean ‘copyBufferSubData’?
 2206 |     ::glCopyBufferSubData(readTarget, writeTarget, readOffset, writeOffset, size);
      |       ^~~~~~~~~~~~~~~~~~~
      |       copyBufferSubData
../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGLCommon.cpp: In member function ‘virtual void WebCore::GraphicsContextGLOpenGL::getBufferSubData(GCGLenum, GCGLintptr, GCGLSpan<void>)’:
../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGLCommon.cpp:2219:23: error: ‘::glMapBufferRange’ has not been declared; did you mean ‘bindBufferRange’?
 2219 |     GCGLvoid* ptr = ::glMapBufferRange(target, offset, data.bufSize, GraphicsContextGL::MAP_READ_BIT);
      |                       ^~~~~~~~~~~~~~~~
      |                       bindBufferRange
Comment 1 Alberto Garcia 2021-05-17 08:22:48 PDT
Created attachment 428830 [details]
Patch
Comment 2 Michael Catanzaro 2021-05-17 09:03:05 PDT
Comment on attachment 428830 [details]
Patch

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

> Source/WebCore/platform/graphics/opengl/ExtensionsGLOpenGLES.h:40
> +#if HAVE(OPENGL_ES_3)
> +#include <GLES3/gl3.h>
> +#endif // HAVE(OPENGL_ES_3)
>  #include <GLES2/gl2.h>
>  #include <GLES2/gl2ext.h>

We want *both* the GLES3 header *and* the GLES2 headers...?
Comment 3 Adrian Perez 2021-05-17 09:18:29 PDT
Comment on attachment 428830 [details]
Patch

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

>> Source/WebCore/platform/graphics/opengl/ExtensionsGLOpenGLES.h:40
>>  #include <GLES2/gl2ext.h>
> 
> We want *both* the GLES3 header *and* the GLES2 headers...?

IIUC any GLES3 implementation needs to *also* be a GLES2 implementation, and
including both should be harmless. As a matter of fact libGLESv2.so is expected
to provide both the GLES2 + GLES3 symbols, too :)
Comment 4 Alberto Garcia 2021-05-17 14:15:13 PDT
Created attachment 428869 [details]
Patch v2

Actually gl3.h has the entire contents of gl2.h plus the extra definitions, so there's no reason to include both.

Here's an alternative patch.

(gl3ext.h is currently empty so I'm not including it)
Comment 5 Alberto Garcia 2021-05-17 14:45:47 PDT
(In reply to Alberto Garcia from comment #4)
> Created attachment 428869 [details]
> Patch v2
> 
> Actually gl3.h has the entire contents of gl2.h plus the extra definitions,
> so there's no reason to include both.
> 
> Here's an alternative patch.

Actually this one doesn't work because WebKit uses symbols from 
gl2ext.h that are not defined elsewhere, so I'm committing the original patch.
Comment 6 EWS 2021-05-17 15:55:21 PDT
Committed r277609 (237826@main): <https://commits.webkit.org/237826@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 428830 [details].