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

core: improve dynamic output handling #386

Merged
merged 9 commits into from
Jul 1, 2024
Merged

Conversation

PaideiaDilemma
Copy link
Collaborator

@PaideiaDilemma PaideiaDilemma commented Jun 27, 2024

I reproduced some crashes with the lid switching the internal monitor on and off.
That is because the session lock surface is getting created via handleDone for a new output.
Even if a full display roundtrip is done, some events might arrive earlier than the output done event causing a crash because the sessionLockSurface pointer is null. Checking for the sessionLockSurface pointer fixes that. Other option would be to only add dynamic outputs to the m_vOutputs vector when the sessionLockSurface already exists

Then I noticed that the widgets for a surface are not removed, when it is destroyed (I think that caused #337). Sometimes the new sessionLockSurface happens to be the same pointer as the original one. And because the widgets map is keyed by those pointers it will reuse the widgets leading to some weird stuff.

[LOG]   | removed iface 60
[LOG] output 905814070 done
[LOG]   | got iface: wl_output v4
[LOG]    > Bound to wl_output v4
[LOG] output 61 done
[LOG] Creating a surface dynamically for output as we are already locked
[LOG] sessionLockSurface 0x36054330
[LOG] output 61 make null model null
[LOG] output 61 name HEADLESS-1
[LOG] output 61 description Headless output 1
[LOG] output 61 done
...
[LOG]   | got iface: wl_output v4
[LOG]    > Bound to wl_output v4
[LOG]   | removed iface 61
[LOG] output 62 make Najing CEC Panda FPD Technology CO. ltd model 0x002B
[LOG] output 62 name eDP-1
[LOG] output 62 description Najing CEC Panda FPD Technology CO. ltd 0x002B (eDP-1)
[LOG] output 62 done
[LOG] Creating a surface dynamically for output as we are already locked
[LOG] sessionLockSurface 0x36054330

Stuff that is still to be investigated:

  • DMA Frames are identified by output stringPorts, so when someone would unplug one monitor and plug another one in with different dimensions, that would cause weirdness probably. Should we identify them by description or something instead so that we don't need to remove them, or just remove existing ones when an output is removed?
  • (Hyprland) My active workspace switches when I deactivate and reactivate my only output. I can imagine that is kind of supposed to happen, but I will need to check what is actually going on.
  • (Hyprland) There is a red screen flashing, when reactivating a monitor. Also happens with swaylock. But not when replugging and external monitor.

Closes #337
Probably Closes #117

This is needed, because when a new monitor is added via `onGlobal` the
order of the events is not guaranteed. Meaning that render for a
particular monitor might get called before a `CSessionLockSurface` for
that monitor exists.
@vaxerski
Copy link
Member

or just remove existing ones when an output is removed?

prolly.

My active workspace switches when I deactivate and reactivate my only output. I can imagine that is kind of supposed to happen, but I will need to check what is actually going on.

hl quirk.

There is a red screen flashing, when reactivating a monitor. Also happens with swaylock. But not when replugging and external monitor.

hl bug.

@PaideiaDilemma
Copy link
Collaborator Author

Idk how dynamic dma frames can work.
The workspace below would need to flash for the screencopy to copy what you would want to blur.
Currently when a dynamic dma frame is created hyprlock itself gets screencopied making the new background a blur of existing widgets.

I think it might be better to just not support dynamic dma frames at all. But if one for a stringPort already exists, check if the dimensions match the ones of the new output. Otherwise fall back to the background color.

But maybe you have an idea how to make it work properly?

@vaxerski
Copy link
Member

right, you're right. Would need an extension or something. Fair. Let's not.

@PaideiaDilemma
Copy link
Collaborator Author

Alright done.

What we don't support yet is dynamically updating widgets, when mode/scale of a monitor changes for some reason. I guess we could destroy the widgets in that case and let them be recreated. But thats for another mr.

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

thanks!

@vaxerski vaxerski merged commit 88b9ce4 into hyprwm:main Jul 1, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants