Bug 103610

Summary:

[EFL] libwebcore_efl.a fail to link when enable webgl

Product:

WebKit

Reporter:

Halton Huo <halton.huo>

Component:

WebKit EFL

Assignee:

Halton Huo <halton.huo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, gyuyoung.kim, kalyan.kondapally, lucas.de.marchi, rakuco, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

77219

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Halton Huo

Reported 2012-11-29 00:42:18 PST

Build trunk@136087 on Ubuntu 12.10 64 bit, $./Tools/Scripts/build-webkit --efl --cmakearg="-DENABLE_WEBGL=ON" [...] ../../../lib/libwebcore_efl.a(GLXSurface.cpp.o):GLXSurface.cpp:function WebCore::GLXTransportSurface::destroy(): error: undefined reference to 'XDestroyWindow' ../../../lib/libwebcore_efl.a(GLXSurface.cpp.o):GLXSurface.cpp:function WebCore::GLXPBuffer::configuration(): error: undefined reference to 'XFree' ../../../lib/libwebcore_efl.a(GLXSurface.cpp.o):GLXSurface.cpp:function WebCore::GLXPBuffer::configuration(): error: undefined reference to 'XOpenDisplay' ../../../lib/libwebcore_efl.a(GLXSurface.cpp.o):GLXSurface.cpp:function WebCore::GLXTransportSurface::configuration(): error: undefined reference to 'XRenderQueryExtension' ../../../lib/libwebcore_efl.a(GLXSurface.cpp.o):GLXSurface.cpp:function WebCore::GLXTransportSurface::configuration(): error: undefined reference to 'XRenderFindVisualFormat' [...] ../../../lib/libwebcore_efl.a(GraphicsSurfaceGLX.cpp.o):GraphicsSurfaceGLX.cpp:function WebCore::GraphicsSurface::platformGetTextureID(): error: undefined reference to 'XDefaultScreen' ../../../lib/libwebcore_efl.a(GraphicsSurfaceGLX.cpp.o):GraphicsSurfaceGLX.cpp:function WebCore::GraphicsSurface::platformGetTextureID(): error: undefined reference to 'XCompositeNameWindowPixmap' [...] It shows me X11, Xrender and XSomposite libs are not linked with libwebcore, will submit a patch later.

Attachments
Patch (1.42 KB, patch) 2012-11-29 01:00 PST, Halton Huo	no flags	Details Formatted Diff Diff
Patch (1.50 KB, patch) 2012-11-29 02:35 PST, Halton Huo	no flags	Details Formatted Diff Diff
Patch (1.50 KB, patch) 2012-11-29 02:58 PST, Halton Huo	no flags	Details Formatted Diff Diff
Patch (1.50 KB, patch) 2012-11-29 19:19 PST, Halton Huo	no flags	Details Formatted Diff Diff
Patch (1.49 KB, patch) 2012-11-30 00:09 PST, Halton Huo	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Halton Huo

Comment 1 2012-11-29 01:00:34 PST

Created attachment 176670 [details] Patch

Gyuyoung Kim

Comment 2 2012-11-29 01:26:47 PST

Comment on attachment 176670 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176670&action=review > Source/WebCore/ChangeLog:10 > + bin/DumpRenderTree What is this ? > Source/WebCore/PlatformEfl.cmake:302 > + "-lXcomposite -lXrender" IMO, it would be good if you link this libraries from FindXXX.cmake file or macro.

Halton Huo

Comment 3 2012-11-29 02:27:22 PST

