New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Converted Unix Window implementation from XCB back to Xlib. #1138

Merged
merged 1 commit into from Sep 29, 2016

Conversation

Projects
None yet
6 participants
@binary1248
Member

binary1248 commented Aug 24, 2016

After almost 2 years of poking around XCB, it is time to declare defeat and go back to good old stable Xlib.

In retrospect, there weren't really many if any advantages for us in switching from Xlib to XCB in the first place. The main advantage users of XCB are supposed to have comes from its non-blocking nature. This might make sense in certain applications, however the way SFML interacts with the X server completely counteracts this usage pattern. There is simply no performance difference between using Xlib and XCB for SFML. The second advantage users of XCB are supposed to have is "less legacy (buggy) code" in its implementation. Being a thin (autogenerated) wrapper on top of the X wire protocol, I think it is kind of obvious: If there is no implementation to speak of, there also can't be that many bugs.

We take for granted how many other things Xlib actually does for us on the side. Those things were supposed to be supported in XCB at some point as well, but considering that it is an open source project and Wayland is arriving faster than many anticipated, one can understand the lack of motivation to continue development on XCB.

Even when using XCB, SFML still has to make use of Xlib to interact with GLX since that is still the primary API that the graphics drivers expose to this day. Mixing Xlib and XCB is a nightmare as well, which is why I already reverted the event processing back to using Xlib events a while ago.

The Unix sfml-window implementation could be considered relatively stable up to 2.2. Most of the instability came in 2.3 when we switched over to XCB. This PR seeks to restore the stability of 2.2 so that we can focus our efforts on other more pressing issues and remain an attractive library for users on Unix platforms. Once Wayland support becomes ubiquitous, we can consider supporting it in parallel to Xlib.

Fixes #1089 and possibly other bugs (requires re-evaluation if this gets accepted).

@achpile

This comment has been minimized.

Show comment
Hide comment
@achpile

achpile Aug 31, 2016

Thank you, Binary!!! Your branch helped me again!
(first time was 3 years ago, it was about ALContext)

PS: This fixes #921

Ubuntu 14.04 (XFCE4) 64bit

achpile commented Aug 31, 2016

Thank you, Binary!!! Your branch helped me again!
(first time was 3 years ago, it was about ALContext)

PS: This fixes #921

Ubuntu 14.04 (XFCE4) 64bit

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Sep 3, 2016

Member

Once this is merged, #827 will need rebasing and the linux implementation can be started.

Member

mantognini commented Sep 3, 2016

Once this is merged, #827 will need rebasing and the linux implementation can be started.

@eXpl0it3r eXpl0it3r added this to the 2.4.1 milestone Sep 12, 2016

@oprypin

This comment has been minimized.

Show comment
Hide comment
@oprypin

oprypin Sep 12, 2016

Contributor

Works for me. I notice no change.
(Arch Linux, NVIDIA, KDE)

I see that in ldd libxcb-* libs are not used anymore, but libxcb still is.

Contributor

oprypin commented Sep 12, 2016

Works for me. I notice no change.
(Arch Linux, NVIDIA, KDE)

I see that in ldd libxcb-* libs are not used anymore, but libxcb still is.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Sep 12, 2016

Member

I see that in ldd libxcb-* libs are not used anymore, but libxcb still is.

Did you rerun CMake as well?

Member

eXpl0it3r commented Sep 12, 2016

I see that in ldd libxcb-* libs are not used anymore, but libxcb still is.

Did you rerun CMake as well?

@oprypin

This comment has been minimized.

Show comment
Hide comment
@oprypin

oprypin Sep 12, 2016

Contributor

Did you rerun CMake as well?

Sure. I think the change in the list of referenced libraries confirms that.

Contributor

oprypin commented Sep 12, 2016

Did you rerun CMake as well?

Sure. I think the change in the list of referenced libraries confirms that.

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Sep 18, 2016

Member

Did you rerun CMake as well?

Sure. I think the change in the list of referenced libraries confirms that.

That's completely normal, since XCB will be loaded (or even injected?) by some X11 stuff. It's showing up in that list due to the way LDD works.

Running ldd lib/libsfml-window.so:

    linux-vdso.so.1 =>  (0x00007ffd7e2ec000)
    libsfml-system.so.2.4 => /home/mario/git/SFML/lib/libsfml-system.so.2.4 (0x00007fee94d4f000)
    libX11.so.6 => /usr/lib/x86_64-linux-gnu/libX11.so.6 (0x00007fee94a06000)
    libXrandr.so.2 => /usr/lib/x86_64-linux-gnu/libXrandr.so.2 (0x00007fee947fa000)
    libudev.so.1 => /lib/x86_64-linux-gnu/libudev.so.1 (0x00007fee947da000)
    libGL.so.1 => /usr/lib/x86_64-linux-gnu/mesa/libGL.so.1 (0x00007fee94569000)
    libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fee9434b000)
    libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fee93fc9000)
    libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fee93db3000)
    libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fee939e9000)
    librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007fee937e1000)
    libxcb.so.1 => /usr/lib/x86_64-linux-gnu/libxcb.so.1 (0x00007fee935bf000)
    libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fee933ba000)
    libXext.so.6 => /usr/lib/x86_64-linux-gnu/libXext.so.6 (0x00007fee931a8000)
    libXrender.so.1 => /usr/lib/x86_64-linux-gnu/libXrender.so.1 (0x00007fee92f9e000)
    /lib64/ld-linux-x86-64.so.2 (0x00005591d8db2000)
    libexpat.so.1 => /lib/x86_64-linux-gnu/libexpat.so.1 (0x00007fee92d74000)
    libxcb-dri3.so.0 => /usr/lib/x86_64-linux-gnu/libxcb-dri3.so.0 (0x00007fee92b71000)
    libxcb-present.so.0 => /usr/lib/x86_64-linux-gnu/libxcb-present.so.0 (0x00007fee9296e000)
    libxcb-sync.so.1 => /usr/lib/x86_64-linux-gnu/libxcb-sync.so.1 (0x00007fee92766000)
    libxshmfence.so.1 => /usr/lib/x86_64-linux-gnu/libxshmfence.so.1 (0x00007fee92563000)
    libglapi.so.0 => /usr/lib/x86_64-linux-gnu/libglapi.so.0 (0x00007fee92335000)
    libXdamage.so.1 => /usr/lib/x86_64-linux-gnu/libXdamage.so.1 (0x00007fee92131000)
    libXfixes.so.3 => /usr/lib/x86_64-linux-gnu/libXfixes.so.3 (0x00007fee91f2b000)
    libX11-xcb.so.1 => /usr/lib/x86_64-linux-gnu/libX11-xcb.so.1 (0x00007fee91d29000)
    libxcb-glx.so.0 => /usr/lib/x86_64-linux-gnu/libxcb-glx.so.0 (0x00007fee91b0f000)
    libxcb-dri2.so.0 => /usr/lib/x86_64-linux-gnu/libxcb-dri2.so.0 (0x00007fee9190a000)
    libXxf86vm.so.1 => /usr/lib/x86_64-linux-gnu/libXxf86vm.so.1 (0x00007fee91704000)
    libdrm.so.2 => /usr/lib/x86_64-linux-gnu/libdrm.so.2 (0x00007fee914f4000)
    libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fee911eb000)
    libXau.so.6 => /usr/lib/x86_64-linux-gnu/libXau.so.6 (0x00007fee90fe6000)
    libXdmcp.so.6 => /usr/lib/x86_64-linux-gnu/libXdmcp.so.6 (0x00007fee90de0000)

But you can also read that information directly from the binary using readelf -b lib/libsfml-window.so | grep NEEDED:

0x0000000000000001 (NEEDED)             Shared library: [libsfml-system.so.2.4]
0x0000000000000001 (NEEDED)             Shared library: [libX11.so.6]
0x0000000000000001 (NEEDED)             Shared library: [libXrandr.so.2]
0x0000000000000001 (NEEDED)             Shared library: [libudev.so.1]
0x0000000000000001 (NEEDED)             Shared library: [libGL.so.1]
0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]

As you can see, everything is fine, no direct reference to XCB anymore. :)

Member

MarioLiebisch commented Sep 18, 2016

Did you rerun CMake as well?

Sure. I think the change in the list of referenced libraries confirms that.

That's completely normal, since XCB will be loaded (or even injected?) by some X11 stuff. It's showing up in that list due to the way LDD works.

