Fixing a few broken things with the implementation and robustifying t…

…he code

include/platform/mir/graphics/initial_render.h

@@ -30,6 +30,7 @@ class Renderable;
 
 enum class SmoothSupportBehavior : uint32_t
 {
+    none,
     fade,
     count
 };

src/miral/smooth_boot_support.cpp

@@ -32,6 +32,8 @@ const char* to_option(mg::SmoothSupportBehavior behavior)
 {
     switch (behavior)
     {
+        case mg::SmoothSupportBehavior::none:
+            return "none";
         case mg::SmoothSupportBehavior::fade:
             return "fade";
         default:

src/server/graphics/default_initial_render_manager.cpp

@@ -29,24 +29,31 @@
 namespace mg = mir::graphics;
 namespace mo = mir::options;
 
+namespace
+{
+auto constexpr FADE_TIME = std::chrono::milliseconds{5000};
+
 mg::SmoothSupportBehavior from_option(std::string const& option)
 {
     if (option == "fade")
         return mg::SmoothSupportBehavior::fade;
+    else if (option == "none")
+        return mg::SmoothSupportBehavior::none;
     else
     {
         std::string error_message = "Invalid option string for smooth support behavior: " + option;
         BOOST_THROW_EXCEPTION(std::invalid_argument(error_message));
     }
 }
+}
 
 mg::DefaultInitialRenderManager::DefaultInitialRenderManager(
     std::shared_ptr<options::Option> const& options,
     std::shared_ptr<Executor> const& scene_executor,
     std::shared_ptr<time::Clock> const& clock,
     time::AlarmFactory& alarm_factory,
     std::shared_ptr<input::Scene>  const& scene)
-    : behavior{from_option(options->get<std::string>(mo::smooth_boot_opt))},
+    : behavior{SmoothSupportBehavior::none},
       scene_executor{scene_executor},
       clock{clock},
       scene{scene},
@@ -55,7 +62,10 @@ mg::DefaultInitialRenderManager::DefaultInitialRenderManager(
          alarm->cancel();
       })}
 {
-    time::Timestamp scheduled_time = clock->now() + std::chrono::microseconds{500};
+    if (options->is_set(mo::smooth_boot_opt))
+        behavior = from_option(options->get<std::string>(mo::smooth_boot_opt));
+
+    time::Timestamp scheduled_time = clock->now() + FADE_TIME;
     alarm->reschedule_for(scheduled_time);
 }
 
@@ -94,7 +104,7 @@ void mg::DefaultInitialRenderManager::remove_renderables()
 
     if (!scene)
     {
-        mir::log_error("Unable to remove the initial renerables because the scene does not exist");
+        mir::log_error("Unable to remove the initial renderables because the scene does not exist");
         return;
     }
 

src/server/graphics/multiplexing_display.cpp

@@ -19,6 +19,7 @@
 #include "mir/graphics/initial_render.h"
 #include "mir/renderer/gl/context.h"
 #include "mir/output_type_names.h"
+#include "mir/log.h"
 
 #include <boost/throw_exception.hpp>
 #include <stdexcept>
@@ -66,7 +67,7 @@ mg::MultiplexingDisplay::MultiplexingDisplay(
     auto conf = configuration();
     initial_configuration_policy.apply_to(*conf);
     configure(*conf);
-    initial_render_manager->add_initial_render(std::make_shared<MultiplexingInitialRender>(displays));
+    initial_render_manager->add_initial_render(std::make_shared<MultiplexingInitialRender>(this->displays));
 }
 
 mg::MultiplexingDisplay::~MultiplexingDisplay() = default;
@@ -352,5 +353,5 @@ auto mg::MultiplexingDisplay::create_hardware_cursor() -> std::shared_ptr<Cursor
 
 std::shared_ptr<mg::InitialRender> mg::MultiplexingDisplay::create_initial_render()
 {
-    return initial_render;
+    return std::make_shared<MultiplexingInitialRender>(displays);
 }

tests/include/mir/test/doubles/null_initial_render_manager.h

@@ -0,0 +1,39 @@
+/*
+ * Copyright © 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 2 or 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/>.
+ */
+
+#ifndef MIR_TEST_DOUBLES_NULL_INITIAL_RENDER_MANAGER_H
+#define MIR_TEST_DOUBLES_NULL_INITIAL_RENDER_MANAGER_H
+
+#include "mir/graphics/initial_render.h"
+
+namespace mir
+{
+    namespace  test
+    {
+        namespace doubles
+        {
+            class NullInitialRenderManager : public mir::graphics::InitialRenderManager
+            {
+            public:
+                void add_initial_render(std::shared_ptr<graphics::InitialRender> const&) override
+                {
+                }
+            };
+        }
+    }
+}
+
+#endif //MIR_TEST_DOUBLES_NULL_INITIAL_RENDER_MANAGER_H

tests/unit-tests/graphics/test_multiplexing_display.cpp

@@ -28,6 +28,7 @@
 #include "mir/test/doubles/null_gl_context.h"
 #include "mir/test/doubles/stub_display_configuration.h"
 #include "mir/test/doubles/null_display_configuration_policy.h"
+#include "mir/test/doubles/null_initial_render_manager.h"
 
 #include "mir_toolkit/common.h"
 #include "src/server/graphics/multiplexing_display.h"
@@ -139,7 +140,7 @@ TEST(MultiplexingDisplay, forwards_for_each_display_sync_group)
     }
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
 
     display.for_each_display_sync_group([](auto const&) {});
 }
@@ -182,7 +183,7 @@ TEST(MultiplexingDisplay, each_display_in_configuration_has_unique_id)
     }
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
 
     auto conf = display.configuration();
 
@@ -232,7 +233,7 @@ TEST(MultiplexingDisplay, configuration_is_union_of_all_displays)
     }
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
 
     auto conf = display.configuration();
 
@@ -317,7 +318,7 @@ TEST(MultiplexingDisplay, dispatches_configure_to_associated_platform)
     displays.push_back(std::move(d2));
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
     auto conf = display.configuration();
 
     display.configure(*conf);
@@ -375,7 +376,7 @@ TEST(MultiplexingDisplay, apply_if_confguration_preserves_display_buffers_succee
     displays.push_back(std::move(d2));
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
     auto conf = display.configuration();
 
     display.apply_if_configuration_preserves_display_buffers(*conf);
@@ -422,7 +423,7 @@ TEST(MultiplexingDisplay, apply_if_confguration_preserves_display_buffers_fails_
     displays.push_back(std::move(d2));
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
     auto conf = display.configuration();
 
     EXPECT_THAT(display.apply_if_configuration_preserves_display_buffers(*conf), Eq(false));
@@ -482,7 +483,7 @@ TEST(MultiplexingDisplay, apply_if_configuration_preserves_display_buffers_fails
     displays.push_back(std::move(d2));
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
     auto preexisting_conf = display.configuration();
     auto changed_conf = display.configuration();
 
@@ -555,7 +556,7 @@ TEST(MultiplexingDisplay, apply_if_configuration_preserves_display_buffers_throw
     displays.push_back(std::move(d2));
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
     auto preexisting_conf = display.configuration();
     auto changed_conf = display.configuration();
 
@@ -582,7 +583,7 @@ TEST(MultiplexingDisplay, delegates_registering_configuration_change_handlers)
     }
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(mock_displays), policy};
+    mg::MultiplexingDisplay display{std::move(mock_displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
 
     mtd::MockMainLoop ml;
     display.register_configuration_change_handler(ml, [](){});
@@ -602,7 +603,7 @@ TEST(MultiplexingDisplay, delegates_pause_resume_to_underlying_platforms)
     }
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(mock_displays), policy};
+    mg::MultiplexingDisplay display{std::move(mock_displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
 
     display.pause();
     display.resume();
@@ -707,7 +708,7 @@ TEST(MultiplexingDisplay, applies_initial_display_configuration)
     displays.push_back(std::move(d1));
     displays.push_back(std::move(d2));
 
-    mg::MultiplexingDisplay display{std::move(displays), *policy};
+    mg::MultiplexingDisplay display{std::move(displays), *policy, std::make_shared<mtd::NullInitialRenderManager>()};
 
     display.configuration()->for_each_output(
         [policy](mg::DisplayConfigurationOutput const& output)
@@ -753,7 +754,7 @@ TEST(MultiplexingDisplay, sets_output_names_to_unique_values)
     }
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
 
     auto conf = display.configuration();
 
@@ -805,7 +806,7 @@ TEST(MultiplexingDisplay, output_names_begin_with_connector_type)
     }
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
 
     auto conf = display.configuration();
 
@@ -848,7 +849,7 @@ TEST(MultiplexingDisplay, output_names_are_bucketed_by_type)
     }
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
 
     auto conf = display.configuration();
 
@@ -890,7 +891,7 @@ TEST(MultiplexingDisplay, output_name_does_not_contain_card_id_when_only_one_car
     }
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
 
     auto conf = display.configuration();
 
@@ -947,7 +948,7 @@ TEST(MultiplexingDisplay, when_additional_cards_exist_their_outputs_are_numbered
     }
 
     mtd::NullDisplayConfigurationPolicy policy;
-    mg::MultiplexingDisplay display{std::move(displays), policy};
+    mg::MultiplexingDisplay display{std::move(displays), policy, std::make_shared<mtd::NullInitialRenderManager>()};
 
     auto conf = display.configuration();