Add dragging window with cursor feature

[daxpedda]

• Tested on all platforms changed
• Compilation warnings were addressed
• cargo fmt has been run on this branch
• cargo doc builds successfully
• Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality
• Updated feature matrix, if new features were added or implemented

---

Partially fixes #221.

Still working on the other platforms, see #221 (comment) for details.
Would really appreciate somebody looking at the Wayland implementation, any bikeshedding is welcome too.
I've successfully tried all implementations, currently stuck at trying to fix the Windows implementation deadlock.

cargo run --example drag_window

Blocked on Smithay/client-toolkit#174.
Blocked on smithay-client-toolkit releasing a new version.

Wayland implementation at #1854.

[maroider]

Could you use a more descriptive PR title?

[daxpedda]

Could you use a more descriptive PR title?

That was an accident :/, can't think of a better formulation, suggestions welcome.

[maroider]

I don't think the currently proposed API is a good fit for the Windows backend.

For a given mouse action, the WM_NCHITTEST message will, in my experience, arrive before any other mouse events. If we try to set a flag when the user calls Window::set_drag_window and then modify the next WM_NCHITTEST result, we run the risk of introducing some unfortunate edge cases. If the window spawns beneath the user's cursor, and the user doesn't move the cursor before pressing a mouse button, then the drag won't be initiated at all.

Note that I haven't tested this, but there was a similar issue I had to solve in #1807.

[daxpedda]

@maroider I've used WM_NCLBUTTONDOWN instead, does this resolve your concern?

[daxpedda]

I wanna thank @maroider and @vberger, they were a lot of help with this PR.

If no more concerns are raised I believe the implementation is complete, we are just waiting for an update from smithay-client-toolkit.

[daxpedda]

Wayland implementation split off to #1854.
This is now ready to be merged.

EDIT: just rebased

[maroider]

Bikeshed: I don't feel like set_drag_window is a particularly good name. The other set_* methods on Window either manipulate some piece of retained state, or aren't quite as restricted in when they can be called successfully. Perhaps begin_drag would be better.

[daxpedda]

begin_drag sounds better indeed, on it.

EDIT: the Wayland implementation uses start_interactive_move, we can change it to that too if people like that more.

[daxpedda]

(Sth to discuss might be the method name as it sounds like drag-and-drop related - maybe drag_window would fit better?)

Agreed.

I intend to merge the Wayland implementation, which needs to upgrade SCTK to 0.13, this adds libfreetype-dev and libexpat-dev as a dependency on Linux, is that acceptable or should I keep it in a seperate PR for now?

[elinorbgr]

I intend to merge the Wayland implementation, which needs to upgrade SCTK to 0.13, this adds libfreetype-dev and libexpat-dev as a dependency on Linux, is that acceptable or should I keep it in a seperate PR for now?

These are actually transitive dependencies of the new dependency on libfontconfig, which is used to find a font for rendering the text on decorations. I don't expect to find many systems that don't have fontconfig. (And, of course, the -dev dependencies are only build-time dependencies, not runtime).

[ArturKovacs]

This mostly looks good regarding the macOS implementation. There is something strange I experienced though. Start the example, bring another application to the front, and while the other application is in the front, click the close button on one of the winit windows. They won't close, not even if I bring them back to the front after this.

I put some printlns into src/platform_impl/macos/app_state.rs within the block that starts with

if HANDLER.should_exit() {

Turns out that somehow the application gets stuck in a state where neither of the two windows is the "main window". The source of this dialog handling code is #1581. I didn't dig deeper, I don't really have an idea what causes this or if it's normal.

[daxpedda]

@ArturKovacs I am assuming this has nothing to do with drag_window but that the example is using multiple windows?

[ArturKovacs]

That's what I'm suspecting as well, but I couldn't reproduce it with the multithreaded example.

[daxpedda]

@vberger Do you know what the builds issue is? Apparently expat doesn't like the 32-bit C++ compiler GitHub Actions is offering?

[elinorbgr]

An option would be to just install fontconfig in the CI environment, and avoid having the -sys crates build their own versions of the system libs they are associated with (which is apparently something servo likes to do?).

[daxpedda]

So do I have to add a new Winit crate feature that activates servo-fontconfig's force_system_lib to fix this then? I can't figure out a simpler/cleaner way.

[elinorbgr]

Ah wait, looks like the lib was already installed. Looks like the issue I had with my CI: is pkgconfig installed in the CI environment? Without it the build scripts cannot find the libraries.

[daxpedda]

is pkgconfig installed in the CI environment? Without it the build scripts cannot find the libraries.

Definitely installed: https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-README.md#installed-apt-packages.

[elinorbgr]

Hmm, given the error log says /usr/bin/ld: cannot find -lstdc++, I guess some part of the multilib c++ toolchain is missing. Maybe try installing both gcc-multilib and g++-multilib, rather than just the former?

[daxpedda]

I don't even know what to do with this error message now.
@vberger I'm willing to try new ideas if you have any, but I would rather spend my time making a PR for rust_fontconfig, would you accept one?

[elinorbgr]

Hmm, I guess servo-fontconfig-sys is unhappy because it cannot find an i686 version of libfontconfig, and then tries to build its own which fails horribly...

I myself am not opposed to using rust_fontconfig as long as it works correctly, but I don't know if the winit maintainers would be okay with using such a new and not battle-tested crate.

An other option would be to completely disable writing the title in SCTK fallback decorations, I guess. Having text rendering in SCTK always struck me as quite an heavy dependency for so little gain.

[daxpedda]

..., but I don't know if the winit maintainers would be okay with using such a new and not battle-tested crate.

Should I ping the rest of the Linux maintainers for this?

An other option would be to completely disable writing the title in SCTK fallback decorations, I guess. Having text rendering in SCTK always struck me as quite an heavy dependency for so little gain.

Personally, I agree with that, do we need to aquire consensus for that, or can you decide that?

[elinorbgr]

@daxpedda To avoid blocking this further on unrelated stuff, I've released version 0.12.3 which only contains the change you need on top of 0.12.2, so you can just use that. All the stuff about fontconfig is unrelated an should be tackled elsewhere.

[daxpedda]

Alright, thats great. Thanks!

[elinorbgr]

The wayland part looks good.

[daxpedda]

That's what I'm suspecting as well, but I couldn't reproduce it with the multithreaded example.

@ArturKovacs I got my hands on a Apple machine just now and tried it, I wasn't able to reproduce the issue 😞.
I am still unsure what this could have to do with the drag_window implementation, but seeing that I don't see how I will fix this, I would rather table the MacOS implementation.

Potentially we could also just document not to use this function with multiple windows on MacOS?

[ArturKovacs]

Never mind, I could reproduce the faulty behavior on the master branch, so it's in fact completely unrelated. This can be merged as far as I'm concerned.

[ArturKovacs]

Considering that there hasn't been Windows related changes since @msiglreith approved, it seems that everyone required approved these changes. I'm merging this now.