Mir

Merge lp:~alan-griffiths/mir/fix-1506846 into lp:mir

  1. fix-1506846
  2. Merge into development-branch
Proposed by Alan Griffiths
Status: Superseded
Proposed branch: lp:~alan-griffiths/mir/fix-1506846
Merge into: lp:mir
Diff against target: 206 lines (+63/-13)
7 files modified
include/server/mir/shell/display_layout.h (+4/-2)
include/server/mir/shell/system_compositor_window_manager.h (+9/-0)
src/server/shell/CMakeLists.txt (+2/-0)
src/server/shell/graphics_display_layout.cpp (+2/-6)
src/server/shell/graphics_display_layout.h (+4/-4)
src/server/shell/system_compositor_window_manager.cpp (+41/-0)
tests/acceptance-tests/test_system_compositor_window_manager.cpp (+1/-1)
To merge this branch: bzr merge lp:~alan-griffiths/mir/fix-1506846
Reviewer Review Type Date Requested Status
Alexandros Frantzis Pending
PS Jenkins bot continuous-integration Pending
Review via email: mp+277091@code.launchpad.net

This proposal supersedes a proposal from 2015-11-09.

This proposal has been superseded by a proposal from 2015-11-10.

Commit message

shell: system compositors should apply nested display configurations without waiting for buffers to be submitted

Description of the change

shell: system compositors should apply nested display configurations without waiting for buffers to be submitted

To post a comment you must log in.
Revision history for this message
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal

I think this will break the phone startup by showing the unity8 session too early, while it is still blank. We could fix this by reintroducing similar logic as the one removed (i.e. a SurfaceReadyObserver) in USC's window manager.

review: Needs Information
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

> I think this will break the phone startup by showing the unity8 session too
> early, while it is still blank.

I see your point but...

> We could fix this by reintroducing similar
> logic as the one removed (i.e. a SurfaceReadyObserver) in USC's window
> manager.

...that would then break this fix.

Revision history for this message
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal

... and this also brings up a few questions: What does it mean for a session to be ready? Does it make sense for readiness to differ depending on the WM?

review: Needs Information
Revision history for this message
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal

> > We could fix this by reintroducing similar
> > logic as the one removed (i.e. a SurfaceReadyObserver) in USC's window
> > manager.
>
> ...that would then break this fix.

Ah, correct... The purpose of this fix is to focus the session early so its display config is applied.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

