From 65531a3b826c89986797861bade56750e0a1a5c0 Mon Sep 17 00:00:00 2001 From: Simon Fels Date: Thu, 8 Dec 2016 07:08:25 +0100 Subject: [PATCH] Handle window movement/resize/close on our side and not in Android This leads to much better performance and user interaction. Resizing still has some flickering but that is a different area of problems. --- .../service/activity_manager_interface.cpp | 19 ++++++ android/service/activity_manager_interface.h | 8 +++ android/service/android_api_skeleton.cpp | 33 +++++++++- android/service/android_api_skeleton.h | 10 +++ android/service/message_processor.cpp | 14 ++-- src/anbox/bridge/android_api_stub.cpp | 64 +++++++++++++++++++ src/anbox/bridge/android_api_stub.h | 7 ++ src/anbox/graphics/rect.cpp | 14 ++++ src/anbox/graphics/rect.h | 4 ++ src/anbox/protobuf/anbox_bridge.proto | 17 +++++ src/anbox/ubuntu/platform_policy.cpp | 33 ++++++++-- src/anbox/ubuntu/platform_policy.h | 2 + src/anbox/ubuntu/window.cpp | 25 ++++---- src/anbox/ubuntu/window.h | 6 +- src/anbox/wm/default_platform_policy.cpp | 5 -- src/anbox/wm/window.cpp | 6 -- src/anbox/wm/window.h | 4 -- 17 files changed, 230 insertions(+), 41 deletions(-) diff --git a/android/service/activity_manager_interface.cpp b/android/service/activity_manager_interface.cpp index 84dce86..c749a4f 100644 --- a/android/service/activity_manager_interface.cpp +++ b/android/service/activity_manager_interface.cpp @@ -31,5 +31,24 @@ status_t BpActivityManager::setFocusedTask(int32_t id) { return remote()->transact(IActivityManager::SET_FOCUSED_TASK, data, &reply); } +status_t BpActivityManager::removeTask(int32_t id) { + Parcel data, reply; + data.writeInterfaceToken(IActivityManager::getInterfaceDescriptor()); + data.writeInt32(id); + return remote()->transact(IActivityManager::REMOVE_TASK, data, &reply); +} + +status_t BpActivityManager::resizeTask(int32_t id, const anbox::graphics::Rect &rect, int resize_mode) { + Parcel data, reply; + data.writeInterfaceToken(IActivityManager::getInterfaceDescriptor()); + data.writeInt32(id); + data.writeInt32(resize_mode); + data.writeInt32(rect.left()); + data.writeInt32(rect.top()); + data.writeInt32(rect.right()); + data.writeInt32(rect.bottom()); + return remote()->transact(IActivityManager::RESIZE_TASK, data, &reply); +} + IMPLEMENT_META_INTERFACE(ActivityManager, "android.app.IActivityManager"); } // namespace android diff --git a/android/service/activity_manager_interface.h b/android/service/activity_manager_interface.h index 655fdb6..ba0c869 100644 --- a/android/service/activity_manager_interface.h +++ b/android/service/activity_manager_interface.h @@ -27,6 +27,8 @@ #include +#include "anbox/graphics/rect.h" + namespace android { class IActivityManager : public IInterface { public: @@ -35,9 +37,13 @@ public: enum { // This needs to stay synchronized with frameworks/base/core/java/android/app/IActivityManager.java SET_FOCUSED_TASK = IBinder::FIRST_CALL_TRANSACTION + 130, + REMOVE_TASK = IBinder::FIRST_CALL_TRANSACTION + 131, + RESIZE_TASK = IBinder::FIRST_CALL_TRANSACTION + 285, }; virtual status_t setFocusedTask(int32_t id) = 0; + virtual status_t removeTask(int32_t id) = 0; + virtual status_t resizeTask(int32_t id, const anbox::graphics::Rect &rect, int resize_mode) = 0; }; class BpActivityManager : public BpInterface { @@ -45,6 +51,8 @@ public: BpActivityManager(const sp &binder); status_t setFocusedTask(int32_t id) override; + status_t removeTask(int32_t id) override; + status_t resizeTask(int32_t id, const anbox::graphics::Rect &rect, int resize_mode); }; } // namespace android #endif diff --git a/android/service/android_api_skeleton.cpp b/android/service/android_api_skeleton.cpp index fa388d5..03ae1c7 100644 --- a/android/service/android_api_skeleton.cpp +++ b/android/service/android_api_skeleton.cpp @@ -119,8 +119,6 @@ void AndroidApiSkeleton::launch_application(anbox::protobuf::bridge::LaunchAppli void AndroidApiSkeleton::set_focused_task(anbox::protobuf::bridge::SetFocusedTask const *request, anbox::protobuf::rpc::Void *response, google::protobuf::Closure *done) { - (void) response; - connect_services(); if (activity_manager_.get()) @@ -130,4 +128,35 @@ void AndroidApiSkeleton::set_focused_task(anbox::protobuf::bridge::SetFocusedTas done->Run(); } + +void AndroidApiSkeleton::remove_task(anbox::protobuf::bridge::RemoveTask const *request, + anbox::protobuf::rpc::Void *response, + google::protobuf::Closure *done) { + connect_services(); + + if (activity_manager_.get()) + activity_manager_->removeTask(request->id()); + else + response->set_error("ActivityManager is not available"); + + done->Run(); + +} + +void AndroidApiSkeleton::resize_task(anbox::protobuf::bridge::ResizeTask const *request, + anbox::protobuf::rpc::Void *response, + google::protobuf::Closure *done) { + connect_services(); + + if (activity_manager_.get()) { + auto r = request->rect(); + activity_manager_->resizeTask(request->id(), + anbox::graphics::Rect{r.left(), r.top(), r.right(), r.bottom()}, + request->resize_mode()); + } else { + response->set_error("ActivityManager is not available"); + } + + done->Run(); +} } // namespace anbox diff --git a/android/service/android_api_skeleton.h b/android/service/android_api_skeleton.h index c4d2fd3..fd37840 100644 --- a/android/service/android_api_skeleton.h +++ b/android/service/android_api_skeleton.h @@ -39,6 +39,8 @@ class InstallApplication; class LaunchApplication; class SetDnsServers; class SetFocusedTask; +class RemoveTask; +class ResizeTask; } // namespace bridge namespace rpc { class Void; @@ -57,6 +59,14 @@ public: anbox::protobuf::rpc::Void *response, google::protobuf::Closure *done); + void remove_task(anbox::protobuf::bridge::RemoveTask const *request, + anbox::protobuf::rpc::Void *response, + google::protobuf::Closure *done); + + void resize_task(anbox::protobuf::bridge::ResizeTask const *request, + anbox::protobuf::rpc::Void *response, + google::protobuf::Closure *done); + private: void wait_for_process(core::posix::ChildProcess &process, anbox::protobuf::rpc::Void *response); diff --git a/android/service/message_processor.cpp b/android/service/message_processor.cpp index 0fc2054..eb1617d 100644 --- a/android/service/message_processor.cpp +++ b/android/service/message_processor.cpp @@ -15,6 +15,8 @@ * */ +#define LOG_TAG "Anboxd" + #include "android/service/message_processor.h" #include "android/service/android_api_skeleton.h" @@ -35,10 +37,14 @@ MessageProcessor::~MessageProcessor() { } void MessageProcessor::dispatch(rpc::Invocation const& invocation) { - if (invocation.method_name() == "launch_application") - invoke(this, platform_api_.get(), &AndroidApiSkeleton::launch_application, invocation); - else if (invocation.method_name() == "set_focused_task") - invoke(this, platform_api_.get(), &AndroidApiSkeleton::set_focused_task, invocation); + if (invocation.method_name() == "launch_application") + invoke(this, platform_api_.get(), &AndroidApiSkeleton::launch_application, invocation); + else if (invocation.method_name() == "set_focused_task") + invoke(this, platform_api_.get(), &AndroidApiSkeleton::set_focused_task, invocation); + else if (invocation.method_name() == "remove_task") + invoke(this, platform_api_.get(), &AndroidApiSkeleton::remove_task, invocation); + else if (invocation.method_name() == "resize_task") + invoke(this, platform_api_.get(), &AndroidApiSkeleton::resize_task, invocation); } void MessageProcessor::process_event_sequence(const std::string&) { diff --git a/src/anbox/bridge/android_api_stub.cpp b/src/anbox/bridge/android_api_stub.cpp index 05e6de1..b26d383 100644 --- a/src/anbox/bridge/android_api_stub.cpp +++ b/src/anbox/bridge/android_api_stub.cpp @@ -115,5 +115,69 @@ void AndroidApiStub::focused_task_set(Request *request) { (void)request; set_focused_task_handle_.result_received(); } + +void AndroidApiStub::remove_task(const std::int32_t &id) { + ensure_rpc_channel(); + + auto c = std::make_shared>(); + + DEBUG(""); + + protobuf::bridge::RemoveTask message; + message.set_id(id); + + { + std::lock_guard lock(mutex_); + remove_task_handle_.expect_result(); + } + + channel_->call_method("remove_task", &message, c->response.get(), + google::protobuf::NewCallback( + this, &AndroidApiStub::task_removed, c.get())); + + remove_task_handle_.wait_for_all(); + + if (c->response->has_error()) throw std::runtime_error(c->response->error()); +} + +void AndroidApiStub::task_removed(Request *request) { + (void)request; + remove_task_handle_.result_received(); +} + +void AndroidApiStub::resize_task(const std::int32_t &id, const anbox::graphics::Rect &rect, + const std::int32_t &resize_mode) { + ensure_rpc_channel(); + + auto c = std::make_shared>(); + + protobuf::bridge::ResizeTask message; + message.set_id(id); + message.set_resize_mode(resize_mode); + + auto r = message.mutable_rect(); + r->set_left(rect.left()); + r->set_top(rect.top()); + r->set_right(rect.right()); + r->set_bottom(rect.bottom()); + + { + std::lock_guard lock(mutex_); + resize_task_handle_.expect_result(); + } + + channel_->call_method("resize_task", &message, c->response.get(), + google::protobuf::NewCallback( + this, &AndroidApiStub::task_resized, c.get())); + + resize_task_handle_.wait_for_all(); + + if (c->response->has_error()) throw std::runtime_error(c->response->error()); +} + +void AndroidApiStub::task_resized(Request *request) { + (void)request; + resize_task_handle_.result_received(); +} } // namespace bridge } // namespace anbox diff --git a/src/anbox/bridge/android_api_stub.h b/src/anbox/bridge/android_api_stub.h index aa77f12..1dbde4e 100644 --- a/src/anbox/bridge/android_api_stub.h +++ b/src/anbox/bridge/android_api_stub.h @@ -20,6 +20,7 @@ #include "anbox/application_manager.h" #include "anbox/common/wait_handle.h" +#include "anbox/graphics/rect.h" #include #include @@ -45,6 +46,8 @@ class AndroidApiStub : public anbox::ApplicationManager { void launch(const android::Intent &intent) override; void set_focused_task(const std::int32_t &id); + void remove_task(const std::int32_t &id); + void resize_task(const std::int32_t &id, const anbox::graphics::Rect &rect, const std::int32_t &resize_mode); private: void ensure_rpc_channel(); @@ -58,11 +61,15 @@ class AndroidApiStub : public anbox::ApplicationManager { void application_launched(Request *request); void focused_task_set(Request *request); + void task_removed(Request *request); + void task_resized(Request *request); mutable std::mutex mutex_; std::shared_ptr channel_; common::WaitHandle launch_wait_handle_; common::WaitHandle set_focused_task_handle_; + common::WaitHandle remove_task_handle_; + common::WaitHandle resize_task_handle_; }; } // namespace bridge } // namespace anbox diff --git a/src/anbox/graphics/rect.cpp b/src/anbox/graphics/rect.cpp index 5915766..2dd5c61 100644 --- a/src/anbox/graphics/rect.cpp +++ b/src/anbox/graphics/rect.cpp @@ -32,6 +32,20 @@ void Rect::merge(const Rect &rhs) { bottom_ = std::max(bottom_, rhs.bottom()); } +void Rect::translate(const std::int32_t &x, const std::int32_t &y) { + auto old_width = width(); + auto old_height = height(); + left_ = x; + top_ = y; + right_ = x + old_width; + bottom_ = y + old_height; +} + +void Rect::resize(const std::int32_t &width, const std::int32_t &height) { + right_ = left_ + width; + bottom_ = top_ + height; +} + std::ostream &operator<<(std::ostream &out, const Rect &rect) { return out << "{" << rect.left() << "," << rect.top() << "," << rect.right() << "," << rect.bottom() << "}"; diff --git a/src/anbox/graphics/rect.h b/src/anbox/graphics/rect.h index ec26683..8c3060e 100644 --- a/src/anbox/graphics/rect.h +++ b/src/anbox/graphics/rect.h @@ -63,6 +63,10 @@ class Rect { void merge(const Rect &rhs); + void translate(const std::int32_t &x, const std::int32_t &y); + + void resize(const std::int32_t &width, const std::int32_t &height); + private: std::int32_t left_; std::int32_t top_; diff --git a/src/anbox/protobuf/anbox_bridge.proto b/src/anbox/protobuf/anbox_bridge.proto index 9c448ba..239e10d 100644 --- a/src/anbox/protobuf/anbox_bridge.proto +++ b/src/anbox/protobuf/anbox_bridge.proto @@ -16,6 +16,13 @@ message Intent { repeated string categories = 6; } +message Rect { + optional int32 left = 1; + optional int32 top = 2; + optional int32 right = 3; + optional int32 bottom = 4; +} + message Notification { required string package_name = 1; required string category = 2; @@ -32,6 +39,16 @@ message SetFocusedTask { required int32 id = 1; } +message RemoveTask { + required int32 id = 1; +} + +message ResizeTask { + required int32 id = 1; + required int32 resize_mode = 2; + required Rect rect = 3; +} + message BootFinishedEvent { } diff --git a/src/anbox/ubuntu/platform_policy.cpp b/src/anbox/ubuntu/platform_policy.cpp index e813c72..680e8e3 100644 --- a/src/anbox/ubuntu/platform_policy.cpp +++ b/src/anbox/ubuntu/platform_policy.cpp @@ -85,10 +85,6 @@ void PlatformPolicy::process_events() { while (SDL_WaitEventTimeout(&event, 100)) { switch (event.type) { case SDL_QUIT: - // This is the best way to reliable terminate the whole application - // for now. It will - // trigger a correct shutdown in the main part. - ::kill(getpid(), SIGTERM); break; case SDL_WINDOWEVENT: for (auto &iter : windows_) { @@ -198,6 +194,8 @@ void PlatformPolicy::window_deleted(const Window::Id &id) { WARNING("Got window removed event for unknown window (id %d)", id); return; } + if (auto window = w->second.lock()) + android_api_->remove_task(window->task()); windows_.erase(w); } @@ -209,6 +207,33 @@ void PlatformPolicy::window_wants_focus(const Window::Id &id) { android_api_->set_focused_task(window->task()); } +void PlatformPolicy::window_moved(const Window::Id &id, const std::int32_t &x, const std::int32_t &y) { + auto w = windows_.find(id); + if (w == windows_.end()) return; + + if (auto window = w->second.lock()) { + auto new_frame = window->frame(); + new_frame.translate(x, y); + android_api_->resize_task(window->task(), new_frame, 3); + } +} + +void PlatformPolicy::window_resized(const Window::Id &id, const std::int32_t &width, const std::int32_t &height) { + auto w = windows_.find(id); + if (w == windows_.end()) return; + + if (auto window = w->second.lock()) { + auto new_frame = window->frame(); + new_frame.resize(width, height); + // We need to update the window frame in advance here as otherwise we may + // get a movement event before we got an update of the actual layer + // representing this window and then we're back to the original size of + // the task. + window->update_frame(new_frame); + android_api_->resize_task(window->task(), new_frame, 1); + } +} + DisplayManager::DisplayInfo PlatformPolicy::display_info() const { return display_info_; } diff --git a/src/anbox/ubuntu/platform_policy.h b/src/anbox/ubuntu/platform_policy.h index 57de532..567093b 100644 --- a/src/anbox/ubuntu/platform_policy.h +++ b/src/anbox/ubuntu/platform_policy.h @@ -52,6 +52,8 @@ class PlatformPolicy : public std::enable_shared_from_this, void window_deleted(const Window::Id &id) override; void window_wants_focus(const Window::Id &id) override; + void window_moved(const Window::Id &id, const std::int32_t &x, const std::int32_t &y) override; + void window_resized(const Window::Id &id, const std::int32_t &width, const std::int32_t &height) override; DisplayInfo display_info() const override; diff --git a/src/anbox/ubuntu/window.cpp b/src/anbox/ubuntu/window.cpp index 692f910..cb4c7e1 100644 --- a/src/anbox/ubuntu/window.cpp +++ b/src/anbox/ubuntu/window.cpp @@ -45,7 +45,7 @@ Window::Window(const Id &id, const wm::Task::Id &task, window_ = SDL_CreateWindow(default_window_name, frame.left(), frame.top(), frame.width(), frame.height(), - SDL_WINDOW_OPENGL | SDL_WINDOW_BORDERLESS); + SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE); if (!window_) { const auto message = utils::string_format("Failed to create window: %s", SDL_GetError()); @@ -75,15 +75,7 @@ Window::Window(const Id &id, const wm::Task::Id &task, Window::~Window() { if (window_) SDL_DestroyWindow(window_); - if (observer_) observer_->window_deleted(id_); -} - -void Window::resize(int width, int height) { - SDL_SetWindowSize(window_, width, height); -} - -void Window::update_position(int x, int y) { - SDL_SetWindowPosition(window_, x, y); + // if (observer_) observer_->window_deleted(id_); } void Window::process_event(const SDL_Event &event) { @@ -93,16 +85,25 @@ void Window::process_event(const SDL_Event &event) { break; case SDL_WINDOWEVENT_FOCUS_LOST: break; - case SDL_WINDOWEVENT_RESIZED: - break; + // Not need to listen for SDL_WINDOWEVENT_RESIZED here as the + // SDL_WINDOWEVENT_SIZE_CHANGED is always sent. case SDL_WINDOWEVENT_SIZE_CHANGED: + if (observer_) + observer_->window_resized(id_, event.window.data1, event.window.data2); break; case SDL_WINDOWEVENT_MOVED: + if (observer_) + observer_->window_moved(id_, event.window.data1, event.window.data2); break; case SDL_WINDOWEVENT_SHOWN: break; case SDL_WINDOWEVENT_HIDDEN: break; + case SDL_WINDOWEVENT_CLOSE: + DEBUG("Got close event"); + if (observer_) + observer_->window_deleted(id_); + break; } } diff --git a/src/anbox/ubuntu/window.h b/src/anbox/ubuntu/window.h index 4e66cb4..22e0c34 100644 --- a/src/anbox/ubuntu/window.h +++ b/src/anbox/ubuntu/window.h @@ -39,6 +39,8 @@ class Window : public std::enable_shared_from_this, public wm::Window { virtual ~Observer(); virtual void window_deleted(const Id &id) = 0; virtual void window_wants_focus(const Id &id) = 0; + virtual void window_moved(const Id &id, const std::int32_t &x, const std::int32_t &y) = 0; + virtual void window_resized(const Id &id, const std::int32_t &x, const std::int32_t &y) = 0; }; Window(const Id &id, const wm::Task::Id &task, @@ -52,10 +54,6 @@ class Window : public std::enable_shared_from_this, public wm::Window { Id id() const; std::uint32_t window_id() const; - protected: - void resize(int width, int height) override; - void update_position(int x, int y) override; - private: Id id_; std::shared_ptr observer_; diff --git a/src/anbox/wm/default_platform_policy.cpp b/src/anbox/wm/default_platform_policy.cpp index 88aa831..31adf59 100644 --- a/src/anbox/wm/default_platform_policy.cpp +++ b/src/anbox/wm/default_platform_policy.cpp @@ -25,11 +25,6 @@ class NullWindow : public anbox::wm::Window { NullWindow(const anbox::wm::Task::Id &task, const anbox::graphics::Rect &frame) : anbox::wm::Window(0, frame) {} - - protected: - void resize(int width, int height) override { WARNING("Not implemented"); } - - void update_position(int x, int y) override { WARNING("Not implemented"); } }; } diff --git a/src/anbox/wm/window.cpp b/src/anbox/wm/window.cpp index 49d681e..398ebb2 100644 --- a/src/anbox/wm/window.cpp +++ b/src/anbox/wm/window.cpp @@ -31,12 +31,6 @@ void Window::update_state(const WindowState::List &states) {} void Window::update_frame(const graphics::Rect &frame) { if (frame == frame_) return; - if (frame.width() != frame_.width() || frame.height() != frame_.height()) - resize(frame.width(), frame.height()); - - if (frame.top() != frame_.top() || frame.left() != frame_.left()) - update_position(frame.left(), frame.top()); - frame_ = frame; } diff --git a/src/anbox/wm/window.h b/src/anbox/wm/window.h index 0e44e58..aba0d26 100644 --- a/src/anbox/wm/window.h +++ b/src/anbox/wm/window.h @@ -54,10 +54,6 @@ class Window { graphics::Rect frame() const; Task::Id task() const; - protected: - virtual void resize(int width, int height) = 0; - virtual void update_position(int x, int y) = 0; - private: Task::Id task_; graphics::Rect frame_;