handling screen display change corner cases

openlp/core/display/screens.py

@@ -163,7 +163,7 @@ class Screen(object):
         Callback function for when the screens geometry changes
         """
         self.geometry = geometry
-        Registry().execute('config_screen_changed')
+        emit_config_screen_changed()
 
 
 class ScreenList(metaclass=Singleton):
@@ -396,7 +396,7 @@ class ScreenList(metaclass=Singleton):
                                    is_primary=self.application.primaryScreen() == changed_screen))
         self.find_new_display_screen()
         changed_screen.geometryChanged.connect(self.screens[-1].on_geometry_changed)
-        Registry().execute('config_screen_changed')
+        emit_config_screen_changed()
 
     def on_screen_removed(self, removed_screen):
         """
@@ -417,7 +417,7 @@ class ScreenList(metaclass=Singleton):
         self.screens.pop(removed_screen_number)
         if removed_screen_is_display:
             self.find_new_display_screen()
-        Registry().execute('config_screen_changed')
+        emit_config_screen_changed()
 
     def on_primary_screen_changed(self):
         """
@@ -426,5 +426,21 @@ class ScreenList(metaclass=Singleton):
         for screen in self.screens:
             screen.is_primary = self.application.primaryScreen().geometry() == screen.geometry
         self.find_new_display_screen()
+        emit_config_screen_changed()
 
-        Registry().execute('config_screen_changed')
+
+SCREEN_CHANGED_DEBOUNCE_TIMEOUT = 350
+
+
+def emit_config_screen_changed():
+    screen_changed_debounce.start()
+
+
+def __do_emit_config_screen_changed():
+    Registry().execute('config_screen_changed')
+
+
+screen_changed_debounce = QtCore.QTimer(None)
+screen_changed_debounce.setInterval(SCREEN_CHANGED_DEBOUNCE_TIMEOUT)
+screen_changed_debounce.setSingleShot(True)
+screen_changed_debounce.timeout.connect(__do_emit_config_screen_changed)

openlp/core/display/window.py

@@ -130,11 +130,13 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties, LogMixin):
     """
     This is a window to show the output
     """
-    def __init__(self, parent=None, screen=None, can_show_startup_screen=True):
+    def __init__(self, parent=None, screen=None, can_show_startup_screen=True, start_hidden=False,
+                 after_loaded_callback=None):
         """
         Create the display window
         """
         super(DisplayWindow, self).__init__(parent)
+        self.after_loaded_callback = after_loaded_callback
         # Gather all flags for the display window
         flags = QtCore.Qt.FramelessWindowHint | QtCore.Qt.Tool | QtCore.Qt.WindowStaysOnTopHint
         if self.settings.value('advanced/x11 bypass wm'):
@@ -183,7 +185,7 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties, LogMixin):
             self.update_from_screen(screen)
             self.is_display = True
             # Only make visible on single monitor setup if setting enabled.
-            if len(ScreenList()) > 1 or self.settings.value('core/display on monitor'):
+            if not start_hidden and (len(ScreenList()) > 1 or self.settings.value('core/display on monitor')):
                 self.show()
 
     def closeEvent(self, event):
@@ -303,6 +305,8 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties, LogMixin):
             self.set_scale(self.scale)
         if self._can_show_startup_screen:
             self.set_startup_screen()
+        if self.after_loaded_callback:
+            self.after_loaded_callback()
 
     def run_javascript(self, script, is_sync=False):
         """

openlp/core/ui/mainwindow.py

@@ -25,6 +25,7 @@ import shutil
 from datetime import datetime, date
 from pathlib import Path
 from tempfile import gettempdir
+from threading import Lock
 
 from PyQt5 import QtCore, QtGui, QtWidgets
 
@@ -526,6 +527,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert
         # Starting up web services
         self.http_server = HttpServer(self)
         self.ws_server = WebSocketServer()
+        self.screen_updating_lock = Lock()
 
     def _wait_for_threads(self):
         """
@@ -1015,25 +1017,33 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert
         """
         The screen has changed so we have to update components such as the renderer.
         """
-        self.application.set_busy_cursor()
+        try:
-        self.renderer.resize(self.live_controller.screens.current.display_geometry.size())
+            self.screen_updating_lock.acquire()
-        self.preview_controller.screen_size_changed()
+            # if a warning has been shown within the last 5 seconds, skip showing again to avoid spamming user,
-        self.live_controller.setup_displays()
+            # also do not show if the settings window is visible
-        self.live_controller.screen_size_changed()
+            if not self.settings_form.isVisible() and not self.screen_change_timestamp or \
-        self.setFocus()
+                    self.screen_change_timestamp and (datetime.now() - self.screen_change_timestamp).seconds > 5:
-        self.activateWindow()
+                self.screen_change_timestamp = datetime.now()
-        self.application.set_normal_cursor()
+                QtWidgets.QMessageBox.warning(self, translate('OpenLP.MainWindow', 'Screen setup has changed'),
-        # if a warning has been shown within the last 5 seconds, skip showing again to avoid spamming user,
+                                              translate('OpenLP.MainWindow',
-        # also do not show if the settings window is visible
+                                                        'The screen setup has changed. '
-        if not self.settings_form.isVisible() and not self.screen_change_timestamp or \
+                                                        'OpenLP will try to automatically select a display screen, but '
-                self.screen_change_timestamp and (datetime.now() - self.screen_change_timestamp).seconds > 5:
+                                                        'you should consider updating the screen settings.'),
-            self.screen_change_timestamp = datetime.now()
+                                              QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok))
-            QtWidgets.QMessageBox.warning(self, translate('OpenLP.MainWindow', 'Screen setup has changed'),
+            self.application.set_busy_cursor()
-                                          translate('OpenLP.MainWindow',
+            self.renderer.resize(self.live_controller.screens.current.display_geometry.size())
-                                                    'The screen setup has changed. '
+            self.preview_controller.screen_size_changed()
-                                                    'OpenLP will try to automatically select a display screen, but '
+            self.live_controller.setup_displays()
-                                                    'you should consider updating the screen settings.'),
+            self.live_controller.screen_size_changed()
-                                          QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok))
+            self.setFocus()
+            self.activateWindow()
+            self.application.set_normal_cursor()
+            # Forcing application to process events to trigger display update
+            # We need to wait a little of time as it would otherwise need a mouse move
+            # to process the screen change, for example
+            QtCore.QTimer.singleShot(150, lambda: self.application.process_events())
+        finally:
+            self.screen_updating_lock.release()
 
     def closeEvent(self, event):
         """

openlp/core/ui/slidecontroller.py

@@ -181,12 +181,23 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
         self.close_displays()
         for screen in self.screens:
             if screen.is_display:
-                display = DisplayWindow(self, screen)
+                will_start_hidden = self._current_hide_mode == HideMode.Screen
+                display = DisplayWindow(self, screen, start_hidden=will_start_hidden,
+                                        after_loaded_callback=self._display_after_loaded_callback)
                 self.displays.append(display)
                 self._reset_blank(False)
         if self.display:
             self.__add_actions_to_widget(self.display)
 
+    def _display_after_loaded_callback(self):
+        # As the display was reloaded, we'll need to process current item again
+        if self.service_item:
+            self._process_item(self.service_item, self.selected_row, is_reloading=True)
+        if self._current_hide_mode == HideMode.Screen:
+            # Forcing screen to be on transparent mode if already hidden, otherwise the 'show' animation would not
+            # be performed.
+            self.display.hide_display(HideMode.Screen)
+
     def close_displays(self):
         """
         Close all open displays
@@ -929,12 +940,13 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
         for display in self.displays:
             display.set_theme(theme_data, service_item_type=service_item.service_item_type)
 
-    def _process_item(self, service_item, slide_no):
+    def _process_item(self, service_item, slide_no, is_reloading=False):
         """
         Loads a ServiceItem into the system from ServiceManager. Display the slide number passed.
 
         :param service_item: The current service item
         :param slide_no: The slide number to select
+        :param is_reloading: If the controller is reloading the current item, due to a display update (for example).
         """
         self.log_debug('_process_item start')
         self.on_stop_loop()
@@ -1044,7 +1056,7 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
                 self.application.process_events()
             self.ignore_toolbar_resize_events = False
             self.on_controller_size_changed()
-            if self.settings.value('core/auto unblank'):
+            if not is_reloading and self.settings.value('core/auto unblank'):
                 self.set_hide_mode(None)
         self.log_debug('_process_item end')
 

tests/openlp_core/display/test_window.py

@@ -29,6 +29,7 @@ from pathlib import Path
 from unittest.mock import MagicMock, patch
 
 from PyQt5 import QtCore
+from openlp.core.display.screens import Screen
 
 # Mock QtWebEngineWidgets
 sys.modules['PyQt5.QtWebEngineWidgets'] = MagicMock()
@@ -123,6 +124,48 @@ def test_not_macos_toolwindow_attribute_set(mocked_is_macosx, mock_settings, dis
     assert display_window.testAttribute(QtCore.Qt.WA_MacAlwaysShowToolWindow) is False
 
 
+@patch.object(DisplayWindow, 'show')
+def test_not_shown_if_start_hidden_is_set(mocked_show, display_window_env, mock_settings):
+    """
+    Tests if DisplayWindow's .show() method is not called on constructor if constructed with start_hidden=True
+    """
+
+    # GIVEN: A mocked DisplayWindow's show method, a fake screen and relevant settings
+    settings = {
+        'advanced/x11 bypass wm': False,
+        'core/display on monitor': True
+    }
+    mock_settings.value.side_effect = lambda key: settings[key]
+    screen = Screen(1, QtCore.QRect(0, 0, 800, 600), is_display=True)
+
+    # WHEN: A DisplayWindow is created with start_hidden=True
+    DisplayWindow(screen=screen, start_hidden=True)
+
+    # THEN: Window is not shown
+    mocked_show.assert_not_called()
+
+
+@patch.object(DisplayWindow, 'show')
+def test_shown_if_start_hidden_is_not_set(mocked_show, display_window_env, mock_settings):
+    """
+    Tests if DisplayWindow's .show() method is called on constructor if constructed with start_hidden=False
+    """
+
+    # GIVEN: A mocked DisplayWindow's show method, a fake screen and relevant settings
+    settings = {
+        'advanced/x11 bypass wm': False,
+        'core/display on monitor': True
+    }
+    mock_settings.value.side_effect = lambda key: settings[key]
+    screen = Screen(1, QtCore.QRect(0, 0, 800, 600), is_display=True)
+
+    # WHEN: A DisplayWindow is created with start_hidden=True
+    DisplayWindow(screen=screen, start_hidden=False)
+
+    # THEN: Window is shown
+    mocked_show.assert_called()
+
+
 def test_set_scale_not_initialised(display_window_env, mock_settings):
     """
     Test that the scale js is not run if the page is not initialised
@@ -318,6 +361,26 @@ def test_after_loaded_hide_mouse_not_display(display_window_env, mock_settings):
                                                           '});')
 
 
+def test_after_loaded_callback(display_window_env, mock_settings):
+    """
+    Test if the __ is loaded on after_loaded() method correctly
+    """
+    # GIVEN: An initialised display window and settings for item transitions and hide mouse returns true
+    mocked_after_loaded_callback = MagicMock()
+    display_window = DisplayWindow(after_loaded_callback=mocked_after_loaded_callback)
+    display_window.is_display = True
+    mock_settings.value.return_value = True
+    display_window._is_initialised = True
+    display_window.run_javascript = MagicMock()
+    display_window.set_scale = MagicMock()
+    display_window.set_startup_screen = MagicMock()
+
+    # WHEN: after_loaded is run
+    display_window.after_loaded()
+
+    # THEN: The after_loaded_callback should be called
+    mocked_after_loaded_callback.assert_called_once()
+
 @patch.object(time, 'time')
 def test_run_javascript_no_sync_no_wait(mock_time, display_window_env, mock_settings):
     """

tests/openlp_core/ui/test_slidecontroller.py

@@ -1134,6 +1134,61 @@ def test_process_item_song_no_vlc(mocked_execute, registry, state_media):
     assert 2 == slide_controller.preview_display.load_verses.call_count, 'Execute should have been called 2 times'
 
 
+@patch.object(Registry, 'execute')
+def test_process_item_is_reloading_wont_change_display_hide_mode(mocked_execute, registry, state_media):
+    """
+    Test if the display's hide mode is not changed when using is_reloading parameter
+    """
+    # GIVEN: A mocked presentation service item, a mocked media service item, a mocked Registry.execute
+    #        and a slide controller with many mocks.
+    #        and the setting 'themes/item transitions' = True
+    mocked_media_item = MagicMock()
+    mocked_media_item.name = 'mocked_media_item'
+    mocked_media_item.get_transition_delay.return_value = 0
+    mocked_media_item.is_text.return_value = True
+    mocked_media_item.is_command.return_value = False
+    mocked_media_item.is_media.return_value = False
+    mocked_media_item.requires_media.return_value = False
+    mocked_media_item.is_image.return_value = False
+    mocked_media_item.from_service = False
+    mocked_media_item.get_frames.return_value = []
+    mocked_media_item.display_slides = [{'verse': 'Verse name'}]
+    mocked_settings = MagicMock()
+    mocked_settings.value.return_value = True
+    mocked_main_window = MagicMock()
+    Registry().register('main_window', mocked_main_window)
+    Registry().register('media_controller', MagicMock())
+    Registry().register('application', MagicMock())
+    Registry().register('settings', mocked_settings)
+    slide_controller = SlideController(None)
+    slide_controller.service_item = None
+    slide_controller.is_live = True
+    slide_controller._reset_blank = MagicMock()
+    slide_controller.preview_widget = MagicMock()
+    slide_controller.preview_display = MagicMock()
+    slide_controller.enable_tool_bar = MagicMock()
+    slide_controller.on_controller_size_changed = MagicMock()
+    slide_controller.on_media_start = MagicMock()
+    slide_controller.on_media_close = MagicMock()
+    slide_controller.slide_selected = MagicMock()
+    slide_controller.set_hide_mode = MagicMock()
+    slide_controller.new_song_menu = MagicMock()
+    slide_controller.on_stop_loop = MagicMock()
+    slide_controller.info_label = MagicMock()
+    slide_controller.song_menu = MagicMock()
+    slide_controller.displays = [MagicMock()]
+    slide_controller.toolbar = MagicMock()
+    slide_controller.split = 0
+    slide_controller.type_prefix = 'test'
+    slide_controller.screen_capture = 'old_capture'
+
+    # WHEN: _process_item is called with is_reloading=True
+    slide_controller._process_item(mocked_media_item, 0, is_reloading=True)
+
+    # THEN: set_hide_mode should not be called
+    slide_controller.set_hide_mode.assert_not_called()
+
+
 def test_live_stolen_focus_shortcuts(settings):
     """
     Test that all the needed shortcuts are available in scenarios where Live has stolen focus.