~eliasnaur/gio#8: 
X11 support

IIUC, gio currently requires Wayland, which I don’t (yet) have in any of my Linux environments for various reasons.

I suspect I’m not the only one with this issue. Is there any chance X11 support could be added?

Thanks in advance,

Status
RESOLVED FIXED
Submitter
~stapelberg
Assigned to
No-one
Submitted
4 years ago
Updated
2 years ago
Labels
No labels applied.

~eliasnaur 4 years ago

I had hoped to avoid X11 support altogether given the stage of the Wayland transition. However, given enough interest, there is no technical reason X11 couldn't be added.

~stapelberg 4 years ago

I think you might be over-estimating how far the Wayland transition is along :)

I certainly will have to use X11 in various environments for years to come.

~dolanor 4 years ago

Same here, still on Ubuntu 16.04, and given the feedbacks of breakage with 18.04, I'm hesitant to migrate.

~rkanchan 4 years ago

Same with elementaryOS Juno. Wayland is yet to arrive.

~theclapp 4 years ago

I don't know much about Wayland. Is there some docker- or vm-based solution y'all could run? That is, some docker image or virtual machine image that runs Wayland, that you could use to run Gio? What's a few gig of disk among friends? :)

Failing that, I've had some luck getting Gio to run in Chrome on the Mac, so running via your browser might be a possibility.

Failing that ... there's only (he said naively) about 1500 lines of Go and C (mostly Go) in the Wayland-specific code. (There's about 3400 lines of generated C.) Are Wayland and X11 similar enough that the Wayland code would make the X11 port comparatively easy? (Compared to developing it from scratch.)

~mvdan 4 years ago

Yesterday, I successfully ran sway out of the box within X11, fired a terminal, and ran a couple of gio examples in Wayland. Ctrl+D allows you to run any program via dmenu, so that let me start my terminal emulator.

It's a bit clunky, because the parent window manager takes precedence with keyboard shortcuts, but it mostly works.

Having said this, I agree X11 support would be good. We all have plans to move to Wayland eventually, but I think X11 support would be key to Gio's success in the short term.

~eliasnaur could you write a short doc with the steps one would have to follow to implement X11 support? Perhaps then someone with enough interest could step in and start contributing patches.

~eliasnaur 4 years ago

~eliasnaur could you write a short doc with the steps one would have to follow to implement X11 support? Perhaps then someone with enough interest could step in and start contributing patches.

Good idea.

The platform specific packages in Gio are ui/app and ui/app/internal/gl.

I don't think you need anything X11 specific in the gl package.

The app package takes care of creating a native window, binding an OpenGL ES context and handling input. I suggest you use the Wayland port as a starting point.

Dynamic switching between the Wayland and X11 backends would be nice, but as a first goal I suggest defining a build tag, say "x11", that switches from Wayland to X11 at build time.

So the first step is adding

// +build !x11

to every wayland-specific Go and C files. You'll have to modify the go:generate directives in os_wayland.go to add the x11 condition to the generated Wayland C files (wayland_xdg_shell.c, wayland_xdg_decoration.c, wayland_text_input.c).

What I usually do at this point of adding a new port is to make minimal changes and empty types, functions, methods etc. to make a Gio program build and run. Use the hello or gophers example for that.

Then, I work through the necessary steps for creating a Window and binding a context. The goal is to get the drawing to show up in a window. Note that Gio programs won't draw before Window.setStage(StateRunning) has been called.

For input I wire up event handling to route mouse and keyboard input back to the Gio program. My experience with X11 tells that this is where most of the issues will be. Things like key code translation, scroll wheel etc. were fiddly when I did the X11 port for LWJGL.

Finally, polish: lifecycle for minimizing/maxizing, setting the window title and size and so on.

Hope that helps. I'll gladly expand on any issues you run into.

  • elias

~dennwc 4 years ago

Initial version of X11 backend can be checked here:

