Mir

Merge lp:~afrantzis/mir/fix-1344024 into lp:mir

  1. fix-1344024
  2. Merge into development-branch
Proposed by Alexandros Frantzis
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 1805
Proposed branch: lp:~afrantzis/mir/fix-1344024
Merge into: lp:mir
Diff against target: 346 lines (+92/-63)
11 files modified
src/server/shell/CMakeLists.txt (+1/-1)
src/server/shell/default_configuration.cpp (+2/-2)
src/server/shell/default_placement_strategy.cpp (+3/-12)
src/server/shell/default_placement_strategy.h (+8/-8)
tests/acceptance-tests/CMakeLists.txt (+1/-0)
tests/acceptance-tests/test_focus_selection.cpp (+0/-1)
tests/acceptance-tests/test_server_without_active_outputs.cpp (+64/-0)
tests/integration-tests/session_management.cpp (+1/-4)
tests/mir_test_framework/stubbed_server_configuration.cpp (+7/-0)
tests/unit-tests/shell/CMakeLists.txt (+1/-1)
tests/unit-tests/shell/test_default_placement_strategy.cpp (+4/-34)
To merge this branch: bzr merge lp:~afrantzis/mir/fix-1344024
Reviewer Review Type Date Requested Status
Alan Griffiths Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+228484@code.launchpad.net

Commit message