(In reply to comment #2) > (From update of attachment 176670 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=176670&action=review > > > Source/WebCore/ChangeLog:10 > > + bin/DumpRenderTree > > What is this ? This link error happens when link libwebcore_efl to DumpRenderTree, I just check whether this binary generated and can be run with my patch. I'll update the comment with more specific word. > > > Source/WebCore/PlatformEfl.cmake:302 > > + "-lXcomposite -lXrender" > > IMO, it would be good if you link this libraries from FindXXX.cmake file or macro. I do this because WebCore.pri add those two libs directly. 226 use?(GRAPHICS_SURFACE) { 227 mac: LIBS += -framework IOSurface -framework CoreFoundation 228 linux-*: { 229 LIBS += -lXcomposite -lXrender 230 CONFIG *= x11 231 } 232 } After going through the FindX11.cmake, I found X11_Xcomposite_LIB and X11_Xrandr_LIB are defined, I'll use them and update the patch.

Halton Huo

Comment 4 2012-11-29 02:35:55 PST

Created attachment 176683 [details] Patch

Gyuyoung Kim

Comment 5 2012-11-29 02:44:30 PST

Comment on attachment 176683 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176683&action=review > Source/WebCore/ChangeLog:6 > + Reviewed by Gyuyoung Kim. I don't set r+ yet. So, remove my name here.

Halton Huo

Comment 6 2012-11-29 02:58:00 PST

Created attachment 176688 [details] Patch

Yael

Comment 7 2012-11-29 15:24:32 PST

Comment on attachment 176688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176688&action=review > Source/WebCore/ChangeLog:8 > + libwebcore_efl.a should link to X11, Xcomposite and Xrender libraries when WTF_USE_3D_GRAPHICS=1 It is ENABLE_WEBGL, not WTF_USE_3D_GRAPHICS

Yael

Comment 8 2012-11-29 15:25:07 PST

*** Bug 103107 has been marked as a duplicate of this bug. ***

Halton Huo

Comment 9 2012-11-29 19:19:18 PST

Created attachment 176887 [details] Patch

Gyuyoung Kim

Comment 10 2012-11-29 19:33:33 PST

Comment on attachment 176887 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176887&action=review > Source/WebCore/ChangeLog:10 > + Test: bin/DumpRenderTree are generated and can be run. It looks this comment is meaningless. Generally, we have been written test case place in LayoutTest.

Halton Huo

Comment 11 2012-11-29 23:46:07 PST

(In reply to comment #10) > > Source/WebCore/ChangeLog:10 > > + Test: bin/DumpRenderTree are generated and can be run. > > It looks this comment is meaningless. Generally, we have been written test case place in LayoutTest. Is that acceptable that I just remove the "Test:" line?

Chris Dumez

Comment 12 2012-11-29 23:53:27 PST

Comment on attachment 176887 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176887&action=review >> Source/WebCore/ChangeLog:10 >> + Test: bin/DumpRenderTree are generated and can be run. > > It looks this comment is meaningless. Generally, we have been written test case place in LayoutTest. Replace with something like "No new tests, no behavior change for layout tests." This is common practice.

Halton Huo

Comment 13 2012-11-30 00:09:44 PST

Created attachment 176909 [details] Patch

Chris Dumez

Comment 14 2012-11-30 00:10:56 PST

You already got r+ from Gyuyoung and only did a minor change. Do not request r? flag again, just cq?

Halton Huo

Comment 15 2012-11-30 00:25:56 PST

(In reply to comment #14) > You already got r+ from Gyuyoung and only did a minor change. Do not request r? flag again, just cq? Thanks for let me know. In this case should I set the review to null or +?

Gyuyoung Kim

Comment 16 2012-11-30 00:28:45 PST

Chris Dumez

Comment 17 2012-11-30 00:29:09 PST

(In reply to comment #15) > (In reply to comment #14) > > You already got r+ from Gyuyoung and only did a minor change. Do not request r? flag again, just cq? > Thanks for let me know. In this case should I set the review to null or +? Don't change anything here, the patch is on its way to land. But next time, just let leave r flag empty and set only cq? (provided that you already got r+ on a previous patch that you did minimal changes to and provided that you updated the reviewer name in the Changelog).

WebKit Review Bot

Comment 18 2012-11-30 00:55:30 PST

Comment on attachment 176909 [details] Patch Clearing flags on attachment: 176909 Committed r136212: <http://trac.webkit.org/changeset/136212>

WebKit Review Bot

Comment 19 2012-11-30 00:55:35 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.