Mir

include/client/mir_toolkit/mir_window.h (+9/-0)
src/client/mir_surface.cpp (+5/-0)
src/client/mir_surface.h (+1/-0)
src/client/mir_surface_api.cpp (+14/-0)
src/client/symbols.map (+2/-0)
src/protobuf/mir_protobuf.proto (+1/-0)
tests/acceptance-tests/CMakeLists.txt (+1/-0)
tests/acceptance-tests/client_mediated_user_gestures.cpp (+259/-0)
tests/acceptance-tests/drag_and_drop.cpp (+1/-0)

To merge this branch:

bzr merge lp:~alan-griffiths/mir/client-initiates-user-move-and-resize

Related bugs:

Bug #1420334: [enhancement] Missing client API for relative surface movement (e.g. dragging client-decorated windows)	Medium	Fix Released
Bug #1669524: GTK window functions `Always on Top, Move and Resize' don't work in Mir/Unity8	High	In Progress

Link a bug report

Reviewer	Review Type	Date Requested	Status
Mir CI Bot	continuous-integration		Approve on 2017-03-29
Gerry Boland			Abstain on 2017-03-28
Daniel van Vugt		2017-03-24	Abstain on 2017-03-28
Chris Halse Rogers			Approve on 2017-03-27
Andreas Pokorny (community)			Approve on 2017-03-27
Review via email: mp+320917@code.launchpad.net

Commit Message

Client side and initial tests for a client initiating a user move gesture

Mir CI Bot (mir-ci-bot) wrote on 2017-03-24:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3238/rebuild

review: Approve (continuous-integration)

Daniel van Vugt (vanvugt) wrote on 2017-03-27:

I've linked the 'move' bug 1420334.

But why write more code for client-initiated resizing? That already works :)

review: Needs Information

Daniel van Vugt (vanvugt) wrote on 2017-03-27:

As an example of a self-resizing client, try 'tiled' which has a resize grip in the bottom right corner. It already works.

Alan Griffiths (alan-griffiths) wrote on 2017-03-27:

> I've linked the 'move' bug 1420334.
>
> But why write more code for client-initiated resizing? That already works :)

No it doesn't. ;)

I agree clients can resize, but they can't request the shell to initiate a resize gesture. There is no way to invoke the touch resize handles under U8, drag the left edge or top.

Daniel van Vugt (vanvugt) wrote on 2017-03-27:

That's true, but the feature you're talking about in Unity7 also does not have resize handles (right click titlebar and then Resize, or just Alt+F8).

So either the client initiates the resize, and that works perfectly already; or
the server initiates resize, and that works perfectly already too.

Even if you did want it to pop up the resize handles, that's still a shell operation since the window menu itself is part of the shell. Not part of the client.

I think this branch should be just about movement.

Andreas Pokorny (andreas-pokorny) wrote on 2017-03-27:

There is a difference between submitting a buffer with a different size, and resizing a window. So if we want to support CSD, this change is necessary.

review: Approve

Gerry Boland (gerboland) wrote on 2017-03-27:

How is this API to be used by the client? I.e. where is the client specifying the new size/position?

Is it that the client gets a mouseDown event with a cookie that it decides should cause a resize/move, so calls the mir_window_request_user_resize with that cookie. Then
(a) shell intercepts all other input events, instead sending resize events to the client? Or
(b) shell keep sending the mouseMove events to the client, and client resizes its window/buffer as it pleases (WM can restrict those choices though)?
On resize completion, mouseUp event finally sent to client?

If a user wants to resize the top-left corner of a window, is that a move & resize? Should that be an atomic operation instead?

The only reason I see that Mir's current window resize api is insufficient for client-size-initiated-resize is the cursor - we want the cursor to be the shell-themed resize image, and to remain that image even if the cursor moves beyond the window borders.

review: Needs Information

Alan Griffiths (alan-griffiths) wrote on 2017-03-27:

> How is this API to be used by the client? I.e. where is the client specifying
> the new size/position?

The client isn't specifying the position or size. It is requesting the shell treats an input event as the beginning of a gesture.

> Is it that the client gets a mouseDown event with a cookie that it decides
> should cause a resize/move, so calls the mir_window_request_user_resize with
> that cookie. Then
> (a) shell intercepts all other input events, instead sending resize events to
> the client? Or
> (b) shell keep sending the mouseMove events to the client, and client resizes
> its window/buffer as it pleases (WM can restrict those choices though)?
> On resize completion, mouseUp event finally sent to client?

(a)

> If a user wants to resize the top-left corner of a window, is that a move &
> resize? Should that be an atomic operation instead?

That's an atomic operation in those shells that currently support it. Initiating the resize through a client API shouldn't change that.

> The only reason I see that Mir's current window resize api is insufficient for
> client-size-initiated-resize is the cursor - we want the cursor to be the
> shell-themed resize image, and to remain that image even if the cursor moves
> beyond the window borders.

Hmm, I can see it will be easier to reach consensus on the USER_MOVE codepath. Maybe I should split that out?

William Hua (attente) wrote on 2017-03-27:

Andreas is right, even if we can change the buffer size, this isn't the same as requesting a resize operation from an input event.

In gtk, we have this api we're expected to implement:

https://git.gnome.org/browse/gtk+/tree/gdk/gdkwindow.c#n8649

So for mir_window_request_user_resize(), I'd expect it to be able to at least accept a directional argument that allows us to do a fixed resize in a certain direction. I believe there should also be special semantics if the client doesn't specify any direction, where the server should decide that based on the initial mouse motion or initial arrow key press.

I wouldn't expect the client to get a mouse up event on completion of the resize, or any events other than resize ones during the client's resizing. So from Gerry's comment, (a) is the most sensible for us in gtk, especially from the api's point of view. If there's non-resizable windows, the server should already know that from the time the window was created, and just reject any resize operation in that case.

It also needs to be implemented so that the user can also resize using the keyboard arrow keys as well.

Alan Griffiths (alan-griffiths) wrote on 2017-03-27:

> Hmm, I can see it will be easier to reach consensus on the USER_MOVE codepath.
> Maybe I should split that out?

OK, I'm dropping the resize code from this MP as it clearly needs further consideration.

Mir CI Bot (mir-ci-bot) wrote on 2017-03-27:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3253/rebuild

review: Approve (continuous-integration)

Chris Halse Rogers (raof) wrote on 2017-03-27:

This seems sensible to me.

review: Approve

Daniel van Vugt (vanvugt) wrote on 2017-03-28:

*shrug*

My request has been satisfied but I would hazard a guess this functionality may take us a few attempts to cover all the major use cases.

review: Abstain

Daniel van Vugt (vanvugt) wrote on 2017-03-28:

I'm fairly sure we need zero more work done on resizing. All the resizing use cases are covered by existing APIs.

On the topic of client-initiated movement though, please start thinking about the Chrome/Chromium use case too: You drag a tab out of the window, that then creates a new window where the cursor remains on the same relative tab coordinate in the new window. If it's not baked in to the drag action then we'll need to be able to create that new top-level window at a precise coordinate relative to the old window (which may be the parent but can also be closed leaving just the child open).

Gerry Boland (gerboland) wrote on 2017-03-28:

No objections here

review: Abstain

Mir CI Bot (mir-ci-bot) wrote on 2017-03-29:

review: Needs Fixing (continuous-integration)

Daniel van Vugt (vanvugt) wrote on 2017-03-29:

04:02:04 Text conflict in src/client/mir_surface.cpp
04:02:04 1 conflicts encountered.

lp:~alan-griffiths/mir/client-initiates-user-move-and-resize updated on 2017-03-29

4116. By Alan Griffiths on 2017-03-29: merge --weave :parent

Mir CI Bot (mir-ci-bot) wrote on 2017-03-29:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3277/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Alan Griffiths

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'include/client/mir_toolkit/mir_window.h'
 --- include/client/mir_toolkit/mir_window.h	2017-03-14 04:41:33 +0000
 +++ include/client/mir_toolkit/mir_window.h	2017-03-29 08:37:58 +0000
@@ -738,6 +738,15 @@
  void mir_window_raise(MirWindow* window, MirCookie const* cookie);
  /**
++ * Informs the window manager that the user is moving the window.
++ *
++ * \param [in] window  The window to move
++ * \param [in] cookie  A cookie instance obtained from an input event.
++ *                     An invalid cookie will terminate the client connection.
++ */
++void mir_window_request_user_move(MirWindow* window, MirCookie const* cookie);
++
++/**
   * Get the type (purpose) of a window.
   *   \param [in] window   The window to query
   *   \return              The type of the window
 === modified file 'src/client/mir_surface.cpp'
 --- src/client/mir_surface.cpp	2017-03-28 14:28:26 +0000
 +++ src/client/mir_surface.cpp	2017-03-29 08:37:58 +0000
@@ -588,6 +588,11 @@
      request_operation(cookie, mp::RequestOperation::MAKE_ACTIVE);
+ }
++void MirSurface::request_user_move(MirCookie const* cookie)
++{
++    request_operation(cookie, mp::RequestOperation::USER_MOVE);
++}
++
  void MirSurface::request_drag_and_drop(MirCookie const* cookie)
+ {
      request_operation(cookie, mp::RequestOperation::START_DRAG_AND_DROP);
 === modified file 'src/client/mir_surface.h'
 --- src/client/mir_surface.h	2017-03-28 14:28:26 +0000
 +++ src/client/mir_surface.h	2017-03-29 08:37:58 +0000
@@ -200,6 +200,7 @@
      MirWaitHandle* set_preferred_orientation(MirOrientationMode mode);
      void raise_surface(MirCookie const* cookie);
++    void request_user_move(MirCookie const* cookie);
      void request_drag_and_drop(MirCookie const* cookie);
      void set_drag_and_drop_start_handler(std::function<void(MirWindowEvent const*)> const& callback);
 === modified file 'src/client/mir_surface_api.cpp'
 --- src/client/mir_surface_api.cpp	2017-03-14 04:41:33 +0000
 +++ src/client/mir_surface_api.cpp	2017-03-29 08:37:58 +0000
@@ -695,6 +695,20 @@
+     }
+ }
++void mir_window_request_user_move(MirWindow* window, MirCookie const* cookie)
++{
++    mir::require(mir_window_is_valid(window));
++
++    try
++    {
++        window->request_user_move(cookie);
++    }
++    catch (std::exception const& ex)
++    {
++        MIR_LOG_UNCAUGHT_EXCEPTION(ex);
++    }
++}
++
  MirWindowType mir_window_get_type(MirWindow* window)
+ {
      MirWindowType type = mir_window_type_normal;
 === modified file 'src/client/symbols.map'
 --- src/client/symbols.map	2017-03-28 11:32:58 +0000
 +++ src/client/symbols.map	2017-03-29 08:37:58 +0000
@@ -604,6 +604,8 @@
      mir_keyboard_event_key_text;
      mir_output_get_logical_height;
      mir_output_get_logical_width;
++    mir_window_request_user_move;
++    mir_window_request_user_resize;
      mir_touchscreen_config_get_mapping_mode;
      mir_touchscreen_config_get_output_id;
      mir_touchscreen_config_set_mapping_mode;
 === modified file 'src/protobuf/mir_protobuf.proto'
 --- src/protobuf/mir_protobuf.proto	2017-03-23 15:00:37 +0000
 +++ src/protobuf/mir_protobuf.proto	2017-03-29 08:37:58 +0000
@@ -443,6 +443,7 @@
    UNKNOWN = 0;
    START_DRAG_AND_DROP = 1;
    MAKE_ACTIVE = 2;
++  USER_MOVE = 3;
+ }
  message RequestWithAuthority {
 === modified file 'tests/acceptance-tests/CMakeLists.txt'
 --- tests/acceptance-tests/CMakeLists.txt	2017-03-15 09:30:18 +0000
 +++ tests/acceptance-tests/CMakeLists.txt	2017-03-29 08:37:58 +0000
@@ -3,6 +3,7 @@
  set(
    SOURCES
++  client_mediated_user_gestures.cpp
    drag_and_drop.cpp
    # Catch double-free bugs by wrapping close() and abort()ing on EBADF
 === added file 'tests/acceptance-tests/client_mediated_user_gestures.cpp'
 --- tests/acceptance-tests/client_mediated_user_gestures.cpp	1970-01-01 00:00:00 +0000
 +++ tests/acceptance-tests/client_mediated_user_gestures.cpp	2017-03-29 08:37:58 +0000
@@ -0,0 +1,259 @@
++/*
++ * Copyright © 2017 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Alan Griffiths <alan@octopull.co.uk>
++ */
++
++#include <mir_toolkit/mir_window.h>
++#include <mir_toolkit/mir_blob.h>
++
++#include <mir/geometry/displacement.h>
++#include <mir/input/input_device_info.h>
++#include <mir/input/device_capability.h>
++#include <mir/shell/shell.h>
++
++#include <mir_test_framework/connected_client_with_a_window.h>
++#include <mir_test_framework/fake_input_device.h>
++#include <mir_test_framework/stub_server_platform_factory.h>
++#include <mir/test/event_factory.h>
++#include <mir/test/signal.h>
++
++#include <gmock/gmock.h>
++#include <gtest/gtest.h>
++
++#include <linux/input.h>
++
++#include <atomic>
++
++using namespace std::chrono_literals;
++using namespace mir::geometry;
++using namespace testing;
++using mir::test::Signal;
++
++namespace
++{
++class Cookie
++{
++public:
++    Cookie() = default;
++
++    explicit Cookie(MirCookie const* cookie) : self{cookie, deleter} {}
++
++    operator MirCookie const*() const { return self.get(); }
++
++    auto get() const -> MirCookie const* { return self.get(); }
++
++    void reset() { self.reset(); }
++
++    void reset(MirCookie const* cookie) { self.reset(cookie, deleter); }
++
++private:
++    static void deleter(MirCookie const* cookie) { mir_cookie_release(cookie); }
++
++    std::shared_ptr<MirCookie const> self;
++};
++
++void mir_cookie_release(Cookie const&) = delete;
++
++struct MouseMoverAndFaker
++{
++    void start_dragging_mouse()
++    {
++        using namespace mir::input::synthesis;
++        fake_mouse->emit_event(a_button_down_event().of_button(BTN_LEFT));
++    }
++
++    void move_mouse(Displacement const& displacement)
++    {
++        using mir::input::synthesis::a_pointer_event;
++        fake_mouse->emit_event(a_pointer_event().with_movement(displacement.dx.as_int(), displacement.dy.as_int()));
++    }
++
++    void release_mouse()
++    {
++        using namespace mir::input::synthesis;
++        fake_mouse->emit_event(a_button_up_event().of_button(BTN_LEFT));
++    }
++
++private:
++    std::unique_ptr<mir_test_framework::FakeInputDevice> fake_mouse{
++        mir_test_framework::add_fake_input_device(
++            mir::input::InputDeviceInfo{"mouse", "mouse-uid", mir::input::DeviceCapability::pointer})
++    };
++};
++
++Rectangle const screen_geometry{{0,   0}, {800, 600}};
++auto const receive_event_timeout = 90s;
++
++struct ClientMediatedUserGestures : mir_test_framework::ConnectedClientWithAWindow,
++                                    MouseMoverAndFaker
++{
++    void SetUp() override
++    {
++        initial_display_layout({screen_geometry});
++        mir_test_framework::ConnectedClientWithAWindow::SetUp();
++        mir_window_set_event_handler(window, &window_event_handler, this);
++
++        paint_window();
++
++        center_mouse();
++    }
++
++    void TearDown() override
++    {
++        reset_window_event_handler();
++        mir_test_framework::ConnectedClientWithAWindow::TearDown();
++    }
++
++    auto user_initiates_gesture() -> Cookie;
++
++private:
++    void center_mouse();
++    void paint_window();
++    void set_window_event_handler(std::function<void(MirEvent const* event)> const& handler);
++    void reset_window_event_handler();
++    void invoke_window_event_handler(MirEvent const* event)
++    {
++        std::lock_guard<decltype(window_event_handler_mutex)> lock{window_event_handler_mutex};
++        window_event_handler_(event);
++    }
++
++    std::mutex window_event_handler_mutex;
++    std::function<void(MirEvent const* event)> window_event_handler_ = [](MirEvent const*) {};
++
++    static void window_event_handler(MirWindow* window, MirEvent const* event, void* context);
++};
++
++void ClientMediatedUserGestures::set_window_event_handler(std::function<void(MirEvent const* event)> const& handler)
++{
++    std::lock_guard<decltype(window_event_handler_mutex)> lock{window_event_handler_mutex};
++    window_event_handler_ = handler;
++}
++
++void ClientMediatedUserGestures::reset_window_event_handler()
++{
++    std::lock_guard<decltype(window_event_handler_mutex)> lock{window_event_handler_mutex};
++    window_event_handler_ = [](MirEvent const*) {};
++}
++
++void ClientMediatedUserGestures::window_event_handler(MirWindow* /*window*/, MirEvent const* event, void* context)
++{
++    static_cast<ClientMediatedUserGestures*>(context)->invoke_window_event_handler(event);
++}
++
++void ClientMediatedUserGestures::paint_window()
++{
++    Signal have_focus;
++
++    set_window_event_handler([&](MirEvent const* event)
++        {
++            if (mir_event_get_type(event) != mir_event_type_window)
++                return;
++
++            auto const window_event = mir_event_get_window_event(event);
++            if (mir_window_event_get_attribute(window_event) != mir_window_attrib_focus)
++                return;
++
++            if (mir_window_event_get_attribute_value(window_event))
++                have_focus.raise();
++        });
++
++    mir_buffer_stream_swap_buffers_sync(mir_window_get_buffer_stream(window));
++
++    EXPECT_THAT(have_focus.wait_for(receive_event_timeout), Eq(true));
++
++    reset_window_event_handler();
++}
++
++void ClientMediatedUserGestures::center_mouse()
++{
++    Signal have_mouseover;
++
++    set_window_event_handler([&](MirEvent const* event)
++        {
++            if (mir_event_get_type(event) != mir_event_type_input)
++                return;
++
++            auto const input_event = mir_event_get_input_event(event);
++
++            if (mir_input_event_get_type(input_event) != mir_input_event_type_pointer)
++                return;
++
++            auto const pointer_event = mir_input_event_get_pointer_event(input_event);
++
++            if (mir_pointer_event_action(pointer_event) != mir_pointer_action_enter)
++                return;
++
++            have_mouseover.raise();
++        });
++
++    move_mouse(0.5 * as_displacement(screen_geometry.size));
++
++// We miss the "mouseover" occasionally (with valgrind and heavy stress about 1/20).
++// But it isn't essential for the test and we've probably waited long enough
++// for the mouse-down needed by the test to reach the window.
++//    EXPECT_THAT(have_mouseover.wait_for(receive_event_timeout), Eq(true));
++    have_mouseover.wait_for(receive_event_timeout);
++
++    reset_window_event_handler();
++}
++
++auto ClientMediatedUserGestures::user_initiates_gesture() -> Cookie
++{
++    Cookie cookie;
++    Signal have_cookie;
++
++    set_window_event_handler([&](MirEvent const* event)
++        {
++            if (mir_event_get_type(event) != mir_event_type_input)
++                return;
++
++            auto const input_event = mir_event_get_input_event(event);
++
++            if (mir_input_event_get_type(input_event) != mir_input_event_type_pointer)
++                return;
++
++            auto const pointer_event = mir_input_event_get_pointer_event(input_event);
++
++            if (mir_pointer_event_action(pointer_event) != mir_pointer_action_button_down)
++                return;
++
++            cookie = Cookie{mir_input_event_get_cookie(input_event)};
++            have_cookie.raise();
++        });
++
++    start_dragging_mouse();
++
++    EXPECT_THAT(have_cookie.wait_for(receive_event_timeout), Eq(true));
++
++    reset_window_event_handler();
++    return cookie;
++}
++}
++
++TEST_F(ClientMediatedUserGestures, when_user_initiates_gesture_client_receives_cookie)
++{
++    auto const cookie = user_initiates_gesture();
++
++    EXPECT_THAT(cookie.get(), NotNull());
++}
++
++// TODO extend this test when server side implemented
++TEST_F(ClientMediatedUserGestures, when_client_initiates_move_nothing_bad_happens)
++{
++    auto const cookie = user_initiates_gesture();
++
++    mir_window_request_user_move(window, cookie);
++}
 === modified file 'tests/acceptance-tests/drag_and_drop.cpp'
 --- tests/acceptance-tests/drag_and_drop.cpp	2017-03-17 15:32:15 +0000
 +++ tests/acceptance-tests/drag_and_drop.cpp	2017-03-29 08:37:58 +0000
@@ -213,6 +213,7 @@
  void DragAndDrop::reset_window_event_handler(MirWindow* window)
+ {
++    std::lock_guard<decltype(window_event_handler_mutex)> lock{window_event_handler_mutex};
      if (window == this->window) window_event_handler_ = [](MirEvent const*) {};
      if (window == target_window) target_window_event_handler_ = [](MirEvent const*) {};
+ }

Mir

Merge lp:~alan-griffiths/mir/client-initiates-user-move-and-resize into lp:mir

Commit Message

Description of the Change

Preview Diff

Subscribers