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

New release for compatibility with wlroots 0.9.x? #107

Closed
primeos opened this issue Jan 9, 2020 · 20 comments
Closed

New release for compatibility with wlroots 0.9.x? #107

primeos opened this issue Jan 9, 2020 · 20 comments

Comments

@primeos
Copy link
Contributor

primeos commented Jan 9, 2020

The current release (0.1.1) fails to build with wlroots 0.9.x (due to breaking changes in wlroots):

../cage.c: In function 'main':
../cage.c:377:2: error: implicit declaration of function 'wlr_server_decoration_manager_destroy'; did you mean 'wlr_server_decoration_manager_create'? [-Werror=implicit-function-declaration]
  377 |  wlr_server_decoration_manager_destroy(server_decoration_manager);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |  wlr_server_decoration_manager_create
../cage.c:378:2: error: implicit declaration of function 'wlr_xdg_decoration_manager_v1_destroy'; did you mean 'wlr_xdg_decoration_manager_v1_create'? [-Werror=implicit-function-declaration]
  378 |  wlr_xdg_decoration_manager_v1_destroy(xdg_decoration_manager);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |  wlr_xdg_decoration_manager_v1_create
../cage.c:379:2: error: implicit declaration of function 'wlr_xdg_shell_destroy'; did you mean 'wlr_xdg_popup_destroy'? [-Werror=implicit-function-declaration]
  379 |  wlr_xdg_shell_destroy(xdg_shell);
      |  ^~~~~~~~~~~~~~~~~~~~~
      |  wlr_xdg_popup_destroy
../cage.c:380:2: error: implicit declaration of function 'wlr_idle_inhibit_v1_destroy'; did you mean 'wlr_idle_inhibit_v1_create'? [-Werror=implicit-function-declaration]
  380 |  wlr_idle_inhibit_v1_destroy(server.idle_inhibit_v1);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
      |  wlr_idle_inhibit_v1_create
../cage.c:382:3: error: implicit declaration of function 'wlr_idle_destroy'; did you mean 'wlr_seat_destroy'? [-Werror=implicit-function-declaration]
  382 |   wlr_idle_destroy(server.idle);
      |   ^~~~~~~~~~~~~~~~
      |   wlr_seat_destroy
../cage.c:384:2: error: implicit declaration of function 'wlr_data_device_manager_destroy'; did you mean 'wlr_data_device_manager_create'? [-Werror=implicit-function-declaration]
  384 |  wlr_data_device_manager_destroy(data_device_mgr);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |  wlr_data_device_manager_create
../cage.c:385:2: error: implicit declaration of function 'wlr_compositor_destroy'; did you mean 'wlr_cursor_destroy'? [-Werror=implicit-function-declaration]
  385 |  wlr_compositor_destroy(compositor);
      |  ^~~~~~~~~~~~~~~~~~~~~~
      |  wlr_cursor_destroy
cc1: all warnings being treated as errors

But the current version from the master branch is already compatible with wlroots 0.9.x.

Would it be possible to release a new version that builds with wlroots 0.9.x so that downstream package maintainers could use a tagged version instead of a commit from the master branch?

There's currently no hurry because Sway 1.2 is also not compatible with wlroots 0.9.x, but if there is no reason against cutting a new release, it would be nice if the new release is ready once Sway 1.3 will be released.

@jbeich
Copy link
Contributor

jbeich commented Jan 10, 2020

could use a tagged version instead of a commit from the master branch?

Why not backport the fixes to release? Like FreeBSD Ports.

@primeos
Copy link
Contributor Author

primeos commented Jan 10, 2020

@jbeich that would also be possible. Usually I try to avoid it as it could also introduce regressions (especially if one uses this strategy to aggressively and/or doesn't pay enough attention), but in this case it should most likely be fine. In any case: Thanks for pointing out the relevant patch :)

@Hjdskes
Copy link
Collaborator

Hjdskes commented Jan 10, 2020

I'm working on the rendering code right now, which I'd like to get merged before I cut a new point release.

@Hjdskes
Copy link
Collaborator

Hjdskes commented Jan 11, 2020

I've merged the rendering work today. If you can, please give it a test (especially over multiple outputs) so that we can get a new point release out.

@primeos
Copy link
Contributor Author

primeos commented Jan 12, 2020

@Hjdskes cool :) I gave it a test today and it failed on one of my systems (with or without my second monitor).

I initially planned to open an extra issue but it seems like a wlroots bug and also affects sway. I'll include the information in this comment but it's probably not relevant for cage:

Tested commit: e3f9959

When I run cage from a virtual terminal (e.g. cage alacritty) it fails with the following error messages (with or without my second display):

