[Webkit-unassigned] [Bug 137684] Patch to build a wayland-only webkit/EFL(Without X11/Ecore_X)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=137684





--- Comment #5 from cjacker <jianzhong.huang at i-soft.com.cn>  2014-10-14 18:25:59 PST ---
(In reply to comment #4)
Thanks for your helpful reply.
comment in content.

I will try git codes and refind this patch.

> (From update of attachment 239789 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239789&action=review
> 
> First, thanks for the patch.
> Patch looks almost fine to me.
> 
> I didn't try to prepare wayland backend yet.
> I will check it more when I have it.
> 
> > ewebkitn/Source/cmake/OptionsEfl.cmake:128
> > +option(ENABLE_ECORE_X "Enable Ecore_X specific usage (cursor, bell)")
> > +
> > +option(ENABLE_WAYLAND_ONLY "Enable wayland only build.")
> > +if (ENABLE_WAYLAND_ONLY)
> > +    set(ENABLE_X11_TARGET OFF)
> > +    set(ENABLE_ECORE_X OFF)
> > +    set(ENABLE_NETSCAPE_PLUGIN_API OFF)
> > +else ()
> > +    set(ENABLE_ECORE_X ON)
> > +endif ()
> 
> In my opinion,
> We'd better to choose window system between X or WAYLAND.
> 
> Perhaps, like below. (I am bad with naming)
> set(WINDOW_SYSTEM "X" CACHE STRING "Choose window system for EWebKit (X or WAYLAND)")
> ...
> 

Good suggestion.

> In addition, I have one question.
> Doesn't we need to use ECORE_WAYLAND?
> 

It seems there is no direct dependency on ECORE_WAYLAND currently.
I think it should be checked to ensure EFL wayland backend enabled.

> > ewebkitn/Source/cmake/OptionsEfl.cmake:192
> > +    if (ENABLE_WAYLAND_ONLY)
> > +        set(ENABLE_X11_TARGET OFF)
> > +    else ()
> > +        set(ENABLE_X11_TARGET ON)
> > +    endif ()
> 
> It looks not necessary if we checked similar logic in above statement.
> 
> > ewebkitn/Source/cmake/OptionsEfl.cmake:248
> > +    if (ENABLE_WAYLAND_ONLY)
> > +        set(ENABLE_GLES2 ON)
> > +    endif ()
> 
> Should WAYLAND backend depend on this?
> 
> > ewebkitn/Source/cmake/OptionsEfl.cmake:264
> > +    if (ENABLE_WAYLAND_ONLY)
> > +        set(ENABLE_EGL ON)
> > +    endif ()
> 
> Ditto.

Above two defines will be enabled when WEBGL or WTF_USE_3D_GRAPHICS set to ON, because there is no libGL(glx) under wayland-only environment, these two option should be and MUST be enabled on wayland-only build.

for example, "Source/WebKit2/UIProcess/CoordinatedGraphics/WebView.cpp" and other uiprocess related codes depend on COORDINATED_GRAPHICS macro, it is controlbed by WTF_USE_3D_GRAPHICS option.

Currently, acceleration will be disabled automatically, since evas_gl_new will fail without libGL. it will fallback to cairo surface instead of GLSurface, I guess WEBGL did not work now(untest), But the cairo rendering works under wayland-only enviroment.(Thank god, at least, it works.)

> 
> > ewebkit2/Source/WebCore/platform/efl/EflScreenUtilities.cpp:69
> > +#ifdef HAVE_ECORE_X
> >  #include <Ecore_X.h>
> > +#endif
> 
> Usually, we moved conditional includes to the bottom of includes with empty line.
> 
> #include ...
> #include ...
> 
> #if XXX
> #include ...
> #endif
> 
> #if YYY
> #include ..
> #endif
> 
OK, looks more clear.

> > ewebkit2/Source/WebCore/css/ViewportStyleResolver.cpp:43
> > +//for undefined renderStyle(), it's a inline function in header.
> > +#include "NodeRenderStyle.h"
> 
> I think that this is not related to EFL port.
> So I think that we should make different bug for this.

Yes, I found this commit in Git codes(No more bug report needed), It should be clang issue.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.