From c60604062c089522e7ca7499ecae11dcb083e892 Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Wed, 3 Oct 2018 18:11:51 +0200 Subject: [PATCH] decorator: improve robustness of window restacking This patch improves the detection of new appearing top-most windows. Such a window should prompt the decorator to bring the corresponding nitpicker view(s) to the front of the view stack. The original implementation relied on hints provided by the layouter (the 'topped' attribute). With the patch, the decorator tracks the top-most window by itself, which improves the robustness. As a second improvement, the patch defers the destruction of windows to the point when all other window operations are completed. This hides intermediate states when replacing one window by another in one step, which is typical for console-like scenarios. Hence, this patch should eliminate flickering artifacts when switching from one virtual console to another. Issue #3031 --- repos/gems/src/app/decorator/main.cc | 5 +- repos/gems/src/app/decorator/window.cc | 4 +- repos/gems/src/app/decorator/window.h | 2 +- repos/gems/src/app/themed_decorator/main.cc | 6 +- repos/gems/src/app/themed_decorator/window.h | 4 +- repos/os/include/decorator/window.h | 6 +- repos/os/include/decorator/window_stack.h | 58 +++++++++++++++++--- 7 files changed, 69 insertions(+), 16 deletions(-) diff --git a/repos/gems/src/app/decorator/main.cc b/repos/gems/src/app/decorator/main.cc index 9a70dcb58f..2fc231844c 100644 --- a/repos/gems/src/app/decorator/main.cc +++ b/repos/gems/src/app/decorator/main.cc @@ -269,7 +269,10 @@ void Decorator::Main::_handle_nitpicker_sync() try { Xml_node xml(_window_layout.local_addr(), _window_layout.size()); - _window_stack.update_model(xml); + + auto flush_window_stack_changes = [&] () { }; + + _window_stack.update_model(xml, flush_window_stack_changes); model_updated = true; diff --git a/repos/gems/src/app/decorator/window.cc b/repos/gems/src/app/decorator/window.cc index 44e4f8fb7c..b4709eabae 100644 --- a/repos/gems/src/app/decorator/window.cc +++ b/repos/gems/src/app/decorator/window.cc @@ -193,7 +193,7 @@ void Decorator::Window::draw(Decorator::Canvas_base &canvas, } -bool Decorator::Window::update(Genode::Xml_node window_node) +bool Decorator::Window::update(Genode::Xml_node window_node, bool new_top_most) { bool updated = false; @@ -202,7 +202,7 @@ bool Decorator::Window::update(Genode::Xml_node window_node) * view stack. */ unsigned const topped_cnt = attribute(window_node, "topped", 0UL); - if (topped_cnt != _topped_cnt) { + if (topped_cnt != _topped_cnt || new_top_most) { _topped_cnt = topped_cnt; diff --git a/repos/gems/src/app/decorator/window.h b/repos/gems/src/app/decorator/window.h index e7d2ce9393..d6902681e2 100644 --- a/repos/gems/src/app/decorator/window.h +++ b/repos/gems/src/app/decorator/window.h @@ -464,7 +464,7 @@ class Decorator::Window : public Window_base void draw(Canvas_base &canvas, Rect clip, Draw_behind_fn const &) const override; - bool update(Xml_node window_node) override; + bool update(Xml_node, bool) override; Hover hover(Point) const override; diff --git a/repos/gems/src/app/themed_decorator/main.cc b/repos/gems/src/app/themed_decorator/main.cc index 75f85caecc..9379ce097a 100644 --- a/repos/gems/src/app/themed_decorator/main.cc +++ b/repos/gems/src/app/themed_decorator/main.cc @@ -263,7 +263,11 @@ void Decorator::Main::_handle_nitpicker_sync() try { Xml_node xml(_window_layout.local_addr(), _window_layout.size()); - _window_stack.update_model(xml); + + auto flush_window_stack_changes = [&] () { + _window_stack.update_nitpicker_views(); }; + + _window_stack.update_model(xml, flush_window_stack_changes); model_updated = true; diff --git a/repos/gems/src/app/themed_decorator/window.h b/repos/gems/src/app/themed_decorator/window.h index 9632478dad..ffece1f39c 100644 --- a/repos/gems/src/app/themed_decorator/window.h +++ b/repos/gems/src/app/themed_decorator/window.h @@ -405,7 +405,7 @@ class Decorator::Window : public Window_base, public Animator::Item animate(); } - bool update(Xml_node window_node) override + bool update(Xml_node window_node, bool new_top_most) override { bool updated = false; @@ -414,7 +414,7 @@ class Decorator::Window : public Window_base, public Animator::Item * view stack. */ unsigned const topped_cnt = attribute(window_node, "topped", 0UL); - if (topped_cnt != _topped_cnt) { + if (topped_cnt != _topped_cnt || new_top_most) { _topped_cnt = topped_cnt; diff --git a/repos/os/include/decorator/window.h b/repos/os/include/decorator/window.h index 6bb78abdd3..2ff74f394d 100644 --- a/repos/os/include/decorator/window.h +++ b/repos/os/include/decorator/window.h @@ -125,6 +125,10 @@ class Decorator::Window_base : public Window_list::Element /** * Update internal window representation from XML model * + * \param new_top_most true if window became the new top-most + * window, which should prompt a corresponding + * nitpicker stacking operation. + * * \return true if window changed * * We do not immediately update the views as part of the update @@ -132,7 +136,7 @@ class Decorator::Window_base : public Window_list::Element * decorations haven't been redrawn already. If we updated the * nitpicker views at this point, we would reveal not-yet-drawn pixels. */ - virtual bool update(Xml_node window_node) = 0; + virtual bool update(Xml_node window_node, bool new_top_most) = 0; virtual void update_nitpicker_views() { } diff --git a/repos/os/include/decorator/window_stack.h b/repos/os/include/decorator/window_stack.h index 3941683f3a..b2cdd94679 100644 --- a/repos/os/include/decorator/window_stack.h +++ b/repos/os/include/decorator/window_stack.h @@ -35,6 +35,8 @@ class Decorator::Window_stack : public Window_base::Draw_behind_fn Window_factory_base &_window_factory; Dirty_rect mutable _dirty_rect; + unsigned long _top_most_id = ~0UL; + inline void _draw_rec(Canvas_base &canvas, Window_base const *win, Rect rect) const; @@ -44,7 +46,7 @@ class Decorator::Window_stack : public Window_base::Draw_behind_fn if (win->id() == id) return win; - return 0; + return nullptr; } static inline @@ -99,7 +101,8 @@ class Decorator::Window_stack : public Window_base::Draw_behind_fn return result; } - inline void update_model(Xml_node root_node); + template + inline void update_model(Xml_node root_node, FN const &flush); bool schedule_animated_windows() { @@ -205,22 +208,37 @@ void Decorator::Window_stack::_draw_rec(Decorator::Canvas_base &canvas, } -void Decorator::Window_stack::update_model(Genode::Xml_node root_node) +template +void Decorator::Window_stack::update_model(Genode::Xml_node root_node, + FN const &flush_window_stack_changes) { + Window_list _destroyed_windows { }; + + unsigned long new_top_most_id = ~0UL; + if (root_node.has_sub_node("window")) + new_top_most_id = root_node.sub_node("window").attribute_value("id", ~0UL); + /* * Step 1: Remove windows that are no longer present. */ - for (Window_base *window = _windows.first(), *next = 0; window; window = next) { + for (Window_base *window = _windows.first(), *next = nullptr; window; window = next) { next = window->next(); try { _xml_node_by_window_id(root_node, window->id()); } catch (Xml_node::Nonexistent_sub_node) { _dirty_rect.mark_as_dirty(window->outer_geometry()); - _destroy(*window); + _windows.remove(window); + _destroyed_windows.insert(window); }; } + /** + * Return true if window has came to front + */ + auto window_is_new_top_most = [&] (Window_base const &window) { + return (_top_most_id != new_top_most_id) && (window.id() == new_top_most_id); }; + /* * Step 2: Update window properties of already present windows. */ @@ -233,7 +251,8 @@ void Decorator::Window_stack::update_model(Genode::Xml_node root_node) */ try { Rect const orig_geometry = window->outer_geometry(); - if (window->update(_xml_node_by_window_id(root_node, window->id()))) { + if (window->update(_xml_node_by_window_id(root_node, window->id()), + window_is_new_top_most(*window))) { _dirty_rect.mark_as_dirty(orig_geometry); _dirty_rect.mark_as_dirty(window->outer_geometry()); } @@ -255,7 +274,7 @@ void Decorator::Window_stack::update_model(Genode::Xml_node root_node) if (new_window) { - new_window->update(window_node); + new_window->update(window_node, window_is_new_top_most(*new_window)); /* * Insert new window in front of all other windows. @@ -275,7 +294,7 @@ void Decorator::Window_stack::update_model(Genode::Xml_node root_node) /* * Step 4: Adjust window order. */ - Window_base *previous_window = 0; + Window_base *previous_window = nullptr; Window_base *window = _windows.first(); for_each_sub_node(root_node, "window", [&] (Xml_node window_node) { @@ -335,6 +354,29 @@ void Decorator::Window_stack::update_model(Genode::Xml_node root_node) w->stack(neighbor->frontmost_view()); } } + + /* + * Apply window-creation operations before destroying windows to prevent + * flickering. + */ + flush_window_stack_changes(); + + /* + * Destroy window objects. + * + * This is done after all other operations to avoid flickering whenever one + * window is replaced by another one. If we first destroyed the original + * one, the window background would appear for a brief moment until the new + * window is created. By deferring the destruction of the old window to the + * point when the new one already exists, one of both windows is visible at + * all times. + */ + for (Window_base *window = _destroyed_windows.first(), *next = nullptr; window; window = next) { + next = window->next(); + _destroy(*window); + } + + _top_most_id = new_top_most_id; } #endif /* _INCLUDE__DECORATOR__WINDOW_STACK_H_ */