2020-01-12 21:38:23 - [EGL] command: eglInitialize, error: 0x3001, message: "DRI2: failed to add configs"
2020-01-12 21:38:23 - [EGL] command: eglInitialize, error: 0x3001, message: "eglInitialize"
2020-01-12 21:38:23 - [render/egl.c:190] Failed to initialize EGL
2020-01-12 21:38:23 - [EGL] command: eglMakeCurrent, error: 0x3008, message: "Invalid display (nil)"
2020-01-12 21:38:23 - [render/wlr_renderer.c:221] Could not initialize EGL
2020-01-12 21:38:23 - [backend/drm/renderer.c:40] Failed to create EGL/WLR renderer
2020-01-12 21:38:23 - [backend/drm/backend.c:203] Failed to initialize renderer
2020-01-12 21:38:23 - [backend/backend.c:198] Failed to open DRM device 11
2020-01-12 21:38:23 - [backend/backend.c:343] Failed to open any DRM device
2020-01-12 21:38:23 - [../cage.c:209] Unable to create the wlroots backend

This happened on my old desktop system with an Radeon HD 6950 (radeon driver).

When I run cage from an existing Wayland session instead of a tty it still runs fine.

Relevant issues: swaywm/sway#4898

@primeos
Copy link
Contributor Author

primeos commented Jan 14, 2020

@Hjdskes turns out this bug was due to an too old Mesa version (sorry about that), I could successfully test it now :)

Regarding the multiple outputs: I didn't have a look at the source-code yet, but IMO it would be nice to have some control over it (via CLI parameters, environment variables, or a configuration file) - if this isn't already possible (and maybe also an option to select only one monitor).

On my old desktop setup I have a secondary monitor with a lower resolution, with is normally fine. But when I use cage it extends windows over both monitors as if they would have the same resolution. As a result parts of the windows aren't visible anymore. In my case the secondary monitor with the lower resolution contains the left part and my primary monitor the right part of the window. But since the "left" monitor has a lower resolution I have no way to see the bottom left part of the window.

Visually this looks approximately like this (not to scale):

+---------------------------------+---------------------------------------+
|                                 |                                       |
|                                 |                                       |
|                                 |    Primary monitor (FHD)              |
|                                 |                                       |
|            Secondary            |                                       |
|             monitor             |                                       |
|            (smaller)            |                                       |
|                                 |                                       |
|                                 |                                       |
|                                 |                                       |
+---------------------------------+                                       |
|                                 |                                       |
|                                 |                                       |
|  Invisible part of the window   |                                       |
|                                 |                                       |
|                                 |                                       |
+---------------------------------+---------------------------------------+

Also my secondary monitor is physically positioned right of my primary monitor while cage does position it left of the other monitor (therefore this should probably be somehow configurable).

Another issue I've noticed is that the Firefox window (experimental Wayland support) is now consistently too small, filling only ~10% of my secondary monitor.

Hope this helps :)

@Hjdskes
Copy link
Collaborator

Hjdskes commented Jan 14, 2020

@kennylevinsen Do you know upfront what to do here? I can have a look but if you know already then that speeds things up 😃

@kennylevinsen
Copy link
Contributor

There are three separate issues here:

  1. Output layout configuration. This is primarily output positioning, but resolution and scaling may also be exposed. This requires us to have a config file, or some fancy command-line arguments.

  2. Window positioning. wlr_layer_shell and fullscreen_shell allows applications to select an output to render on, but this is not the case for xdg_shell.

Ignoring a rule engine (akin to sway's for_window), as well as IPC, the following options are what I would consider to make sense for xdg_shell:

  • Span everything, which is what we do now.
  • Only show on the first output. Other outputs will only be used by fullscreen_shell/wlr_layer_shell.
  • Let each output show one top-level, so that N outputs show the N last top-levels.

The first two are very simple, and just a matter of toggling the view size calculation. The last one requires a little bit more logic, as views has to be rearranged when created/destroyed so that the 3 last top-levels are always shown on each their own output.

These two are just the outstanding tasks from the multi-output PR. I think the main "blocker" here is that it increases the amount of configuration for cage quite significantly, so we should probably introduce a config file first. I didn't want to piggyback introduction of a configuration subsystem in the original PR. :)

  1. Firefox not sizing itself properly. Not sure what's going on here, might just be Firefox being weird as always.

@primeos Just for some user feedback, exactly behavior where you looking for in the multi-output case? Only one monitor being used?

@primeos
Copy link
Contributor Author

primeos commented Jan 15, 2020

@primeos Just for some user feedback, exactly behavior where you looking for in the multi-output case? Only one monitor being used?

@kennylevinsen good question... :)
I actually don't need the multi-output support from cage for my use-case so I guess it would work either way for me. But the ability to chose a specific monitor/output might be useful in some cases to avoid having to disconnect all other outputs/monitors.

  1. Firefox not sizing itself properly. Not sure what's going on here, might just be Firefox being weird as always.

Yes, that sounds likely :) I just noticed that it now happens consistently with cage while it did only happen occasionally with the current stable release. But it's probably best to ignore it and wait for a Firefox fix since all other applications did behave as expected.

@Hjdskes
Copy link
Collaborator

Hjdskes commented Jan 18, 2020

Responding to both of @kennylevinsen and @primeos here, since you both kind of said the same thing.

On my old desktop setup I have a secondary monitor with a lower resolution, with is normally fine. But when I use cage it extends windows over both monitors as if they would have the same resolution.

the following options are what I would consider to make sense for xdg_shell:

  • Span everything, which is what we do now.
  • Only show on the first output. Other outputs will only be used by fullscreen_shell/wlr_layer_shell.
  • Let each output show one top-level, so that N outputs show the N last top-levels.

This is Cage's use-case: to span a single application across the available outputs. I was hoping there would be a way to fix this in wlroots and to dynamically resize clients appropriately during rendering, but it doesn't look like there is. I don't think changing Cage's behavior in multi-monitor setups is the right way to go, since it would detract from what Cage is meant to be. Therefore, Cage will have to require monitors with identical resolutions and orientations so that it can span everything across the available outputs correctly. I will add this to the documentation somewhere.

Also my secondary monitor is physically positioned right of my primary monitor while cage does position it left of the other monitor (therefore this should probably be somehow configurable).
...
IMO it would be nice to have some control over it (via CLI parameters, environment variables, or a configuration file) - if this isn't already possible (and maybe also an option to select only one monitor).

Output layout configuration. This is primarily output positioning, but resolution and scaling may also be exposed. This requires us to have a config file, or some fancy command-line arguments.

Configuring the output layout is something I'm willing to have, but I wonder if Cage is the right place to have this. I'd rather have Cage support the wlr-output-management protocol such that something like kanshi can be used for this.

@kennylevinsen
Copy link
Contributor

This is Cage's use-case: to span a single application across the available outputs.

Sounds good - wlr_layer_shell or fullscreen_shell can be used if/when implemented for output addressing, in case a single application wishes to handle outputs independently (which is perfectly valid).

Cage will have to require monitors with identical resolutions and orientations so that it can span everything across the available outputs correctly.

I don't think that restrictions on the output configuration makes any sense. Grid configurations are common, and it is easy to make a "perfect" grid layout with multiple resolutions, such as one 4k screen beside two stacked 2k screens. And even then, if a user wants to make abstract art out of their displays, let them.

Maybe just state that the application will be sized after the bounding box of the layout, and that some parts of an application surface may be hidden if the layout is imperfect?

I'd rather have Cage support the wlr-output-management protocol such that something like kanshi can be used for this.

I like the idea. Although, we would probably need to document it with an example, as I suppose people would be confused by the lack of built-in configuration.

@Hjdskes
Copy link
Collaborator

Hjdskes commented Jan 18, 2020

Maybe just state that the application will be sized after the bounding box of the layout, and that some parts of an application surface may be hidden if the layout is imperfect?

Yes, this is what I meant to say but didn't. Thanks for writing it down properly :)

I like the idea. Although, we would probably need to document it with an example, as I suppose people would be confused by the lack of built-in configuration.

Sure, that's fine. There's a bunch of documentation to be written anyway :D

@ainola
Copy link

ainola commented Jul 16, 2020

Hello, Hjdskes! wlroots 0.11 has been released and with it there are more changes that need to be pushed out for cage to still work. Do you expect the project will be able to address these?

(I'm not prodding you with expectations - this is a genuine question for Arch packaging!).

@jubalh
Copy link
Contributor

jubalh commented Jul 16, 2020

Same question here from the openSUSE side ;)

@dvzrv
Copy link

dvzrv commented Jul 16, 2020

@Hjdskes I would like to add, that it would be beneficial to have signed commits from your side on master, so that integrating patches such as #145 will become easier.
For packaging on Arch Linux we currently require signed commits, as we can not ship the latest tagged release of cage (due to the wlroots incompatibilities). The latest commit we can currently use for packaging is fc55646 27391f1 (edited, as the previously mentioned commit is signed using the github PGP key which is unusable for us).

Thanks and keep up the good work!

@kennylevinsen
Copy link
Contributor

@Hjdskes I would like to add, that it would be beneficial to have signed commits from your side on master, so that integrating patches such as #145 will become easier.

@dvzrv Please create a separate issue for this.

@Hjdskes
Copy link
Collaborator

Hjdskes commented Jul 16, 2020

Would you like to see a release straight for wlroots 0.11? I'm working on Cage 0.2 with larger changes so while I can cut a release to ease packaging, it won't be a very polished release with up to date documentation. The alternative is to wait a little longer until I am ready with Cage 0.2, or for the community to provide the documentation updates (i.e., the man page, the wiki and website can be updated later).

@dvzrv
Copy link

dvzrv commented Jul 16, 2020

A (patch level) release being able to target wlroots 0.11.0 (i.e. with #145 included) would be very much appreciated! Thank you!

@Hjdskes
Copy link
Collaborator

Hjdskes commented Jul 16, 2020

Done! https://github.com/Hjdskes/cage/releases/tag/v0.1.2

@Hjdskes Hjdskes closed this as completed Jul 16, 2020
@Hjdskes
Copy link
Collaborator

Hjdskes commented Jul 16, 2020

@dvzrv The commit signing should also be fixed, thanks for giving me the incentive to look into it properly. If it happens again, please open a new issue for that.

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

No branches or pull requests

7 participants