https://github.com/dennwc/gio/commits/x11_test

It currently supports:

  • Rendering
  • Window resize
  • Window close
  • Mouse events

To test it, add this line to your go.mod:

replace gioui.org/ui => github.com/dennwc/gio/ui v0.0.0-20190828194120-1078c141cc54

This change will be submitted to the upstream soon.

~eliasnaur 4 years ago

On Thu Aug 29, 2019 at 9:03 PM ~dennwc wrote:

Initial version of X11 backend can be checked here:

https://github.com/dennwc/gio/commits/x11_test

It currently supports:

  • Rendering
  • Window resize
  • Window close
  • Mouse events

Very nice!

-- elias

~rkanchan 4 years ago

Awesome work ~dennwc. Tested with success, the hello app on elementaryOS. Thank you

~rkanchan 4 years ago

text input is not working for me in the gophers app. In fact it did not work in the wayland version as well when I had earlier tried inside of a VM(ubuntu 18.04)

~dolanor 4 years ago

Nice, I need to try it! Will come back with feedback.

~dolanor 4 years ago

It works. That is my feedback. That's great! Thanks!

~dennwc 4 years ago

text input is not working for me in the gophers app

~rkanchan X11 backend doesn't support keyboard input yet :)

~rkanchan 4 years ago

ah ok. I'll wait for it :)

~cryptix referenced this from #35 4 years ago

~eliasnaur referenced this from #35 4 years ago

~fhs 4 years ago

Yesterday, I successfully ran sway out of the box within X11, fired a terminal, and ran a couple of gio examples in Wayland. Ctrl+D allows you to run any program via dmenu, so that let me start my terminal emulator.

It's a bit clunky, because the parent window manager takes precedence with keyboard shortcuts, but it mostly works.

A better alternative to running it within sway is the run in inside cage. Just run:

cage gio_app_command

and it opens a standalone window with the app. I'd imagine clipboard sharing will still be an issue, so a native X11 support would be great.

~rkanchan 4 years ago

~dennwc Are you still working on the X11 backend?

~eliasnaur 4 years ago

FWIW, I extracted the XKB logic to a separate file independent of Wayland. Perhaps the X11 backend can reuse the code.

https://gioui.org/commit/51cfb4e

~dennwc 4 years ago

Are you still working on the X11 backend?

Yes, but I was busy recently. Will make the changes in following days.

FWIW, I extracted the XKB logic to a separate file independent of Wayland. Perhaps the X11 backend can reuse the code.

This is awesome! I'll check if I can reuse it directly, or there is something else needed on X11 side.

On Thu, 26 Sep 2019 at 11:05, ~eliasnaur outgoing@sr.ht wrote:

FWIW, I extracted the XKB logic to a separate file independent of Wayland. Perhaps the X11 backend can reuse the code.

https://gioui.org/commit/51cfb4e

-- View on the web: https://todo.sr.ht/~eliasnaur/gio/8#comment-3896

-- Denys

~dennwc 4 years ago

~eliasnaur Still busy, sorry. If you have time to apply the mentioned change, may I ask you to do it on your side? I'll jump back to it to finish XKB as soon as I can.

On Thu, 26 Sep 2019 at 14:07, Denys Smirnov denis.smirnov.91@gmail.com wrote:

Are you still working on the X11 backend?

Yes, but I was busy recently. Will make the changes in following days.

FWIW, I extracted the XKB logic to a separate file independent of Wayland. Perhaps the X11 backend can reuse the code.

This is awesome! I'll check if I can reuse it directly, or there is something else needed on X11 side.

On Thu, 26 Sep 2019 at 11:05, ~eliasnaur outgoing@sr.ht wrote:

FWIW, I extracted the XKB logic to a separate file independent of Wayland. Perhaps the X11 backend can reuse the code.

https://gioui.org/commit/51cfb4e

