Skip to content
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

Initial working X11 implementation with Vulkan #3372

Draft
wants to merge 59 commits into
base: docking
Choose a base branch
from

Conversation

RoryO
Copy link

@RoryO RoryO commented Jul 31, 2020

This is a basic X11 impl file and example. In it's current state it successfully creates an X11 window using the xcb library, connects the window to Vulkan and then uses the existing Vulkan impl files unmodified.

Since this isn't in complete feature parity with win32 and others, I'm going by this suggestion

In doubt, don't hesitate to push a PR because that is always the first step toward finding the mergeable solution!

This is also my first foray into X11 programming. It's a wild undocumented world, and the X11 protocol is too forgiving. Things will fail silently. I am by no means any sort of expert, so I hope the start of this PR attracts experts commenting on how to proceed with the rest of the features and/or tell me everything I did wrong.

What's left for feature parity, using the win32 impl as a baseline.

  • Keyboard input

  • Mouse cursor visibility and shape

  • Gamepad support

  • Cipboard support

  • OpenGL renderer

  • Mouse cursor
    It looks like xcb_xfixes_show_cursor and xcb_xfixes_hide_cursor does everything we need. I don't have that fully working currently. I think that's more me misunderstanding the boolean imgui flags and what they mean. Changing the X11 mouse cursor to represent the state of the mouse hovering over imgui widgets looks... like not a thing that usually happens with raw X11? That looks like it's a thing the window manager takes care of.

  • Gamepad
    I'm wondering if we should put this in the impl file or leave it up to the library user instead. Gamepad support is either in the main library proper, like win32, or also implemented in the impl file. However, with linux being linux there are two different routes for controller support. One being an older joystick API, and a newer one using libevdev. This would add some decent weight to the X11 impl file.

  • Clipboard
    This is a wild thing in X11 land. From what I can tell the common practice is creating a second unexposed window for clipboard handling. Then, set that window up for receiving clipboard events. When you want the clipboard contents, send a xcb_convert_selection message to the X server. After sending a request, use the second window to poll for the XCB_SELECTION_NOTIFY event. On receiving the event extract the data from the event message and store it, in this case in something like char * g_ClipboardContents. I'm not sure what the second window achieves, potentially better for multithreaded apps. It should be possible to do the same in the event processor in the X11 impl file. Sending clipboard contents to the X server looks more straightforward, if verbose. Example

  • OpenGL
    This is more on me not understanding OpenGL as much as I do Vulkan. Yes, I know that's a weird state for a person's knowledge, yet here I am. Not only that but OpenGL on linux is, as linux being linux, confusing and potentially incompatible. Lots of chatter on how xcb doesn't work with OpenGL straight up, though I see xcb glx libraries available? 🤷‍♂️ Vulkan worked totally fine without issue, they abstracted the idea of a window surface perfectly and it worked first try.

io.MousePos = ImVec2(e->event_x, e->event_y);
// X decided button 4 is mwheel up and 5 is mwheel down
if(e->detail == 4)
io.MouseWheel += 1.0;
Copy link
Author

@RoryO RoryO Jul 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Win32 has GET_WHEEL_DELTA_WPARAM() and WHEEL_DELTA macros for figuring out how 'far' the 'mouse wheel' (potentially touch/trackpad scroll) went. I couldn't find any such thing in X11 proper, that's usually handled by the window manager in different and of course incompatible ways. This works fine with touchpad and mwheel events, and a literal +/-1 felt fine.