server: Allow clients to create surfaces even when there are no active outputs (LP: #1344024)

This is fixed by removing obsolete placement rules from our default placement
policy. These rules had the side-effect of prohibiting surface creation when
there were no active outputs.

Description of the change

server: Allow clients to create surfaces even when there are no active outputs

This is fixed by removing obsolete placement rules from our default placement
policy. These rules had the side-effect of prohibiting surface creation when
there were no active outputs.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1799
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2248/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1118
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1103
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/770
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/770/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/771
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/771/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/38
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/38/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2225
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10531

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2248/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1800
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2252/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1116
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1122
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1107
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/774
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/774/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/775
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/775/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/42
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/42/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2230
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10548

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2252/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Maybe the default could be better, but this will do for now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/server/shell/CMakeLists.txt'
--- src/server/shell/CMakeLists.txt 2014-05-26 22:00:52 +0000
+++ src/server/shell/CMakeLists.txt 2014-07-28 13:50:21 +0000
@@ -2,7 +2,7 @@
2 SHELL_SOURCES2 SHELL_SOURCES
33
4 default_focus_mechanism.cpp4 default_focus_mechanism.cpp
5 consuming_placement_strategy.cpp5 default_placement_strategy.cpp
6 graphics_display_layout.cpp6 graphics_display_layout.cpp
7 default_configuration.cpp7 default_configuration.cpp
8 session_coordinator_wrapper.cpp8 session_coordinator_wrapper.cpp
99
=== modified file 'src/server/shell/default_configuration.cpp'
--- src/server/shell/default_configuration.cpp 2014-07-21 03:35:31 +0000
+++ src/server/shell/default_configuration.cpp 2014-07-28 13:50:21 +0000
@@ -20,7 +20,7 @@
20#include "null_host_lifecycle_event_listener.h"20#include "null_host_lifecycle_event_listener.h"
2121
2222
23#include "consuming_placement_strategy.h"23#include "default_placement_strategy.h"
24#include "default_focus_mechanism.h"24#include "default_focus_mechanism.h"
25#include "graphics_display_layout.h"25#include "graphics_display_layout.h"
2626
@@ -34,7 +34,7 @@
34 return shell_placement_strategy(34 return shell_placement_strategy(
35 [this]35 [this]
36 {36 {
37 return std::make_shared<msh::ConsumingPlacementStrategy>(37 return std::make_shared<msh::DefaultPlacementStrategy>(
38 the_shell_display_layout());38 the_shell_display_layout());
39 });39 });
40}40}
4141
=== renamed file 'src/server/shell/consuming_placement_strategy.cpp' => 'src/server/shell/default_placement_strategy.cpp'
--- src/server/shell/consuming_placement_strategy.cpp 2014-04-15 05:31:19 +0000
+++ src/server/shell/default_placement_strategy.cpp 2014-07-28 13:50:21 +0000
@@ -16,7 +16,7 @@
16 * Authored by: Robert Carr <robert.carr@canonical.com>16 * Authored by: Robert Carr <robert.carr@canonical.com>
17 */17 */
1818
19#include "consuming_placement_strategy.h"19#include "default_placement_strategy.h"
20#include "mir/scene/surface_creation_parameters.h"20#include "mir/scene/surface_creation_parameters.h"
21#include "mir/shell/display_layout.h"21#include "mir/shell/display_layout.h"
22#include "mir/geometry/rectangle.h"22#include "mir/geometry/rectangle.h"
@@ -29,13 +29,13 @@
29namespace msh = mir::shell;29namespace msh = mir::shell;
30namespace geom = mir::geometry;30namespace geom = mir::geometry;
3131
32msh::ConsumingPlacementStrategy::ConsumingPlacementStrategy(32msh::DefaultPlacementStrategy::DefaultPlacementStrategy(
33 std::shared_ptr<msh::DisplayLayout> const& display_layout)33 std::shared_ptr<msh::DisplayLayout> const& display_layout)
34 : display_layout(display_layout)34 : display_layout(display_layout)
35{35{
36}36}
3737
38ms::SurfaceCreationParameters msh::ConsumingPlacementStrategy::place(38ms::SurfaceCreationParameters msh::DefaultPlacementStrategy::place(
39 ms::Session const& /* session */,39 ms::Session const& /* session */,
40 ms::SurfaceCreationParameters const& request_parameters)40 ms::SurfaceCreationParameters const& request_parameters)
41{41{
@@ -49,15 +49,6 @@
49 {49 {
50 display_layout->place_in_output(request_parameters.output_id, rect);50 display_layout->place_in_output(request_parameters.output_id, rect);
51 }51 }
52 else if (request_parameters.size.width > geom::Width{0} &&
53 request_parameters.size.height > geom::Height{0})
54 {
55 display_layout->clip_to_output(rect);
56 }
57 else
58 {
59 display_layout->size_to_output(rect);
60 }
6152
62 placed_parameters.top_left = rect.top_left;53 placed_parameters.top_left = rect.top_left;
63 placed_parameters.size = rect.size;54 placed_parameters.size = rect.size;
6455
=== renamed file 'src/server/shell/consuming_placement_strategy.h' => 'src/server/shell/default_placement_strategy.h'
--- src/server/shell/consuming_placement_strategy.h 2014-04-15 05:31:19 +0000
+++ src/server/shell/default_placement_strategy.h 2014-07-28 13:50:21 +0000
@@ -16,8 +16,8 @@
16 * Authored by: Robert Carr <robert.carr@canonical.com>16 * Authored by: Robert Carr <robert.carr@canonical.com>
17 */17 */
1818
19#ifndef MIR_SHELL_CONSUMING_PLACEMENT_STRATEGY_H_19#ifndef MIR_SHELL_DEFAULT_PLACEMENT_STRATEGY_H_
20#define MIR_SHELL_CONSUMING_PLACEMENT_STRATEGY_H_20#define MIR_SHELL_DEFAULT_PLACEMENT_STRATEGY_H_
2121
22#include "mir/scene/placement_strategy.h"22#include "mir/scene/placement_strategy.h"
2323
@@ -29,18 +29,18 @@
29{29{
30class DisplayLayout;30class DisplayLayout;
3131
32class ConsumingPlacementStrategy : public scene::PlacementStrategy32class DefaultPlacementStrategy : public scene::PlacementStrategy
33{33{
34public:34public:
35 explicit ConsumingPlacementStrategy(35 explicit DefaultPlacementStrategy(
36 std::shared_ptr<DisplayLayout> const& display_layout);36 std::shared_ptr<DisplayLayout> const& display_layout);
37 virtual ~ConsumingPlacementStrategy() {}37 virtual ~DefaultPlacementStrategy() {}
3838
39 virtual scene::SurfaceCreationParameters place(scene::Session const& session, scene::SurfaceCreationParameters const& request_parameters);39 virtual scene::SurfaceCreationParameters place(scene::Session const& session, scene::SurfaceCreationParameters const& request_parameters);
4040
41protected:41protected:
42 ConsumingPlacementStrategy(ConsumingPlacementStrategy const&) = delete;42 DefaultPlacementStrategy(DefaultPlacementStrategy const&) = delete;
43 ConsumingPlacementStrategy& operator=(ConsumingPlacementStrategy const&) = delete;43 DefaultPlacementStrategy& operator=(DefaultPlacementStrategy const&) = delete;
4444
45private:45private:
46 std::shared_ptr<DisplayLayout> const display_layout;46 std::shared_ptr<DisplayLayout> const display_layout;
@@ -49,4 +49,4 @@
49}49}
50} // namespace mir50} // namespace mir
5151
52#endif // MIR_SHELL_CONSUMING_PLACEMENT_STRATEGY_H_52#endif // MIR_SHELL_DEFAULT_PLACEMENT_STRATEGY_H_
5353
=== modified file 'tests/acceptance-tests/CMakeLists.txt'
--- tests/acceptance-tests/CMakeLists.txt 2014-07-23 17:00:22 +0000
+++ tests/acceptance-tests/CMakeLists.txt 2014-07-28 13:50:21 +0000
@@ -41,6 +41,7 @@
41 test_client_with_custom_display_config_deadlock.cpp41 test_client_with_custom_display_config_deadlock.cpp
42 test_macros.cpp42 test_macros.cpp
43 test_stale_frames.cpp43 test_stale_frames.cpp
44 test_server_without_active_outputs.cpp
44 ${GENERATED_PROTOBUF_SRCS}45 ${GENERATED_PROTOBUF_SRCS}
45 ${GENERATED_PROTOBUF_HDRS}46 ${GENERATED_PROTOBUF_HDRS}
46)47)
4748
=== modified file 'tests/acceptance-tests/test_focus_selection.cpp'
--- tests/acceptance-tests/test_focus_selection.cpp 2014-06-03 11:04:15 +0000
+++ tests/acceptance-tests/test_focus_selection.cpp 2014-07-28 13:50:21 +0000
@@ -19,7 +19,6 @@
19#include "clients.h"19#include "clients.h"
2020
21#include "src/server/scene/session_container.h"21#include "src/server/scene/session_container.h"
22#include "src/server/shell/consuming_placement_strategy.h"
23#include "src/server/scene/session_manager.h"22#include "src/server/scene/session_manager.h"
24#include "mir/graphics/display.h"23#include "mir/graphics/display.h"
25#include "mir/shell/input_targeter.h"24#include "mir/shell/input_targeter.h"
2625
=== added file 'tests/acceptance-tests/test_server_without_active_outputs.cpp'
--- tests/acceptance-tests/test_server_without_active_outputs.cpp 1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/test_server_without_active_outputs.cpp 2014-07-28 13:50:21 +0000
@@ -0,0 +1,64 @@
1/*
2 * Copyright © 2014 Canonical Ltd.
3 *
4 * This program is free software: you can redistribute it and/or modify it
5 * under the terms of the GNU General Public License version 3,
6 * as published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 * Authored by: Alexandros Frantzis <alexandros.frantzis@canonical.com>
17 */
18
19#include "mir_toolkit/mir_client_library.h"
20
21#include "mir_test_framework/stubbed_server_configuration.h"
22#include "mir_test_framework/basic_client_server_fixture.h"
23
24#include <gtest/gtest.h>
25#include <gmock/gmock.h>
26
27namespace mtf = mir_test_framework;
28namespace geom = mir::geometry;
29
30namespace
31{
32
33struct NoOutputsServerConfig : mtf::StubbedServerConfiguration
34{
35 NoOutputsServerConfig()
36 : mtf::StubbedServerConfiguration(std::vector<geom::Rectangle>{})
37 {
38 }
39};
40
41using ServerWithoutActiveOutputs = mtf::BasicClientServerFixture<NoOutputsServerConfig>;
42
43}
44
45TEST_F(ServerWithoutActiveOutputs, creates_valid_client_surface)
46{
47 using namespace testing;
48
49 MirSurfaceParameters const request_params =
50 {
51 __PRETTY_FUNCTION__,
52 640, 480,
53 mir_pixel_format_abgr_8888,
54 mir_buffer_usage_hardware,
55 mir_display_output_id_invalid
56 };
57
58 auto const surface = mir_connection_create_surface_sync(connection, &request_params);
59
60 EXPECT_THAT(mir_surface_is_valid(surface), Eq(mir_true))
61 << mir_surface_get_error_message(surface);
62
63 mir_surface_release_sync(surface);
64}
065
=== modified file 'tests/integration-tests/session_management.cpp'
--- tests/integration-tests/session_management.cpp 2014-06-03 11:04:15 +0000
+++ tests/integration-tests/session_management.cpp 2014-07-28 13:50:21 +0000
@@ -98,6 +98,7 @@
98 std::shared_ptr<mf::EventSink> const event_sink = std::make_shared<mtd::NullEventSink>();98 std::shared_ptr<mf::EventSink> const event_sink = std::make_shared<mtd::NullEventSink>();
99 std::shared_ptr<mf::Shell> const session_manager = builder.the_frontend_shell();99 std::shared_ptr<mf::Shell> const session_manager = builder.the_frontend_shell();
100 std::shared_ptr<TestSurfaceStack> const& test_surface_stack = builder.test_surface_stack;100 std::shared_ptr<TestSurfaceStack> const& test_surface_stack = builder.test_surface_stack;
101 ms::SurfaceCreationParameters const params = ms::SurfaceCreationParameters().of_size(100,100);
101102
102 void SetUp()103 void SetUp()
103 {104 {
@@ -117,8 +118,6 @@
117118
118TEST_F(SessionManagement, creating_a_surface_adds_it_to_scene)119TEST_F(SessionManagement, creating_a_surface_adds_it_to_scene)
119{120{
120 ms::SurfaceCreationParameters params;
121
122 auto const session = session_manager->open_session(0, __PRETTY_FUNCTION__, event_sink);121 auto const session = session_manager->open_session(0, __PRETTY_FUNCTION__, event_sink);
123122
124 EXPECT_CALL(*test_surface_stack, add_surface(_,_,_)).Times(1);123 EXPECT_CALL(*test_surface_stack, add_surface(_,_,_)).Times(1);
@@ -129,8 +128,6 @@
129{128{
130 EXPECT_CALL(*test_surface_stack, add_surface(_,_,_)).Times(AnyNumber());129 EXPECT_CALL(*test_surface_stack, add_surface(_,_,_)).Times(AnyNumber());
131130
132 ms::SurfaceCreationParameters params;
133
134 auto const session1 = session_manager->open_session(0, __PRETTY_FUNCTION__, event_sink);131 auto const session1 = session_manager->open_session(0, __PRETTY_FUNCTION__, event_sink);
135 auto const surface1 = session1->create_surface(params);132 auto const surface1 = session1->create_surface(params);
136133
137134
=== modified file 'tests/mir_test_framework/stubbed_server_configuration.cpp'
--- tests/mir_test_framework/stubbed_server_configuration.cpp 2014-06-23 22:25:17 +0000
+++ tests/mir_test_framework/stubbed_server_configuration.cpp 2014-07-28 13:50:21 +0000
@@ -116,6 +116,13 @@
116 public:116 public:
117 std::shared_ptr<mg::Buffer> alloc_buffer(mg::BufferProperties const& properties) override117 std::shared_ptr<mg::Buffer> alloc_buffer(mg::BufferProperties const& properties) override
118 {118 {
119 if (properties.size.width == geom::Width{0} ||
120 properties.size.height == geom::Height{0})
121 {
122 BOOST_THROW_EXCEPTION(
123 std::runtime_error("Request for allocation of buffer with invalid size"));
124 }
125
119 return std::make_shared<StubFDBuffer>(properties);126 return std::make_shared<StubFDBuffer>(properties);
120 }127 }
121};128};
122129
=== modified file 'tests/unit-tests/shell/CMakeLists.txt'
--- tests/unit-tests/shell/CMakeLists.txt 2014-04-15 05:31:19 +0000
+++ tests/unit-tests/shell/CMakeLists.txt 2014-07-28 13:50:21 +0000
@@ -1,5 +1,5 @@
1list(APPEND UNIT_TEST_SOURCES1list(APPEND UNIT_TEST_SOURCES
2 ${CMAKE_CURRENT_SOURCE_DIR}/test_consuming_placement_strategy.cpp2 ${CMAKE_CURRENT_SOURCE_DIR}/test_default_placement_strategy.cpp
3 ${CMAKE_CURRENT_SOURCE_DIR}/test_graphics_display_layout.cpp3 ${CMAKE_CURRENT_SOURCE_DIR}/test_graphics_display_layout.cpp
4)4)
55
66
=== renamed file 'tests/unit-tests/shell/test_consuming_placement_strategy.cpp' => 'tests/unit-tests/shell/test_default_placement_strategy.cpp'
--- tests/unit-tests/shell/test_consuming_placement_strategy.cpp 2014-04-15 05:31:19 +0000
+++ tests/unit-tests/shell/test_default_placement_strategy.cpp 2014-07-28 13:50:21 +0000
@@ -19,7 +19,7 @@
19#include "mir_test_doubles/mock_display_layout.h"19#include "mir_test_doubles/mock_display_layout.h"
20#include "mir_test_doubles/stub_scene_session.h"20#include "mir_test_doubles/stub_scene_session.h"
2121
22#include "src/server/shell/consuming_placement_strategy.h"22#include "src/server/shell/default_placement_strategy.h"
23#include "mir/scene/surface_creation_parameters.h"23#include "mir/scene/surface_creation_parameters.h"
2424
25#include "mir/geometry/rectangle.h"25#include "mir/geometry/rectangle.h"
@@ -35,7 +35,7 @@
35namespace35namespace
36{36{
3737
38struct ConsumingPlacementStrategySetup : public testing::Test38struct DefaultPlacementStrategySetup : public testing::Test
39{39{
40 void SetUp()40 void SetUp()
41 {41 {
@@ -48,37 +48,7 @@
48};48};
49}49}
5050
5151TEST_F(DefaultPlacementStrategySetup, parameters_with_output_id_are_placed_in_output)
52TEST_F(ConsumingPlacementStrategySetup, parameters_with_no_geometry_are_made_fullscreen)
53{
54 using namespace ::testing;
55
56 ms::SurfaceCreationParameters input_params;
57 geom::Rectangle rect{input_params.top_left, input_params.size};
58
59 EXPECT_CALL(*display_layout, size_to_output(rect)).Times(1);
60
61 msh::ConsumingPlacementStrategy placement_strategy(display_layout);
62
63 placement_strategy.place(session, input_params);
64}
65
66TEST_F(ConsumingPlacementStrategySetup, parameters_with_geometry_are_clipped)
67{
68 using namespace ::testing;
69
70 ms::SurfaceCreationParameters input_params;
71 input_params.size = geom::Size{100, 200};
72 geom::Rectangle rect{input_params.top_left, input_params.size};
73
74 EXPECT_CALL(*display_layout, clip_to_output(rect)).Times(1);
75
76 msh::ConsumingPlacementStrategy placement_strategy(display_layout);
77
78 placement_strategy.place(session, input_params);
79}
80
81TEST_F(ConsumingPlacementStrategySetup, parameters_with_output_id_are_placed_in_output)
82{52{
83 using namespace ::testing;53 using namespace ::testing;
8454
@@ -91,7 +61,7 @@
91 place_in_output(input_params.output_id, rect))61 place_in_output(input_params.output_id, rect))
92 .Times(1);62 .Times(1);
9363
94 msh::ConsumingPlacementStrategy placement_strategy(display_layout);64 msh::DefaultPlacementStrategy placement_strategy(display_layout);
9565
96 placement_strategy.place(session, input_params);66 placement_strategy.place(session, input_params);
97}67}

Subscribers

People subscribed via source and target branches