-- View on the web: https://todo.sr.ht/~eliasnaur/gio/8#comment-3896

-- Denys

-- Denys

~eliasnaur 4 years ago

On Wed Oct 2, 2019 at 12:01 PM ~dennwc wrote:

~eliasnaur Still busy, sorry. If you have time to apply the mentioned

change, may I ask you to do it on your side? I'll jump back to it to

finish XKB as soon as I can.

I'm not sure what change you're referring to.

-- elias

~dennwc 4 years ago

The move/rename of the native type (C type for EGL).

~eliasnaur 4 years ago

On Wed Oct 2, 2019 at 6:21 PM ~dennwc wrote:

The move/rename of the native type (C type for EGL).

Done in 9e2c413c1d2ecead60feb07e7863ca368ffd3be8 and fc5b660efc31e38e10acfdba02101da09736f0ea.

-- elias

~eliasnaur 4 years ago

On Wed Oct 2, 2019 at 11:31 PM Elias Naur wrote:

On Wed Oct 2, 2019 at 6:21 PM ~dennwc wrote:

The move/rename of the native type (C type for EGL).

Done in 9e2c413c1d2ecead60feb07e7863ca368ffd3be8 and fc5b660efc31e38e10acfdba02101da09736f0ea.

Whoops, I mean

gioui.org/commit/2dcbf6fe3c9e648c9c8ef049aa58ad4873ccb858
gioui.org/commit/07a36d71d9b0e6c89b6d750b03bb026500ff2dd7

-- elias

~eliasnaur closed duplicate ticket #40 4 years ago

~stuartdd closed duplicate ticket #40 4 years ago

~db47h 4 years ago

While dynamic fallback from Wayland to X11 would be a nice feature, it requires the executable to dynamically link with libwayland-egl.so and a couple others (unless we start to do ugly things like dl'open/GetProcAddress). I'm not sure that this is desirable for X11 builds.

~eliasnaur 4 years ago

On Wed Oct 9, 2019 at 4:11 PM ~db47h wrote:

While dynamic fallback from Wayland to X11 would be a nice feature, it requires the executable to dynamically link with libwayland-egl.so and a couple others (unless we start to do ugly things like dl'open/GetProcAddress). I'm not sure that this is desirable for X11 builds.

I suspect many systems have libwayland-egl.so installed even though the user runs X11.

Your point is still valid, however. A less radical solution is a nowayland tag for building without Wayland support.

~db47h 4 years ago

Your point is still valid, however. A less radical solution is a nowayland tag for building without Wayland support.

fair enough. Looking at the current implementation, I'm wondering why Window.driver is not an actual interface: there's a check that the window type implements a non-declared driver interface, so why not declare it as such? It would make implementing the wayland and x11 drivers (with optional support for the other) much easier.

~eliasnaur 4 years ago

On Thu Oct 10, 2019 at 1:36 PM ~db47h wrote:

Your point is still valid, however. A less radical solution is a nowayland tag for building without Wayland support.

fair enough. Looking at the current implementation, I'm wondering why Window.driver is not an actual interface: there's a check that the window type implements a non-declared driver interface, so why not declare it as such? It would make implementing the wayland and x11 drivers (with optional support for the other) much easier.

FWIW, I believe ~dennwc's work incomplete patch promotes the anonymous interface to a named type.

I tend to avoid interfaces I don't need, and Gio didn't need multiple drivers in one build before X11.

~db47h 4 years ago

FWIW, I believe ~dennwc's work incomplete patch promotes the anonymous interface to a named type.

He resorted to a stub window type that hods pointers to a wlWindow and an x11Window and calls methods of x11Window if the wlWindow is nil.

I tend to avoid interfaces I don't need, and Gio didn't need multiple drivers in one build before X11.

Same here. I was just wondering if it would have any non obvious side effects.

Since @dennwc looks busy, I took the liberty to go ahead and integrate his work on top of latest master: https://github.com/db47h/gio/commits/x11 (still without keyboard support).

The first two commits are pretty straight forward: decalre a windowDriver interface, make newContext a window method in order to prevent conflicts between eglDriver/windowDriver, and remove refs to the wayland-only conn variable in the xkb driver.

X11 support proper is a straight copy-paste of ~dennwc's os_x11.go. I just added the few bits and pieces to have the default linux builds to provide dual wayland/x11 support and optionally disabling them with a nowayland or nox11 tag.

If this looks good to everyone, I'd like to go ahead and add keyboard support.

Denis

~eliasnaur 4 years ago

On Fri Oct 11, 2019 at 1:15 PM ~db47h wrote:

Since @dennwc looks busy, I took the liberty to go ahead and integrate his work on top of latest master: https://github.com/db47h/gio/commits/x11 (still without keyboard support).

Great!

The first two commits are pretty straight forward: decalre a windowDriver interface, make newContext a window method in order to prevent conflicts between eglDriver/windowDriver, and remove refs to the wayland-only conn variable in the xkb driver.

X11 support proper is a straight copy-paste of ~dennwc's os_x11.go. I just added the few bits and pieces to have the default linux builds to provide dual wayland/x11 support and optionally disabling them with a nowayland or nox11 tag.

If this looks good to everyone, I'd like to go ahead and add keyboard support.

Looks good to me; I left a few comments on GitHub. I also took the liberty of integrating your xkb fix.

~db47h 4 years ago

Updated with EGL stuff moved where it belongs. Sorry, I did a force push after rebasing on master with the xkb changes so your comments are gone.

Anyhow, about using an array for drivers in https://github.com/db47h/gio/commit/1c5614b4ed63104f8f98b0ad3eff50312a3b39f6#diff-70a2c89ac82cfe06c897443a159d0563 : I thought about it, but for only two linux specific drivers, I believe it would be a bad case of over engineering (need to set-up some self-registering mechanism with a mutex and a priority system).

On the other hand, this could be the basis for multi-driver support on all platforms (I'm thinking native win API instead of EGL on Windows), but it needs some more thought. For example on what kind of errors do we fallback to the next driver (had to declare errWLDisplayConnectFailed in os_linux.go), how to setup priorities and so on. Maybe talk about it in another issue thread?

~eliasnaur 4 years ago

On Sat Oct 12, 2019 at 12:54 PM ~db47h wrote:

Updated with EGL stuff moved where it belongs. Sorry, I did a force push after rebasing on master with the xkb changes so your comments are gone.

Anyhow, about using an array for drivers in https://github.com/ db47h/gio/commit/1c5614b4ed63104f8f98b0ad3eff50312a3b39f6#diff-70a2c89ac 82cfe06c897443a159d0563 : I thought about it, but for only two linux specific drivers, I believe it would be a bad case of over engineering (need to set-up some self-registering mechanism with a mutex and a priority system).

I don't think you need a mutex, since the drivers are registered in init functions that run in a single goroutine.

You're right about priorities, however. Go probably runs init functions within a package in a particular order, but that's too subtle to rely on.

~db47h 4 years ago

I don't think you need a mutex, since the drivers are registered in init functions that run in a single goroutine.

That's correct, and they are called in lexical order. But like you said, too subtle to rely on.

~db47h 4 years ago

Keyboard support added: https://github.com/db47h/gio/commit/f6458e67809bb39985cc9526be35aff6b7595798

I didn't use XKB and went for XIM. A few things worth noting:

  • setlocale needs to be called in createWindow(). It is only set temporarily for XOpenIM then reverted back to whatever it was before. I don't like to call setlocale in general, even less in Go, but this is unavoidable here.
  • CTRL-SHIFT-U + unicode code point works, however the helper "window" showing what code point you enter appears at the bottom left of the GIO window. If anyone has a clue on how to move it or better show it inline in a text editor, that would be awesome. Plus it shouldn't be available unless actively editing something.

TODOs:

  • merge master...
  • proper dp/sp setup
  • animation

~eliasnaur 4 years ago

On Tue Oct 15, 2019 at 1:50 PM ~db47h wrote:

Keyboard support added: https://github.com/db47h/gio/commit/f6458e67809b b39985cc9526be35aff6b7595798

Nice.

I didn't use XKB and went for XIM. A few things worth noting:

  • CTRL-SHIFT-U

?

  • unicode code point works, however the helper "window" showing what code point you enter appears at the bottom left of the GIO window. If anyone has a clue on how to move it or better show it inline in a text editor, that would be awesome.

This is not something the Editor supports today.

Gio needs better support for IMs for the mobile platforms, at which point I suspect the OS would like to know the window coordinates of the caret.

Plus it shouldn't be available unless actively editing something.

Gio calls ShowTextInput(visible) on the native window to guide the soft keyboards on the mobiles. Perhaps that is also useful to show and hide the XIM window.

~db47h 4 years ago

On a bog standard Ubuntu/X11/Gnome you can enter any unicode code point by pressing CTRL-SHIFT-U followed by the hex value of the unicode code point (much like Alt+keypad on Windows). For example, to enter "…" (U+2026), press CTRL-SHIFT-U, 2, 0, 2, 6, Enter. While you key in the code point, and depending on the application, you have either a small window near the caret that shows an underscored "u" followed by whatever you enter or it's just displayed inline in the input widget (like chrome does).

Gio calls ShowTextInput(visible) on the native window to guide the soft keyboards on the mobiles. Perhaps that is also useful to show and hide the XIM window.

Good thinking. I'll have to check for possible issues with devices without a physical keyboard though.

Onto merging master...

~db47h 4 years ago

And master merged: https://github.com/db47h/gio/tree/x11_v2

Still need to implement animation before it's usable.

~eliasnaur 4 years ago

On Wed Oct 16, 2019 at 4:07 PM ~db47h wrote:

Basic animation added : https://github.com/db47h/gio/tree/x11_v2

It is in a usable state, so please give it a go.

I said "basic" animation because right now the event loop is throttled to draw at a fixed 1/60s interval while animating. While this helps limit CPU usage from the app itself, oddly enough Gnome Shell shows 80% CPU usage...

Is there any way to get feedback from eglDriver.Present() (i.e. vsync), or some other timer in gio in order to throttle the event loop? Well, I know that hoping for a reliable vsync is a pipe dream on X11, but a ticker from gio would be awesome.

There is eglSwapInterval. See the comment in egl.go:

    // eglSwapInterval 1 leads to erratic frame rates and unnecessary blocking.
    // We rely on platform specific frame rate limiting instead, except on Windows
    // where eglSwapInterval is all there is.
    if runtime.GOOS != "windows" {
      eglSwapInterval(eglCtx.disp, 0)
    } else {
      eglSwapInterval(eglCtx.disp, 1)
    }

I suppose eglSwapInterval should be used on X11 as well.

~db47h 4 years ago

Yes, eglSwapInterval should be used on X11, but since context.Present() calls it already, I was hoping for a notification that the function has returned. On the other hand, one cannot rely on vsync on X11 since it's not always working, especially on optimus laptops.

Here's where I'm coming from: https://web.archive.org/web/20190506122532/http://gafferongames.com/post/fix_your_timestep/

In the main loop, I usually wait for events if there is no animation going on, and just poll events if there is any animation. The only thing that is expected from the platform driver is a PollEvent() and an abortable WaitForEvent(). All this with vsync enabled: no tearing and no erratic frame rate.

Also, since one can never be sure that vsync works, I throw in a frame throttling mechanism, just not to kill the CPU while doing nothing (then rely on gl.Flush and hope for the best tearing-wise).

~eliasnaur 4 years ago

On Wed Oct 16, 2019 at 5:29 PM ~db47h wrote:

Yes, eglSwapInterval should be used on X11, but since context.Present() calls it already, I was hoping for a notification that the function has returned. On the other hand, one cannot rely on vsync on X11 since it's not always working, especially on optimus laptops.

It's possible we're talking past each other. Yes, eglSwapBuffers is called by Present, but eglSwapInterval(1) for enabling v-sync (as far as the driver allows), is currently called on Windows only.

There is an implicit notification that eglSwapBuffers completed: the gpu.Flush call waits for the result of the previous frame before returning, which includes the buffer swap. The separate Flush is necessary to give the main goroutine a chance to work concurrently with the GPU goroutine.

On Wayland eglSwapInterval is 0, but the frame rate is throttled by wayland frame callbacks instead, as per

https://emersion.fr/blog/2018/wayland-rendering-loop/

Here's where I'm coming from: https://web.archive.org/web/20190506122532/http://gafferong ames.com/post/fix_your_timestep/

In the main loop, I usually wait for events if there is no animation going on, and just poll events if there is any animation. The only thing that is expected from the platform driver is a PollEvent() and an abortable WaitForEvent(). All this with vsync enabled: no tearing and no erratic frame rate.

It's entirely possible the animation protocol between app.Window and the native peer could be improved. I'm open to alternative designs.

Also, since one can never be sure that vsync works, I throw in a frame throttling mechanism, just not to kill the CPU while doing nothing (then rely on gl.Flush and hope for the best tearing-wise).

SGTM.

-- elias

~db47h 4 years ago

It's possible we're talking past each other. Yes, eglSwapBuffers is called by Present, but eglSwapInterval(1) for enabling v-sync (as far as the driver allows), is currently called on Windows only.

Yes, sorry about that. I did not see the eglSwapInterval(0) on linux. And I still don't have a full picture of the various goroutines running and how they interact with each other.

Thanks for the clarification on how this works on Wayland.

It's entirely possible the animation protocol between app.Window and the native peer could be improved. I'm open to alternative designs.

I'll get a fully working X11 implementation first ;)

~db47h 4 years ago

Regarding gnome-shell's high CPU usage, looks like this is a known issue when running windowed OpenGL applications with NVIDIA proprietary drivers: https://bugzilla.gnome.org/show_bug.cgi?id=781835

~db47h 4 years ago

I just pushed an update to https://github.com/db47h/gio/tree/x11_v2 that properly throttles the frame rate to 60fps. The very good news is that CPU usage is now barely noticeable. A couple of things worth noting:

  • The fps cap is hard coded until I get around to query the real screen refresh rate from the OS.
  • I haven't turned vsync on in this commit, but it can be enabled without issues. Elias, would it be OK to add a SwapInterval function to the EGL driver interface? That would allow enabling it on a per-driver basis.

I still need to implement proper dp/sp configuration and fling support before merging.

~mvdan 4 years ago

Thanks so much for your work, ~db47h!

Speaking as a bystander - would it not be worth it to merge what currently works and call it experimental, and then incremental patches can fix the remaining issues? That way, others can test it more easily, and the project won't get a large amount of new lines of code in a single day.

~db47h 4 years ago

Thanks!

Speaking as a bystander - would it not be worth it to merge what currently works and call it experimental, and then incremental patches can fix the remaining issues?

There's still enough work to do to keep it in a separate branch IMHO (as numerous incremental patches will significantly increase ~eliasnaur' workload).

That way, others can test it more easily, and the project won't get a large amount of new lines of code in a single day.

The x11 branch on github is fairly up to date since I usually merge master before pushing to github. And testing is easy, just add the following line to your go.mod file:

replace gioui.org => github.com/db47h/gio x11_v2

When building, the x11_v2 will be replaced by the commit id. Just revert it back to x11_v2 whenever you want to update.

~eliasnaur 4 years ago

On Tue Oct 22, 2019 at 4:37 PM ~db47h wrote:

I just pushed an update to https://github.com/db47h/gio/tree/x11_v2 that properly throttles the frame rate to 60fps. The very good news is that CPU usage is now barely noticeable. A couple of things worth noting:

Again, thank you for your work.

  • The fps cap is hard coded until I get around to query the real screen refresh rate from the OS.

How will the fps cap interact with v-sync if the refresh rate isn't detected, changes (monitor switch) or just wrongly reported? If the cap is lower than the actual refresh, we'll have jittery animation.

  • I haven't turned vsync on in this commit, but it can be enabled without issues. Elias, would it be OK to add a SwapInterval function to the EGL driver interface? That would allow enabling it on a per-driver basis.

Of course, go ahead.

I still need to implement proper dp/sp configuration and fling support before merging.

Dp scaling is great for consistency. Fling support is a nice-to-have in case you'd like to merge your work without it.

~db47h 4 years ago

How will the fps cap interact with v-sync if the refresh rate isn't detected, changes (monitor switch) or just wrongly reported? If the cap is lower than the actual refresh, we'll have jittery animation.

The rationale behind capping the fps is that we want animated stuff to animate smoothly without killing the CPU, GPU and battery. My initial plan was indeed to enable vsync and use a hard cap as fallback, but indeed, having both enabled can lead to all sorts of problems and is probably not a good idea for an initial release of the X11 driver.

For the time being, I'll just use a timer based fps cap, leave vsync alone, and wait for user feedback. Regarding the actual cap values, jittery animation only occurs when the hard cap and actual refresh rate are not exact multiples. If the cap is lower, the application just feels somewhat sluggish. Also querying the refresh rate from XrandR is fairly reliable (and available on all modern X11 servers on linux) and even if the reported values can often be not quite equal to the real refresh rate (e.g. 60 vs. 59.9) this is not a major issue. Icing on the cake, XrandR can send screen config changes events, so it's easy to keep track of the refresh rate. If bad comes to worst and XrandR is not available, we can always use a sane default of 30 or 60fps and log a message about XrandR missing.

Dp scaling is great for consistency. Fling support is a nice-to-have in case you'd like to merge your work without it.

Ok, I'll get dp scaling in then we can merge.

~eliasnaur 4 years ago

On Tue Oct 22, 2019 at 11:45 PM ~db47h wrote:

How will the fps cap interact with v-sync if the refresh rate isn't detected, changes (monitor switch) or just wrongly reported? If the cap is lower than the actual refresh, we'll have jittery animation.

The rationale behind capping the fps is that we want animated stuff to animate smoothly without killing the CPU, GPU and battery. My initial plan was indeed to enable vsync and use a hard cap as fallback, but indeed, having both enabled can lead to all sorts of problems and is probably not a good idea for an initial release of the X11 driver.

For the time being, I'll just use a timer based fps cap, leave vsync alone, and wait for user feedback. Regarding the actual cap values, jittery animation only occurs when the hard cap and actual refresh rate are not exact multiples. If the cap is lower, the application just feels somewhat sluggish. Also querying the refresh rate from XrandR is fairly reliable (and available on all modern X11 servers on linux) and even if the reported values can often be not quite equal to the real refresh rate (e.g. 60 vs. 59.9) this is not a major issue. Icing on the cake, XrandR can send screen config changes events, so it's easy to keep track of the refresh rate. If bad comes to worst and XrandR is not available, we can always use a sane default of 30 or 60fps and log a message about XrandR missing.

In what circumstances does eglSwapInterval(1) fail? It seems to be the simplest approach, and avoids the complexity and subtle timing issues with manual capping:

For example, even if our cap and the monitor refresh rate match, the exact time Gio submit its framebuffers to X11 will drift relative to the monitor refresh interval. The result depends on whether the X11 compositor has v-sync enabled:

  • If X has v-sync enabled, Gio buffers will miss a frame every X frames, where X depends on how accurately our 60 Hz matches the monitor 60 Hz.
  • If X doesn't have v-sync enabled, the buffers will most likely arrive during the monitor redraw, resulting in visual shearing.

So it seems we should enable eglSwapBuffers(1), in which case the additional frame capping is more trouble than it's worth.

~db47h 4 years ago

In what circumstances does eglSwapInterval(1) fail? It seems to be the simplest approach, and avoids the complexity and subtle timing issues with manual capping

Even with eglSwapInterval(1), on some hardware/OS combinations eglSwapBuffers() does not wait for vertical synchronization ant returns immediately (that's what I mean by "vsync not working"). This used to be the case on all laptops with NVIDIA Optimus (dual Intel+NV GPU), see

https://wiki.archlinux.org/index.php/NVIDIA_Optimus#Tearing/Broken_VSync 

Last time I checked, there was tearing and X11 1.19 wasn't available yet (so that was a long time ago I must admit). While it seems that DRM kernel mode setting must be enabled manually, I did some testing and found out that with a stock Ubuntu 18.04 LTS/GNOME, and without fiddling with kernel parameters:

  • windowed OpenGL: animation is a bit jittery (so the compositor interferes in some way, but there's nothing we can do about it). Everywhere I tested it's the same, and I couldn't find any OpenGL application that did not exhibit this behavior. Regardless, eglSwapBuffers() actually waits as expected.
  • fullscreen OpenGL: no visible tearing for full screen applications (so vsync works properly).

So it seems that I was wrongly assuming that vsync would not work in enough cases to justify going through the trouble of implementing a timer based rendering loop instead of relying on eglSwapInterval(1)/eglSapeBuffers(). I'll just go back to the drawing board...

BTW, why do mouse move events trigger SetAnimating(true) ?

~eliasnaur 4 years ago

BTW, why do mouse move events trigger SetAnimating(true) ?

Because the program registered a pointer handler that matches the mouse move and we must request a new frame to let the app know.

~db47h 4 years ago

I made eglDriver.eglSwapInterval return the swap interval value preferred by the driver instead of calling eglSwapInterval directly. I find it better but you can do it the other way round if you like. The call had to be moved out of createSurfaceAndMakeCurrent anyway.

Commit https://github.com/db47h/gio/commit/feaaeef40a7270678191b8170ca0d50ce0d9f897 : vsync based event loop

That should do it. Testers welcome!

~eliasnaur 4 years ago

On Thu Oct 24, 2019 at 2:52 PM ~db47h wrote:

I made eglDriver.eglSwapInterval return the swap interval value preferred by the driver instead of calling eglSwapInterval directly. I find it better but you can do it the other way round if you like. The call had to be moved out of createSurfaceAndMakeCurrent anyway.

SGTM.

Commit https://github.com/db47h/gio/commit/feaaeef40a7270678191b8170ca0d50ce0d9f897: vsync based event loop

That should do it. Testers welcome!

Great. I won't have access to my Linux machine before Friday, but I'm happy to hear from other X11'ers.

Let me know when you want me to review the code more closely for merging. If you don't like git send-email, there is now a new web-based flow for preparing a patchset on from a git.sr.ht repository.

-- elias

~db47h 4 years ago

Let me know when you want me to review the code more closely for merging. If you don't like git send-email, there is now a new web-based flow for preparing a patchset on from a git.sr.ht repository.

I never used git send-email but I don't mind using it. My The GH web based interface just made it easier to discuss it while it was still a WIP. I'll send in two patch sets, one for eglSwapInterval and the rest for the X11 driver.

~eliasnaur REPORTED FIXED 4 years ago

Thank you ~db47h and ~dennwc for adding X11 support!

~inkeliz referenced this from #274 2 years ago

Register here or Log in to comment, or comment via email.