ImGui::SliderFloat("float", &f, 0.0f, 1.0f); // Edit 1 float using a slider from 0.0f to 1.0f
if(ImGui::ColorEdit3("clear color", (float*)&clear_color)) // Edit 3 floats representing a color
{
wd->ClearValue.color.float32[0] = clear_color.x;
Copy link
Author

@RoryO RoryO Jul 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all the Vulkan examples are currently incorrect with the background color. The ClearValue is never set therefore it's always black. If we set the ClearValue when this widget changes then it works properly. I can create a separate PR fixing the other examples.

case XK_Control_R:
io.KeyCtrl = true;
break;
case XK_Meta_L:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The technical definition is that the meta and alt keys are different, in practice I don't think that's actually true in modern times.

@RoryO RoryO marked this pull request as ready for review August 5, 2020 22:58
@RoryO
Copy link
Author

RoryO commented Aug 5, 2020

I'll open this up as ready for review as it works in a basic case without the extra platform features.

@ocornut
Copy link
Owner

ocornut commented Aug 7, 2020

Thanks for working on this.

Attached a bunch of cumulative fixes to make the code follow the dear imgui coding style.

I also:

  • Renamed ImGui_ImplX11_Event() to ImGui_ImplX11_ProcessEvent()
  • Removed unnecessary indentation and branches from the main loop, making it matches other examples
  • Noted that display size change should be done in _ProcessEvent and not in the main function.
  • Various style fixes.

This has NOT been compiled, I don't have X11.

0001-Fixes-for-3372.zip

To add to your branch (possibly flatten into).

Co-authored-by: omar <omarcornut@gmail.com>
@RoryO
Copy link
Author

RoryO commented Aug 7, 2020

That diff applied cleanly and compiled without issue. Apologies for adding more work.

@RoryO
Copy link
Author

RoryO commented Aug 7, 2020

Bit more progress, hopefully less problematic with style issues. Added hiding the OS cursor. Using xcb-cursors it looks possible changing the OS mouse cursor reflecting the state of what widget it's over like with Windows. I'll do that next.

Example on what's working so far: responding to window resize, keyboard and mouse events, hiding the OS mouse cursor. Unfortunately I can't find a screen recorder which shows the actual cursor state, it does hide the cursor when entering/leaving the window and when the option changes.

Peek 2020-08-07 15-25

There are a few references on WantSetMousePos rarely used. Is it worthwhile implementing that feature?

@@ -34,6 +34,8 @@ static xcb_key_symbols_t* g_KeySyms;
static timespec g_LastTime;
static timespec g_CurrentTime;

static bool g_HideXCursor = false;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Win32 we just call SetCursor(NULL) every frame we want the mouse cursor hidden. With X we have to do a request/reply for that. While I don't think that's a huge issue doing that every frame, I observed how various other 3D context X windows behave. I couldn't find any that did a request/reply every frame, every window I observed did not have any events when the window is idle. So while this seems silly, I can't figure out any other way on checking 'did the state of the MouseDrawCursor flag change?' without keeping a copy and comparing between frames.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should keep a copy indeed (of the ImGuiMouseCursor value) if you want to detect change. When the value is _None that means to hide the cursor.

@RoryO
Copy link
Author

RoryO commented Aug 28, 2020

Merged in #3390 as I'm at the limit on what I can do with viewports without it.

Otherwise have removing window chrome working. Dragging a window around without a frame is pretty choppy. SDL and glfw both are as well, glfw seems a bit less choppy. Different window managers doesn't help much. I wonder what window managers do to get smoother window motions. The framerate is still 60fps internally when moving a window so it should be sending an update for the window position 60 time a second.

Interestingly the glfw and sdl impl doesn't respect changing the window decoration after window creation.

Peek 2020-08-28 02-10

@ocornut
Copy link
Owner

ocornut commented Aug 28, 2020

Interestingly the glfw and sdl impl doesn't respect changing the window decoration after window creation.

That’s right, i suppose we could add support for that in a separate PR. Win32 does support it.

@piotrrak
Copy link

@RoryO about your xcb/GLX/xlib in OpenGL bullet

GLX protocol extension API is defined in terms of libX11 objects on API/ABI level;
libxcb is successor of libX11 and libX11 was rewritten (ie. libX11-xcb) in terms of libxcb to support legacy clients linking libX11.

In other words back in 1992 Khronos standardized that <GL/glx.h> includes <X11/Xlib.h> so we must use libX11 with GLX and no way around it.

Since after around two decades almost everyone uses libX11-xcb flavor of libX11 one can use <X11/Xlib-xcb.h> to obtain xcb_connection_t* and thus xcb_screen_t* form Display.
However contrary is not true, there is no way to obtain libX11 Display using just libxcb.

That is one of few reasons for more recent uptake in adaptation of EGL API as an alternative to straight GLX
It has by extension a concept of platform EGL_EXT_platform_base that can be added and advertised as derived extensions

  • native display (X11 Display, xcb_screen_t*, wl_display*, DRM fd - open("/dev/dri/card%d", ...), gbm_device)
  • native window surface - (X11 Window*, xcb_window_t*, wl_surface*, drm buffer attached to KMS plane, gbm_surface *)
    etc..

You can think about it as WSI in Vulkan is alternative to:

AGL - ( on mac osx - that died in favour of metal)
WGL - (on windows - that no one really cared about until WebGL came)
GLX - (X11 protocol extension that let you add OpenGL server support to X11 server, so X11 OpenGL client can use it)
EGL - initialy targeted embedded where there were no X11 or any other windowing system.

Bit of mess, but maybe in a decade or two, we will have new shiny mess and will forget about this one.
If you have additional questions, please let me know, maybe I'll know the anwser.

@RoryO
Copy link
Author

RoryO commented Oct 26, 2020

Wow thank you for that excellent information @piotrrak ! I've been on another project for a bit as this is at the point where it's usable for myself. When I get back to this I'll use that information on creating an opengl x11 impl.

I haven't tried wayland yet as for some reason systemd does stupid systemd things and crashes when I try a wayland session; though I think Xwayland should work out fine.

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

Successfully merging this pull request may close these issues.

4 participants