Cloudef / wlc

High-level Wayland compositor library
MIT License
330 stars 58 forks source link

Set OUTPUT_MODE_CURRENT flag for the active mode #95

Closed fluxchief closed 8 years ago

fluxchief commented 8 years ago

The current wlc version does not set the 'WL_OUTPUT_MODE_CURRENT' flag for any mode of my second screen which causes some trouble with sway.

Please have a look if my changes make sense (you can only have one current mode I guess), it solves my issues with sway.

Cloudef commented 8 years ago

What issue does this solve? Wlc itself does not use OUTPUT_MODE_CURRENT AFAIK (apart from helping to choose which mode to use), it's just information from drm.

fluxchief commented 8 years ago

So the issue is somewhere else? My screen OSD tells me the current resolution and refresh rate, however wlc tells me that this is not the active resolution(in fact it does not say that any resolution is active at all according to the flag). This crashes applications relying on the wl_output_listener.mode callback

fluxchief commented 8 years ago

To add some more information, this is the output wlc shows when querying the drm backend:

[main.c:33] [wlc] Connector 0 is not connected or has no modes
[main.c:33] [wlc] Connector 1 is not connected or has no modes
[main.c:33] [wlc] Connector 2 is not connected or has no modes
[main.c:35] [wlc] MODE: (3) 1680x1050 !
[main.c:35] [wlc] MODE: (3) 1280x1024 
[main.c:35] [wlc] MODE: (3) 1280x1024 
[main.c:35] [wlc] MODE: (3) 1440x900 
[main.c:35] [wlc] MODE: (3) 1440x900 
[main.c:35] [wlc] MODE: (3) 1280x960 
[main.c:35] [wlc] MODE: (3) 1152x864 
[main.c:35] [wlc] MODE: (3) 1280x720 
[main.c:35] [wlc] MODE: (3) 1024x768 
[main.c:35] [wlc] MODE: (3) 1024x768 
[main.c:35] [wlc] MODE: (3) 800x600 
[main.c:35] [wlc] MODE: (3) 800x600 
[main.c:35] [wlc] MODE: (3) 640x480 
[main.c:35] [wlc] MODE: (3) 640x480 
[main.c:35] [wlc] MODE: (3) 720x400 
[main.c:35] [wlc] MODE: (4) 1920x1080 *
[main.c:35] [wlc] MODE: (4) 1680x1050 
[main.c:35] [wlc] MODE: (4) 1600x900 
[main.c:35] [wlc] MODE: (4) 1280x1024 
[main.c:35] [wlc] MODE: (4) 1280x1024 
[main.c:35] [wlc] MODE: (4) 1280x960 
[main.c:35] [wlc] MODE: (4) 1152x864 
[main.c:35] [wlc] MODE: (4) 1280x720 
[main.c:35] [wlc] MODE: (4) 1152x720 
[main.c:35] [wlc] MODE: (4) 1024x768 
[main.c:35] [wlc] MODE: (4) 1024x768 
[main.c:35] [wlc] MODE: (4) 832x624 
[main.c:35] [wlc] MODE: (4) 800x600 
[main.c:35] [wlc] MODE: (4) 800x600 
[main.c:35] [wlc] MODE: (4) 640x480 
[main.c:35] [wlc] MODE: (4) 640x480 
[main.c:35] [wlc] MODE: (4) 720x400 
[main.c:35] [wlc] DVI-D-1 Chose mode (0) 1680x1050
[handlers.c:118] Output 1 resolution changed to 1680 x 1050

So it tells me that there is no current resolution but a prefered one, the 'Chose mode' output is generated right under my patch so I assumed that this is the right location for handling this case

Cloudef commented 8 years ago

You are actually right. The modes are sent to clients with the wl_output_send_mode in wl_output_bind function. I don't know the reason why drm doesn't give CURRENT for some configurations. But we indeed should set the current mode compositor is using to clients instead of what drm said when compositor was still starting.

The patch could be simpler though

diff --git a/lib/chck b/lib/chck
--- a/lib/chck
+++ b/lib/chck
@@ -1 +1 @@
-Subproject commit bd9e54ec54a11a7ee11947836a85d8c67ddf4801
+Subproject commit bd9e54ec54a11a7ee11947836a85d8c67ddf4801-dirty
diff --git a/src/compositor/output.c b/src/compositor/output.c
index d47782d..a588617 100644
--- a/src/compositor/output.c
+++ b/src/compositor/output.c
@@ -599,6 +599,7 @@ wlc_output_set_information(struct wlc_output *output, struct wlc_output_informat
       wlc_log(WLC_LOG_INFO, "%s Chose mode (%u) %dx%d", output->information.name.data, output->active.mode, mode->width, mode->height);
       output->mode = (struct wlc_size){ mode->width, mode->height };
       wlc_output_set_resolution_ptr(output, &output->mode);
+      mode->flags |= WL_OUTPUT_MODE_CURRENT;
    }
 }

The selection logic will always end up with WL_OUTPUT_MODE_CURRENT mode if drm had such. Otherwise it will take the first PREFERRED mode. So we can safely |= WL_OUTPUT_MODE_CURRENT; to flags. On first case its no-op, on second case it adds the missing bit.

fluxchief commented 8 years ago

I updated the PR

Cloudef commented 8 years ago

Can you squash the commits into one and then it's good. Thanks.

fluxchief commented 8 years ago

Done

Cloudef commented 8 years ago

Merged thanks.