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

output: do not always terminate when last output is destroyed #274

Merged
merged 2 commits into from
Sep 1, 2023

Conversation

joggee-fr
Copy link
Collaborator

Only terminate if the last output was nested under the Wayland or X11 backend. If not, using DRM backend for example, terminating Cage when unplugging the last monitor or simply turning it off does not seem to be the right behavior.

Proposed as a draft for now.
It addresses issue #208 .

Only terminate if the last output was nested under the Wayland or X11
backend. If not, using DRM backend for example, terminating Cage when
unplugging the last monitor or simply turning it off does not seem to be
the right behavior.
@sodface
Copy link

sodface commented Aug 31, 2023

Tested on one machine with a single monitor. Cage continues running when the monitor cable is disconnected then reconnected however screen resolution changes from 1920x1080 to 720x400.

@joggee-fr
Copy link
Collaborator Author

@sodface
Do you know what is the preferred mode of your monitor? wlr-randr can show you this value for example.

@sodface
Copy link

sodface commented Aug 31, 2023

wlr-randr output:

HDMI-A-1 "HP Inc. HP VH240a 6CM0101W00 (HDMI-A-1)"
  Make: HP Inc.
  Model: HP VH240a
  Serial: 6CM0101W00
  Physical size: 530x300 mm
  Enabled: yes
  Modes:
    1920x1080 px, 60.000000 Hz (preferred, current)
    1920x1080 px, 60.000000 Hz
    1920x1080 px, 59.939999 Hz
    1920x1080 px, 50.000000 Hz
    1920x1080 px, 50.000000 Hz
    1680x1050 px, 59.882999 Hz
    1600x900 px, 60.000000 Hz
    1280x1024 px, 60.020000 Hz
    1440x900 px, 59.901001 Hz
    1280x800 px, 59.910000 Hz
    1280x720 px, 60.000000 Hz
    1280x720 px, 60.000000 Hz
    1280x720 px, 59.939999 Hz
    1280x720 px, 50.000000 Hz
    1024x768 px, 60.004002 Hz
    800x600 px, 60.317001 Hz
    720x576 px, 50.000000 Hz
    720x576 px, 50.000000 Hz
    720x480 px, 60.000000 Hz
    720x480 px, 60.000000 Hz
    720x480 px, 59.939999 Hz
    720x480 px, 59.939999 Hz
    640x480 px, 60.000000 Hz
    640x480 px, 59.939999 Hz
    640x480 px, 59.939999 Hz
    720x400 px, 70.082001 Hz
  Position: 0,0
  Transform: normal
  Scale: 1.000000
  Adaptive Sync: disabled

@joggee-fr
Copy link
Collaborator Author

joggee-fr commented Aug 31, 2023

I was expecting the output to be configured in the preferred mode when handled as new. Do you have logs of Cage built in debug mode?

@sodface
Copy link

sodface commented Aug 31, 2023

See attached.

debug.log

@joggee-fr
Copy link
Collaborator Author

Interesting lines are:
[DEBUG] [types/output/output.c:656] Tried to modeset a disabled output

wlr_output_set_mode() is called before the output to be enabled using output_enable(). I suspect it causes the issue you observed. The comment saying Outputs get enabled by the backend before firing the new_output event does not seem to be always true.

Maybe calling wlr_output_enable() before trying to set mode in handle_new_output() could fix the issue (line 288). Let me know if you could give it a try.

@sodface
Copy link

sodface commented Aug 31, 2023

That seems to have fixed it. Log attached.

debug.log

Otherwise, testing to set preferred / "best" mode will always return an
error. Consequently, output will simply be configured with last and
probably "worst" mode.
@joggee-fr joggee-fr marked this pull request as ready for review September 1, 2023 09:18
@joggee-fr
Copy link
Collaborator Author

I also had the opportunity to test using DRM backends with one or two monitors. Now, Cage does not terminate when last monitor is unplugged.

Copy link
Contributor

@emersion emersion left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@emersion emersion merged commit a769943 into cage-kiosk:master Sep 1, 2023
10 checks passed
@joggee-fr joggee-fr deleted the jg/last-output-terminate branch September 1, 2023 09:53
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

Successfully merging this pull request may close these issues.

3 participants