From 3067175ff54a8c20708788feea15f93163e8bb47 Mon Sep 17 00:00:00 2001 From: Mateus Meyer Jiacomelli Date: Sat, 19 Nov 2022 17:12:30 -0300 Subject: [PATCH 1/7] handling screen display change corner cases --- openlp/core/display/screens.py | 24 ++++++-- openlp/core/display/window.py | 8 ++- openlp/core/ui/mainwindow.py | 48 +++++++++------ openlp/core/ui/slidecontroller.py | 18 +++++- tests/openlp_core/display/test_window.py | 63 ++++++++++++++++++++ tests/openlp_core/ui/test_slidecontroller.py | 55 +++++++++++++++++ 6 files changed, 188 insertions(+), 28 deletions(-) diff --git a/openlp/core/display/screens.py b/openlp/core/display/screens.py index d8431f14e..1fa0b250d 100644 --- a/openlp/core/display/screens.py +++ b/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) diff --git a/openlp/core/display/window.py b/openlp/core/display/window.py index eaf329760..5d7e6f42c 100644 --- a/openlp/core/display/window.py +++ b/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): """ diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 583ef9a5c..5b9176c01 100644 --- a/openlp/core/ui/mainwindow.py +++ b/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() - self.renderer.resize(self.live_controller.screens.current.display_geometry.size()) - self.preview_controller.screen_size_changed() - self.live_controller.setup_displays() - self.live_controller.screen_size_changed() - self.setFocus() - self.activateWindow() - self.application.set_normal_cursor() - # if a warning has been shown within the last 5 seconds, skip showing again to avoid spamming user, - # also do not show if the settings window is visible - if not self.settings_form.isVisible() and not self.screen_change_timestamp or \ - self.screen_change_timestamp and (datetime.now() - self.screen_change_timestamp).seconds > 5: - self.screen_change_timestamp = datetime.now() - QtWidgets.QMessageBox.warning(self, translate('OpenLP.MainWindow', 'Screen setup has changed'), - translate('OpenLP.MainWindow', - 'The screen setup has changed. ' - 'OpenLP will try to automatically select a display screen, but ' - 'you should consider updating the screen settings.'), - QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok)) + try: + self.screen_updating_lock.acquire() + # if a warning has been shown within the last 5 seconds, skip showing again to avoid spamming user, + # also do not show if the settings window is visible + if not self.settings_form.isVisible() and not self.screen_change_timestamp or \ + self.screen_change_timestamp and (datetime.now() - self.screen_change_timestamp).seconds > 5: + self.screen_change_timestamp = datetime.now() + QtWidgets.QMessageBox.warning(self, translate('OpenLP.MainWindow', 'Screen setup has changed'), + translate('OpenLP.MainWindow', + 'The screen setup has changed. ' + 'OpenLP will try to automatically select a display screen, but ' + 'you should consider updating the screen settings.'), + QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok)) + self.application.set_busy_cursor() + self.renderer.resize(self.live_controller.screens.current.display_geometry.size()) + self.preview_controller.screen_size_changed() + self.live_controller.setup_displays() + self.live_controller.screen_size_changed() + 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): """ diff --git a/openlp/core/ui/slidecontroller.py b/openlp/core/ui/slidecontroller.py index a0158d159..82fa29a52 100644 --- a/openlp/core/ui/slidecontroller.py +++ b/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') diff --git a/tests/openlp_core/display/test_window.py b/tests/openlp_core/display/test_window.py index 4b7631538..c73e84776 100644 --- a/tests/openlp_core/display/test_window.py +++ b/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): """ diff --git a/tests/openlp_core/ui/test_slidecontroller.py b/tests/openlp_core/ui/test_slidecontroller.py index 61ecaf210..f36331890 100644 --- a/tests/openlp_core/ui/test_slidecontroller.py +++ b/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. From f79f8ae4cd5d8070a486de25e380e4e9d90f1a5a Mon Sep 17 00:00:00 2001 From: Mateus Meyer Jiacomelli Date: Sat, 19 Nov 2022 17:27:07 -0300 Subject: [PATCH 2/7] storing screen changed timestamp after modal dismissed --- openlp/core/ui/mainwindow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 5b9176c01..a7e9737b0 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -1023,13 +1023,13 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert # also do not show if the settings window is visible if not self.settings_form.isVisible() and not self.screen_change_timestamp or \ self.screen_change_timestamp and (datetime.now() - self.screen_change_timestamp).seconds > 5: - self.screen_change_timestamp = datetime.now() QtWidgets.QMessageBox.warning(self, translate('OpenLP.MainWindow', 'Screen setup has changed'), translate('OpenLP.MainWindow', 'The screen setup has changed. ' 'OpenLP will try to automatically select a display screen, but ' 'you should consider updating the screen settings.'), QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok)) + self.screen_change_timestamp = datetime.now() self.application.set_busy_cursor() self.renderer.resize(self.live_controller.screens.current.display_geometry.size()) self.preview_controller.screen_size_changed() From 2f3a575e7c93b24cf50b18e743a838a01ed531a8 Mon Sep 17 00:00:00 2001 From: Mateus Meyer Jiacomelli Date: Mon, 21 Nov 2022 11:19:40 -0300 Subject: [PATCH 3/7] preserving hide mode after screen change --- openlp/core/ui/slidecontroller.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/openlp/core/ui/slidecontroller.py b/openlp/core/ui/slidecontroller.py index 82fa29a52..41aaf5203 100644 --- a/openlp/core/ui/slidecontroller.py +++ b/openlp/core/ui/slidecontroller.py @@ -193,10 +193,9 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties): # 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) + # Restoring last hide mode + if self._current_hide_mode: + self.display.hide_display(self._current_hide_mode) def close_displays(self): """ From b9d507b4559c474f9140771f0d52693fe8399ef0 Mon Sep 17 00:00:00 2001 From: Mateus Meyer Jiacomelli Date: Mon, 21 Nov 2022 11:46:43 -0300 Subject: [PATCH 4/7] fixing tests --- tests/openlp_core/display/test_window.py | 1 + tests/openlp_core/ui/test_mainwindow.py | 14 ++++---------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/tests/openlp_core/display/test_window.py b/tests/openlp_core/display/test_window.py index c73e84776..0a0087917 100644 --- a/tests/openlp_core/display/test_window.py +++ b/tests/openlp_core/display/test_window.py @@ -381,6 +381,7 @@ def test_after_loaded_callback(display_window_env, mock_settings): # 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): """ diff --git a/tests/openlp_core/ui/test_mainwindow.py b/tests/openlp_core/ui/test_mainwindow.py index 1e5f7efe8..7e213a826 100644 --- a/tests/openlp_core/ui/test_mainwindow.py +++ b/tests/openlp_core/ui/test_mainwindow.py @@ -771,9 +771,9 @@ def test_screen_changed_modal(mocked_warning, main_window): @patch('openlp.core.ui.mainwindow.QtWidgets.QMessageBox.warning') -def test_screen_changed_modal_sets_timestamp_before_blocking_on_modal(mocked_warning, main_window): +def test_screen_changed_modal_sets_timestamp_after_blocking_on_modal(mocked_warning, main_window): """ - Test that the screen changed modal latest shown timestamp is set before showing warning message, so + Test that the screen changed modal latest shown timestamp is set after showing warning message, so that duplicate modals due to event spamming on 'config_screen_changed' in less than 5 seconds is mitigated. """ # GIVEN: a newly opened OpenLP instance, mocked screens, renderer and an special QMessageBox warning handler @@ -781,19 +781,13 @@ def test_screen_changed_modal_sets_timestamp_before_blocking_on_modal(mocked_war main_window._preview_controller = MagicMock() main_window._renderer = MagicMock() - def resets_timestamp(*args, **kwargs): - nonlocal main_window - main_window.screen_change_timestamp = None - - mocked_warning.side_effect = resets_timestamp - # WHEN: we trigger a 'config_screen_changed' event Registry().execute('config_screen_changed') - # THEN: main_window.screen_change_timestamp should be "None", indicating that timestamp is set before + # THEN: main_window.screen_change_timestamp should have a timestamp, indicating that timestamp is set after # the blocking modal is shown. mocked_warning.assert_called_once() - assert main_window.screen_change_timestamp is None + assert main_window.screen_change_timestamp is not None @patch('openlp.core.ui.mainwindow.QtWidgets.QMessageBox.critical') From 4cb5cca85d39871e713e6b4125ea4f829edae29a Mon Sep 17 00:00:00 2001 From: Mateus Meyer Jiacomelli Date: Tue, 22 Nov 2022 15:40:54 -0300 Subject: [PATCH 5/7] creating ConfigScreenChangedEmitter singleton to encapsulate debounced screen change emitter; simplifying main_window's screen_changed warning modal condition --- openlp/core/display/screens.py | 29 ++++++++++++++++------------- openlp/core/ui/mainwindow.py | 8 +++++--- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/openlp/core/display/screens.py b/openlp/core/display/screens.py index 1fa0b250d..53043a08b 100644 --- a/openlp/core/display/screens.py +++ b/openlp/core/display/screens.py @@ -163,7 +163,7 @@ class Screen(object): Callback function for when the screens geometry changes """ self.geometry = geometry - emit_config_screen_changed() + ConfigScreenChangedEmitter().emit() 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) - emit_config_screen_changed() + ConfigScreenChangedEmitter().emit() 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() - emit_config_screen_changed() + ConfigScreenChangedEmitter().emit() def on_primary_screen_changed(self): """ @@ -426,21 +426,24 @@ 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() + ConfigScreenChangedEmitter().emit() SCREEN_CHANGED_DEBOUNCE_TIMEOUT = 350 -def emit_config_screen_changed(): - screen_changed_debounce.start() +class ConfigScreenChangedEmitter(metaclass=Singleton): + def __init__(self): + self.timer = QtCore.QTimer(None) + self.timer.setInterval(SCREEN_CHANGED_DEBOUNCE_TIMEOUT) + self.timer.setSingleShot(True) + self.timer.timeout.connect(self.__do_emit_config_screen_changed) + def emit(self): + self.timer.start() -def __do_emit_config_screen_changed(): - Registry().execute('config_screen_changed') + def __do_emit_config_screen_changed(self): + 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) + def __del__(self): + self.timer.stop() diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index a7e9737b0..06b9bf3e8 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -40,7 +40,7 @@ from openlp.core.common.path import create_paths, resolve from openlp.core.common.platform import is_macosx, is_win from openlp.core.common.registry import Registry from openlp.core.common.settings import Settings -from openlp.core.display.screens import ScreenList +from openlp.core.display.screens import ConfigScreenChangedEmitter, ScreenList from openlp.core.lib.plugin import PluginStatus from openlp.core.lib.ui import create_action from openlp.core.projectors.manager import ProjectorManager @@ -1021,8 +1021,10 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert self.screen_updating_lock.acquire() # if a warning has been shown within the last 5 seconds, skip showing again to avoid spamming user, # also do not show if the settings window is visible - if not self.settings_form.isVisible() and not self.screen_change_timestamp or \ - self.screen_change_timestamp and (datetime.now() - self.screen_change_timestamp).seconds > 5: + has_shown_messagebox_recently = self.screen_change_timestamp \ + and (datetime.now() - self.screen_change_timestamp).seconds < 5 + should_show_messagebox = self.settings_form.isHidden() and not has_shown_messagebox_recently + if should_show_messagebox: QtWidgets.QMessageBox.warning(self, translate('OpenLP.MainWindow', 'Screen setup has changed'), translate('OpenLP.MainWindow', 'The screen setup has changed. ' From 38ed5075601d38c7d011f0027e7cb810fde2e885 Mon Sep 17 00:00:00 2001 From: Mateus Meyer Jiacomelli Date: Tue, 22 Nov 2022 15:44:36 -0300 Subject: [PATCH 6/7] creating ConfigScreenChangedEmitter singleton to encapsulate debounced screen change emitter; simplifying main_window's screen_changed warning modal condition --- openlp/core/display/screens.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/openlp/core/display/screens.py b/openlp/core/display/screens.py index 53043a08b..5ba8bb267 100644 --- a/openlp/core/display/screens.py +++ b/openlp/core/display/screens.py @@ -444,6 +444,3 @@ class ConfigScreenChangedEmitter(metaclass=Singleton): def __do_emit_config_screen_changed(self): Registry().execute('config_screen_changed') - - def __del__(self): - self.timer.stop() From d430ad7e12efdb2a3e6a73a258a714c12d0e9251 Mon Sep 17 00:00:00 2001 From: Mateus Meyer Jiacomelli Date: Tue, 22 Nov 2022 15:45:21 -0300 Subject: [PATCH 7/7] lint fix --- openlp/core/ui/mainwindow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 06b9bf3e8..a3243a7b6 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -40,7 +40,7 @@ from openlp.core.common.path import create_paths, resolve from openlp.core.common.platform import is_macosx, is_win from openlp.core.common.registry import Registry from openlp.core.common.settings import Settings -from openlp.core.display.screens import ConfigScreenChangedEmitter, ScreenList +from openlp.core.display.screens import ScreenList from openlp.core.lib.plugin import PluginStatus from openlp.core.lib.ui import create_action from openlp.core.projectors.manager import ProjectorManager