Alternatively, what does "fullscreen_on_output" mean for a session that has set a display configuration that changes that output?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:3093
http://jenkins.qa.ubuntu.com/job/mir-ci/5529/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4835
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3742
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4777
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/735/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1683/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/735
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/735/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4778
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4778/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7378
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25018
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/736/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5529/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'include/server/mir/shell/display_layout.h'
--- include/server/mir/shell/display_layout.h 2013-08-28 03:41:48 +0000
+++ include/server/mir/shell/display_layout.h 2015-11-10 09:17:23 +0000
@@ -53,12 +53,14 @@
53 virtual void size_to_output(geometry::Rectangle& rect) = 0;53 virtual void size_to_output(geometry::Rectangle& rect) = 0;
5454
55 /**55 /**
56 * Places a rectangle in an particular output.56 * Places a rectangle in an particular output if the display is known,
57 * otherwise does nothing.
57 *58 *
58 * @param [in] id the id of the output to place the rectangle in59 * @param [in] id the id of the output to place the rectangle in
59 * @param [in,out] rect the rectangle to place60 * @param [in,out] rect the rectangle to place
61 * @return true iff the display id is recognised
60 */62 */
61 virtual void place_in_output(graphics::DisplayConfigurationOutputId id,63 virtual bool place_in_output(graphics::DisplayConfigurationOutputId id,
62 geometry::Rectangle& rect) = 0;64 geometry::Rectangle& rect) = 0;
6365
64protected:66protected:
6567
=== modified file 'include/server/mir/shell/system_compositor_window_manager.h'
--- include/server/mir/shell/system_compositor_window_manager.h 2015-10-29 22:07:47 +0000
+++ include/server/mir/shell/system_compositor_window_manager.h 2015-11-10 09:17:23 +0000
@@ -20,6 +20,10 @@
20#define MIR_SHELL_SYSTEM_COMPOSITOR_WINDOW_MANAGER_H_20#define MIR_SHELL_SYSTEM_COMPOSITOR_WINDOW_MANAGER_H_
2121
22#include "mir/shell/window_manager.h"22#include "mir/shell/window_manager.h"
23#include "mir/graphics/display_configuration.h"
24
25#include <map>
26#include <mutex>
2327
24namespace mir28namespace mir
25{29{
@@ -96,6 +100,11 @@
96 std::shared_ptr<scene::Surface> const& surface,100 std::shared_ptr<scene::Surface> const& surface,
97 MirSurfaceAttrib attrib,101 MirSurfaceAttrib attrib,
98 int value) override;102 int value) override;
103
104 using OutputMap = std::map<std::weak_ptr<scene::Surface>, graphics::DisplayConfigurationOutputId, std::owner_less<std::weak_ptr<scene::Surface>>>;
105
106 std::mutex mutable mutex;
107 OutputMap output_map;
99};108};
100}109}
101}110}
102111
=== modified file 'src/server/shell/CMakeLists.txt'
--- src/server/shell/CMakeLists.txt 2015-10-29 02:08:24 +0000
+++ src/server/shell/CMakeLists.txt 2015-11-10 09:17:23 +0000
@@ -5,7 +5,9 @@
5 canonical_window_manager.cpp5 canonical_window_manager.cpp
6 frontend_shell.cpp6 frontend_shell.cpp
7 graphics_display_layout.cpp7 graphics_display_layout.cpp
8 graphics_display_layout.h
8 default_configuration.cpp9 default_configuration.cpp
10 null_host_lifecycle_event_listener.h
9 shell_wrapper.cpp11 shell_wrapper.cpp
10 surface_ready_observer.cpp12 surface_ready_observer.cpp
11 system_compositor_window_manager.cpp13 system_compositor_window_manager.cpp
1214
=== modified file 'src/server/shell/graphics_display_layout.cpp'
--- src/server/shell/graphics_display_layout.cpp 2015-04-28 07:54:10 +0000
+++ src/server/shell/graphics_display_layout.cpp 2015-11-10 09:17:23 +0000
@@ -24,9 +24,6 @@
24#include "mir/geometry/rectangles.h"24#include "mir/geometry/rectangles.h"
25#include "mir/geometry/displacement.h"25#include "mir/geometry/displacement.h"
2626
27#include <boost/throw_exception.hpp>
28#include <stdexcept>
29
30namespace msh = mir::shell;27namespace msh = mir::shell;
31namespace mg = mir::graphics;28namespace mg = mir::graphics;
32namespace geom = mir::geometry;29namespace geom = mir::geometry;
@@ -68,7 +65,7 @@
68 rect = output;65 rect = output;
69}66}
7067
71void msh::GraphicsDisplayLayout::place_in_output(68bool msh::GraphicsDisplayLayout::place_in_output(
72 graphics::DisplayConfigurationOutputId id,69 graphics::DisplayConfigurationOutputId id,
73 geometry::Rectangle& rect)70 geometry::Rectangle& rect)
74{71{
@@ -87,8 +84,7 @@
87 }84 }
88 });85 });
8986
90 if (!placed)87 return placed;
91 BOOST_THROW_EXCEPTION(std::runtime_error("Failed to place surface in requested output"));
92}88}
9389
94geom::Rectangle msh::GraphicsDisplayLayout::get_output_for(geometry::Rectangle& rect)90geom::Rectangle msh::GraphicsDisplayLayout::get_output_for(geometry::Rectangle& rect)
9591
=== modified file 'src/server/shell/graphics_display_layout.h'
--- src/server/shell/graphics_display_layout.h 2014-03-06 06:05:17 +0000
+++ src/server/shell/graphics_display_layout.h 2015-11-10 09:17:23 +0000
@@ -37,10 +37,10 @@
37public:37public:
38 GraphicsDisplayLayout(std::shared_ptr<graphics::Display> const& display);38 GraphicsDisplayLayout(std::shared_ptr<graphics::Display> const& display);
3939
40 void clip_to_output(geometry::Rectangle& rect);40 void clip_to_output(geometry::Rectangle& rect) override;
41 void size_to_output(geometry::Rectangle& rect);41 void size_to_output(geometry::Rectangle& rect) override;
42 void place_in_output(graphics::DisplayConfigurationOutputId output_id,42 bool place_in_output(graphics::DisplayConfigurationOutputId output_id,
43 geometry::Rectangle& rect);43 geometry::Rectangle& rect) override;
4444
45private:45private:
46 geometry::Rectangle get_output_for(geometry::Rectangle& rect);46 geometry::Rectangle get_output_for(geometry::Rectangle& rect);
4747
=== modified file 'src/server/shell/system_compositor_window_manager.cpp'
--- src/server/shell/system_compositor_window_manager.cpp 2015-10-29 22:07:47 +0000
+++ src/server/shell/system_compositor_window_manager.cpp 2015-11-10 09:17:23 +0000
@@ -28,6 +28,8 @@
28#include "mir/scene/surface.h"28#include "mir/scene/surface.h"
29#include "mir/scene/surface_creation_parameters.h"29#include "mir/scene/surface_creation_parameters.h"
3030
31#include <boost/throw_exception.hpp>
32
31namespace mf = mir::frontend;33namespace mf = mir::frontend;
32namespace ms = mir::scene;34namespace ms = mir::scene;
33namespace msh = mir::shell;35namespace msh = mir::shell;
@@ -60,6 +62,9 @@
60{62{
61 mir::geometry::Rectangle rect{params.top_left, params.size};63 mir::geometry::Rectangle rect{params.top_left, params.size};
6264
65 if (!params.output_id.as_value())
66 BOOST_THROW_EXCEPTION(std::runtime_error("An output ID must be specified"));
67
63 display_layout->place_in_output(params.output_id, rect);68 display_layout->place_in_output(params.output_id, rect);
6469
65 auto placed_parameters = params;70 auto placed_parameters = params;
@@ -78,6 +83,9 @@
78 surface);83 surface);
7984
80 surface->add_observer(session_ready_observer);85 surface->add_observer(session_ready_observer);
86
87 std::lock_guard<decltype(mutex)> lock{mutex};
88 output_map[surface] = params.output_id;
8189
82 return result;90 return result;
83}91}
@@ -89,6 +97,22 @@
89{97{
90 if (modifications.name.is_set())98 if (modifications.name.is_set())
91 surface->rename(modifications.name.value());99 surface->rename(modifications.name.value());
100
101 if (modifications.output_id.is_set())
102 {
103 auto const output_id = modifications.output_id.value();
104
105 mir::geometry::Rectangle rect{surface->top_left(), surface->size()};
106
107 if (display_layout->place_in_output(output_id, rect))
108 {
109 surface->move_to(rect.top_left);
110 surface->resize(rect.size);
111 }
112
113 std::lock_guard<decltype(mutex)> lock{mutex};
114 output_map[surface] = output_id;
115 }
92}116}
93117
94void msh::SystemCompositorWindowManager::remove_surface(118void msh::SystemCompositorWindowManager::remove_surface(
@@ -99,6 +123,23 @@
99123
100void msh::SystemCompositorWindowManager::add_display(mir::geometry::Rectangle const& /*area*/)124void msh::SystemCompositorWindowManager::add_display(mir::geometry::Rectangle const& /*area*/)
101{125{
126 std::lock_guard<decltype(mutex)> lock{mutex};
127
128 for (auto const& so : output_map)
129 {
130 if (auto surface = so.first.lock())
131 {
132 auto const output_id = so.second;
133
134 mir::geometry::Rectangle rect{surface->top_left(), surface->size()};
135
136 if (display_layout->place_in_output(output_id, rect))
137 {
138 surface->move_to(rect.top_left);
139 surface->resize(rect.size);
140 }
141 }
142 }
102}143}
103144
104void msh::SystemCompositorWindowManager::remove_display(mir::geometry::Rectangle const& /*area*/)145void msh::SystemCompositorWindowManager::remove_display(mir::geometry::Rectangle const& /*area*/)
105146
=== modified file 'tests/acceptance-tests/test_system_compositor_window_manager.cpp'
--- tests/acceptance-tests/test_system_compositor_window_manager.cpp 2015-10-02 04:27:35 +0000
+++ tests/acceptance-tests/test_system_compositor_window_manager.cpp 2015-11-10 09:17:23 +0000
@@ -167,7 +167,7 @@
167 auto surface = client.create_surface(0);167 auto surface = client.create_surface(0);
168168
169 EXPECT_FALSE(mir_surface_is_valid(surface));169 EXPECT_FALSE(mir_surface_is_valid(surface));
170 EXPECT_THAT(mir_surface_get_error_message(surface), HasSubstr("Failed to place surface"));170 EXPECT_THAT(mir_surface_get_error_message(surface), HasSubstr("An output ID must be specified"));
171}171}
172172
173TEST_F(SystemCompositorWindowManager, if_a_surface_posts_client_gets_focus)173TEST_F(SystemCompositorWindowManager, if_a_surface_posts_client_gets_focus)

Subscribers

People subscribed via source and target branches