virtual desktops can be destroyed, active clients update, but deleted
keeps a cache.
Someone needs to do cleanup to avoid dangly pointers.
zzag |
KWin |
virtual desktops can be destroyed, active clients update, but deleted
keeps a cache.
Someone needs to do cleanup to avoid dangly pointers.
I couldn't find a case of someone calling desktops mid way through an
animation, so it's only a hypothetical bug.
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
If a deleted client is/was on a single virtual desktop, then it can end up on all desktops. We probably don't want that.
Technically sure, but putting it on any arbitrary desktop is just as bad - the correct thing would be to introduce a whole new concept for a crazy edge case that I don't think can really come up.
Workspace::updateClientVisibilityOnDesktopChange doesn't include deleted. Deleted isn't emitting any signals either - so the only case we could see this be relevant is where we're viewing multiple desktops and polling dekstops.
In theory I think that could include desktop grid and desktop cube, but that would require triggering multiple actions faster than I can manage even on the slowest animation speed.
I don't want to rip out a window whilst effects are referencing it.
Most happen to connect to windowDeleted and tidy up; but I don't think we can rely on that.
Effects that reference closed windows have to connect to windowDeleted. We clearly state this in documentation.
Documentation also clearly states:
- This means that a closed window is not referenced any more.
We're proposing deleting it whilst it is referenced.
I re-read windowDeleted's documentation. It looks like it can have several meanings depending on how you read it.
I still think it would be fine to destroy Deleted even if some effects still reference it. The documentation states that effects should perform cleanup when windowDeleted is emitted, so it won't cause big problems. Though it would be great to re-clarify when windowDeleted is emitted.
Maybe @graesslin can weigh in here.
Hmmm, difficult. I think it would be wrong to delete the Deleted just because a desktop is destroyed. I don't see a problem with the deleted ending up on all desktops. No effect would start an animation and those animating the window will continue to do so.
deleted.cpp | ||
---|---|---|
144 | Please put a whitespace before : |