Running ldd lib/libsfml-window.so:

    linux-vdso.so.1 =>  (0x00007ffd7e2ec000)
    libsfml-system.so.2.4 => /home/mario/git/SFML/lib/libsfml-system.so.2.4 (0x00007fee94d4f000)
    libX11.so.6 => /usr/lib/x86_64-linux-gnu/libX11.so.6 (0x00007fee94a06000)
    libXrandr.so.2 => /usr/lib/x86_64-linux-gnu/libXrandr.so.2 (0x00007fee947fa000)
    libudev.so.1 => /lib/x86_64-linux-gnu/libudev.so.1 (0x00007fee947da000)
    libGL.so.1 => /usr/lib/x86_64-linux-gnu/mesa/libGL.so.1 (0x00007fee94569000)
    libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fee9434b000)
    libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fee93fc9000)
    libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fee93db3000)
    libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fee939e9000)
    librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007fee937e1000)
    libxcb.so.1 => /usr/lib/x86_64-linux-gnu/libxcb.so.1 (0x00007fee935bf000)
    libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fee933ba000)
    libXext.so.6 => /usr/lib/x86_64-linux-gnu/libXext.so.6 (0x00007fee931a8000)
    libXrender.so.1 => /usr/lib/x86_64-linux-gnu/libXrender.so.1 (0x00007fee92f9e000)
    /lib64/ld-linux-x86-64.so.2 (0x00005591d8db2000)
    libexpat.so.1 => /lib/x86_64-linux-gnu/libexpat.so.1 (0x00007fee92d74000)
    libxcb-dri3.so.0 => /usr/lib/x86_64-linux-gnu/libxcb-dri3.so.0 (0x00007fee92b71000)
    libxcb-present.so.0 => /usr/lib/x86_64-linux-gnu/libxcb-present.so.0 (0x00007fee9296e000)
    libxcb-sync.so.1 => /usr/lib/x86_64-linux-gnu/libxcb-sync.so.1 (0x00007fee92766000)
    libxshmfence.so.1 => /usr/lib/x86_64-linux-gnu/libxshmfence.so.1 (0x00007fee92563000)
    libglapi.so.0 => /usr/lib/x86_64-linux-gnu/libglapi.so.0 (0x00007fee92335000)
    libXdamage.so.1 => /usr/lib/x86_64-linux-gnu/libXdamage.so.1 (0x00007fee92131000)
    libXfixes.so.3 => /usr/lib/x86_64-linux-gnu/libXfixes.so.3 (0x00007fee91f2b000)
    libX11-xcb.so.1 => /usr/lib/x86_64-linux-gnu/libX11-xcb.so.1 (0x00007fee91d29000)
    libxcb-glx.so.0 => /usr/lib/x86_64-linux-gnu/libxcb-glx.so.0 (0x00007fee91b0f000)
    libxcb-dri2.so.0 => /usr/lib/x86_64-linux-gnu/libxcb-dri2.so.0 (0x00007fee9190a000)
    libXxf86vm.so.1 => /usr/lib/x86_64-linux-gnu/libXxf86vm.so.1 (0x00007fee91704000)
    libdrm.so.2 => /usr/lib/x86_64-linux-gnu/libdrm.so.2 (0x00007fee914f4000)
    libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fee911eb000)
    libXau.so.6 => /usr/lib/x86_64-linux-gnu/libXau.so.6 (0x00007fee90fe6000)
    libXdmcp.so.6 => /usr/lib/x86_64-linux-gnu/libXdmcp.so.6 (0x00007fee90de0000)

But you can also read that information directly from the binary using readelf -b lib/libsfml-window.so | grep NEEDED:

0x0000000000000001 (NEEDED)             Shared library: [libsfml-system.so.2.4]
0x0000000000000001 (NEEDED)             Shared library: [libX11.so.6]
0x0000000000000001 (NEEDED)             Shared library: [libXrandr.so.2]
0x0000000000000001 (NEEDED)             Shared library: [libudev.so.1]
0x0000000000000001 (NEEDED)             Shared library: [libGL.so.1]
0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]

As you can see, everything is fine, no direct reference to XCB anymore. :)

@MarioLiebisch

Works fine for me in Ubuntu 16.04 with XFCE (VMware).

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Sep 19, 2016

Member

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

Member

eXpl0it3r commented Sep 19, 2016

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

@eXpl0it3r eXpl0it3r merged commit 1dc3db0 into 2.4.x Sep 29, 2016

14 checks passed

debian-gcc-64 Build #208 done.
Details
freebsd-gcc-64 Build #208 done.
Details
osx-clang-el-capitan Build #91 done.
Details
static-analysis Build #208 done.
Details
windows-gcc-492-tdm-32 Build #93 done.
Details
windows-gcc-492-tdm-64 Build #93 done.
Details
windows-gcc-610-mingw-32 Build #27 done.
Details
windows-gcc-610-mingw-64 Build #27 done.
Details
windows-vc11-32 Build #209 done.
Details
windows-vc11-64 Build #209 done.
Details
windows-vc12-32 Build #209 done.
Details
windows-vc12-64 Build #207 done.
Details
windows-vc14-32 Build #208 done.
Details
windows-vc14-64 Build #210 done.
Details

@eXpl0it3r eXpl0it3r deleted the bugfix/xlib branch Sep 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment