Add necessary include directories in WebKit2
Created attachment 160918 [details] Patch
Attachment 160918 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/CMakeLists.txt', u'Source/W..." exit_code: 1 Source/WebKit2/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] Source/WebKit2/ChangeLog:10: Line contains tab character. [whitespace/tab] [5] Source/WebKit2/ChangeLog:11: Line contains tab character. [whitespace/tab] [5] Source/WebKit2/ChangeLog:12: Line contains tab character. [whitespace/tab] [5] Source/WebKit2/ChangeLog:13: Line contains tab character. [whitespace/tab] [5] Source/WebKit2/ChangeLog:15: Line contains tab character. [whitespace/tab] [5] Source/WebKit2/ChangeLog:16: Line contains tab character. [whitespace/tab] [5] Total errors found: 7 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 160920 [details] Patch
Comment on attachment 160920 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160920&action=review > Source/WebKit2/ChangeLog:8 > + Commit f9d4da... modify the TextureMapperGL.h to include I think that you may want to replace the git sha1 with the SVN revision here.
*** Bug 95165 has been marked as a duplicate of this bug. ***
Created attachment 161118 [details] Patch
(In reply to comment #4) > (From update of attachment 160920 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=160920&action=review > > > Source/WebKit2/ChangeLog:8 > > + Commit f9d4da... modify the TextureMapperGL.h to include > > I think that you may want to replace the git sha1 with the SVN revision here. I modified the change log and resubmitted the patch. Any comment?
LGTM. I want this patch and another patch that adds `#include "WebPreferences.h"` in ewk_view.cpp.
zhigang: would you please ask hausmann for r+?
hausmann, would you please help to review this patch? thx.
(In reply to comment #10) > hausmann, > would you please help to review this patch? thx. Hopefully, we can get r+. Or, you can rquest cq? after changing "Reviewed by NOBODY (OOPS!)." to "Unreviewed build fix" because this is to fix build break.
(In reply to comment #11) > (In reply to comment #10) > > hausmann, > > would you please help to review this patch? thx. > > Hopefully, we can get r+. > > Or, you can rquest cq? after changing "Reviewed by NOBODY (OOPS!)." to "Unreviewed build fix" because this is to fix build break. I will have to do that once i back to work on Mon, as I am not at my development machine now.
Comment on attachment 161118 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161118&action=review > Source/WebKit2/CMakeLists.txt:115 > + "${THIRDPARTY_DIR}/ANGLE/include" I ran into this just a few days ago. Adding ANGLE/include to your include search path means that if you are compiling your code in a platform where EGL and GLES2 are the chosen system wide GL APIs, then an #include <EGL/egl.h> in WebKit2 will end up including ANGLE's EGL header file instead of your system wide one. The same applies to GLES2 inclusions. Do you really need this include or can you limit this change to just the include/GLSLANG line below?
Created attachment 163031 [details] Patch
Comment on attachment 163031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163031&action=review You don't need to request r? for unreviewed build fix. > Source/WebKit2/ChangeLog:3 > + Add necessary include directories in WebKit2 It is good to add [EFL][WK2] prefix to bug title.
Comment on attachment 163031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163031&action=review > Source/WebKit2/CMakeLists.txt:114 > +IF (WTF_USE_3D_GRAPHICS) I think WebKit2/PlatformEfl.cmake is proper place because this build break occurs at EFL port only now.
(In reply to comment #16) > (From update of attachment 163031 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=163031&action=review > > > Source/WebKit2/CMakeLists.txt:114 > > +IF (WTF_USE_3D_GRAPHICS) > > I think WebKit2/PlatformEfl.cmake is proper place because this build break occurs at EFL port only now. Hmm, IMO, it may be not a EFL port only problem even it just triggers problem on EFL platform now. The root cause is that when TEXTURE_MAPPER is enabled, some source files in WebKit2 include TextureMapperGL.h --> GraphicsContext3D -->trigger build problem. And as I know, TEXTURE_MAPPER is not EFL only, right? And those files which include TextureMapperGL.h are also not all for EFL platform only, for example the ShareableSurface.cpp. Any further comments?
Comment on attachment 163031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163031&action=review >>> Source/WebKit2/CMakeLists.txt:114 >>> +IF (WTF_USE_3D_GRAPHICS) >> >> I think WebKit2/PlatformEfl.cmake is proper place because this build break occurs at EFL port only now. > > Hmm, IMO, it may be not a EFL port only problem even it just triggers problem on EFL platform now. The root cause is that when TEXTURE_MAPPER is enabled, some source files in WebKit2 include TextureMapperGL.h --> GraphicsContext3D -->trigger build problem. And as I know, TEXTURE_MAPPER is not EFL only, right? And those files which include TextureMapperGL.h are also not all for EFL platform only, for example the ShareableSurface.cpp. > > Any further comments? Of course, it seems to me all ports (EFL, blackberry and wince) will need to include "GLSLANG" directory if they need to use WEBGL(3D_GRAPHICS) But, blackberry and wince ports don't start to support WK2 yet. So, I would remain to add this to EFL private file until they need to use the "GLSLANG". Because, there will may something stuff further. In WK1 case, blackberry port also include this directory in PlatformBlackBerry.cmake. http://trac.webkit.org/browser/trunk/Source/WebKit/PlatformBlackBerry.cmake#L120
(In reply to comment #18) > (From update of attachment 163031 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=163031&action=review > > >>> Source/WebKit2/CMakeLists.txt:114 > >>> +IF (WTF_USE_3D_GRAPHICS) > >> > >> I think WebKit2/PlatformEfl.cmake is proper place because this build break occurs at EFL port only now. > > > > Hmm, IMO, it may be not a EFL port only problem even it just triggers problem on EFL platform now. The root cause is that when TEXTURE_MAPPER is enabled, some source files in WebKit2 include TextureMapperGL.h --> GraphicsContext3D -->trigger build problem. And as I know, TEXTURE_MAPPER is not EFL only, right? And those files which include TextureMapperGL.h are also not all for EFL platform only, for example the ShareableSurface.cpp. > > > > Any further comments? > > Of course, it seems to me all ports (EFL, blackberry and wince) will need to include "GLSLANG" directory if they need to use WEBGL(3D_GRAPHICS) > But, blackberry and wince ports don't start to support WK2 yet. So, I would remain to add this to EFL private file until they need to use the "GLSLANG". Because, there will may something stuff further. > > In WK1 case, blackberry port also include this directory in PlatformBlackBerry.cmake. > http://trac.webkit.org/browser/trunk/Source/WebKit/PlatformBlackBerry.cmake#L120 I agree with you now. Will submit a new version soon.
Created attachment 163279 [details] Patch
Comment on attachment 163279 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163279&action=review > Source/WebKit2/ChangeLog:3 > + Add necessary include directories in WebKit2 Please add [EFL][WK2] prefix to here as well.
Created attachment 163283 [details] Patch
Comment on attachment 163283 [details] Patch LGTM, thank. :-)
Comment on attachment 163283 [details] Patch Clearing flags on attachment: 163283 Committed r128154: <http://trac.webkit.org/changeset/128154>
All reviewed patches have been landed. Closing bug.