From 526a46d002cd2059856b98a8bebc239058da5940 Mon Sep 17 00:00:00 2001 From: Mateus Meyer Jiacomelli Date: Tue, 1 Mar 2022 08:04:27 +0000 Subject: [PATCH] Adjusting display transitions that causes content flashes --- openlp/core/display/html/display.css | 21 +++ openlp/core/display/html/display.js | 167 ++++++++++++++++++++--- openlp/core/display/window.py | 72 +++++++++- tests/js/test_display.js | 85 ++++++++++++ tests/openlp_core/display/test_window.py | 134 +++++++++++++++++- 5 files changed, 454 insertions(+), 25 deletions(-) diff --git a/openlp/core/display/html/display.css b/openlp/core/display/html/display.css index 5307e589a..053ddc30e 100644 --- a/openlp/core/display/html/display.css +++ b/openlp/core/display/html/display.css @@ -46,6 +46,27 @@ body.transition .reveal .footer { transition: opacity 800ms ease-in-out !important; } +body { + /* + It's a fake transition, but the transitionend event will be + fired at the end of the transition time. For the user there will + be no animation (due to the curve + */ + transition: opacity 75ms steps(1, start); +} + +body.disable-transitions *, +body.disable-transitions.transition, +body.disable-transitions.transition .reveal .slides, +body.disable-transitions.transition .reveal .footer { + transition: none !important; +} + +body.is-desktop .pause-overlay { + /* Avoids a black flash while activating "Show Desktop" */ + opacity: 0 !important; +} + .reveal .slide-background { opacity: 1; visibility: visible; diff --git a/openlp/core/display/html/display.js b/openlp/core/display/html/display.js index c19a177bb..3f1996895 100644 --- a/openlp/core/display/html/display.js +++ b/openlp/core/display/html/display.js @@ -282,6 +282,8 @@ var Display = { width: "100%", height: "100%" }, + _lastRequestAnimationFrameHandle: null, + /** * Start up reveal and do any other initialisation * @param {object} options - The initialisation options: @@ -376,7 +378,7 @@ var Display = { Reveal.slide(0, currentSlide.v); Reveal.sync(); Display._removeLastSection(); - Display._skipNextTransition = false; + Display._skipNextTransition = false; } }, /** @@ -821,17 +823,37 @@ var Display = { /** * Blank the screen */ - toBlack: function () { - var documentBody = $("body")[0]; - documentBody.style.opacity = 1; - if (!Reveal.isPaused()) { - Reveal.togglePause(); - } + toBlack: function (onFinishedEventName) { + /* Avoid race conditions where display goes to transparent and quickly goes to black */ + Display._abortLastTransitionOperation(); + /* + Reveal's black overlay should be shown before the transitions are + restored, to avoid screen flashes + */ + Display._restorePauseBehavior(); + Display._requestAnimationFrameExclusive(function() { + if (!Reveal.isPaused()) { + Reveal.togglePause(); + } + Display._reenableGlobalTransitions(function() { + var documentBody = $("body")[0]; + documentBody.style.opacity = 1; + if (onFinishedEventName) { + displayWatcher.dispatchEvent(onFinishedEventName, {}); + } + }); + }); }, /** * Hide all but theme background */ - toTheme: function () { + toTheme: function (onFinishedEventName) { + Display._abortLastTransitionOperation(); + /* + Reveal's black overlay should be shown before the transitions are + restored, to avoid screen flashes + */ + Display._restorePauseBehavior(); var documentBody = $("body")[0]; documentBody.style.opacity = 1; Display._slidesContainer.style.opacity = 0; @@ -839,31 +861,140 @@ var Display = { if (Reveal.isPaused()) { Reveal.togglePause(); } + Display._reenableGlobalTransitions(function() { + if (onFinishedEventName) { + displayWatcher.dispatchEvent(onFinishedEventName, {}); + } + }); }, /** * Hide everything (CAUTION: Causes a invisible mouse barrier) */ - toTransparent: function () { - Display._slidesContainer.style.opacity = 0; - Display._footerContainer.style.opacity = 0; + toTransparent: function (onFinishedEventName) { + Display._abortLastTransitionOperation(); var documentBody = $("body")[0]; documentBody.style.opacity = 0; if (!Reveal.isPaused()) { + /* + Removing previously the overlay if it's not paused, to avoid a + content flash while going from black screen to transparent + */ + document.body.classList.add('is-desktop'); Reveal.togglePause(); } + /* + Waiting for body transition to happen, now it would be safe to + hide the Webview (as other transitions were suppressed) + */ + Display._abortLastTransitionOperation(); + Display._addTransitionEndEventToBody(transitionEndEvent); + function transitionEndEvent(e) { + // Targeting only body + if (e.target != documentBody) { + return; + } + /* + Disabling all transitions (except body) to allow the Webview to attain the + transparent state before it gets hidden by Qt. + */ + document.body.classList.add('disable-transitions'); + document.body.classList.add('is-desktop'); + Display._slidesContainer.style.opacity = 0; + Display._footerContainer.style.opacity = 0; + /* + Repainting before hiding the Webview to avoid flashes when + showing it again. + */ + displayWatcher.pleaseRepaint(); + /* Waiting for repaint to happen before saying that it's done. */ + Display._requestAnimationFrameExclusive(function() { + /* We're transparent now, aborting any transition event between */ + Display._abortLastTransitionOperation(); + if (onFinishedEventName) { + displayWatcher.dispatchEvent(onFinishedEventName, {}); + } + }); + } }, /** * Show the screen */ - show: function () { + show: function (onFinishedEventName) { var documentBody = $("body")[0]; - documentBody.style.opacity = 1; + /* + Removing transitionend event, avoids the content being hidden if the user + tries to show content again before toTransparent() transitionend event + happens + */ + Display._abortLastTransitionOperation(); + Display._slidesContainer.style.opacity = 1; Display._footerContainer.style.opacity = 1; if (Reveal.isPaused()) { Reveal.togglePause(); } + Display._restorePauseBehavior(); + Display._reenableGlobalTransitions(function() { + documentBody.style.opacity = 1; + if (onFinishedEventName) { + displayWatcher.dispatchEvent(onFinishedEventName, {}); + } + }); }, + + _reenableGlobalTransitions: function(afterCallback) { + Display._requestAnimationFrameExclusive(function() { + /* + Waiting for the previous opacity + unpause operations to complete + to restore the transitions behavior + */ + document.body.classList.remove('disable-transitions'); + if (typeof afterCallback === 'function') { + afterCallback(); + } + }); + }, + + /** + * Shows again the Reveal's black pause overlay that was + * hidden before Webview was hidden + */ + _restorePauseBehavior: function() { + document.body.classList.remove('is-desktop'); + }, + + /** + * Cancels previous requested animationFrame. + * Last animationFrame should be aborted to avoid race condition bugs when + * the user changes the view modes too quickly, for example. + */ + _requestAnimationFrameExclusive: function(callback) { + cancelAnimationFrame(Display._lastRequestAnimationFrameHandle); + Display._lastRequestAnimationFrameHandle = requestAnimationFrame(callback); + }, + + /** + * Aborts last body's transitionend and requestAnimationFrame's events, to avoid + * race condition bugs. + */ + _abortLastTransitionOperation: function() { + Display._removeTransitionEndEventToBody(); + cancelAnimationFrame(Display._lastRequestAnimationFrameHandle); + }, + + /** + * Intercepts the addEventListener call and stores it, so that it acts + * like the ontransitionend GlobalEventHandler. + */ + _addTransitionEndEventToBody: function(listener) { + Display._lastTransitionEndBodyEvent = listener; + document.body.addEventListener('transitionend', listener); + }, + + _removeTransitionEndEventToBody: function() { + document.body.removeEventListener('transitionend', Display._lastTransitionEndBodyEvent); + }, + /** * Figure out how many lines can fit on a slide given the font size * @param fontSize The font size in pts @@ -1129,11 +1260,11 @@ var Display = { * and we don't want any flashbacks to the current slide contents */ finishWithCurrentItem: function () { - Display.setTextSlide(''); - var documentBody = $("body")[0]; + Display.setTextSlide(''); + var documentBody = $("body")[0]; documentBody.style.opacity = 1; - Display._skipNextTransition = true; - displayWatcher.pleaseRepaint(); + Display._skipNextTransition = true; + displayWatcher.pleaseRepaint(); }, /** * Return the video types supported by the video tag @@ -1185,7 +1316,7 @@ var Display = { */ setFooterSlideNumbers: function (slide) { let value = ['', '', '']; - // Reveal does call this function passing undefined + // Reveal does call this function passing undefined if (typeof slide === 'undefined') { return value; } diff --git a/openlp/core/display/window.py b/openlp/core/display/window.py index 3de5fff84..0b9e058e6 100644 --- a/openlp/core/display/window.py +++ b/openlp/core/display/window.py @@ -42,6 +42,7 @@ from openlp.core.ui import HideMode FONT_FOUNDRY = re.compile(r'(.*?) \[(.*?)\]') +TRANSITION_END_EVENT_NAME = 'transparent_transition_end' log = logging.getLogger(__name__) @@ -54,6 +55,9 @@ class DisplayWatcher(QtCore.QObject): def __init__(self, parent): super().__init__() self._display_window = parent + self._transient_dispatch_events = {} + self._permanent_dispatch_events = {} + self._event_counter = 0 @QtCore.pyqtSlot(bool) def setInitialised(self, is_initialised): @@ -70,6 +74,57 @@ class DisplayWatcher(QtCore.QObject): """ self._display_window.webview.update() + @QtCore.pyqtSlot(str, 'QJsonObject') + def dispatchEvent(self, event_name, event_data): + """ + Called from the js in the webengine view for event dispatches + """ + transient_dispatch_events = self._transient_dispatch_events + permanent_dispatch_events = self._permanent_dispatch_events + if event_name in transient_dispatch_events: + event = transient_dispatch_events[event_name] + del transient_dispatch_events[event_name] + event(event_data) + if event_name in permanent_dispatch_events: + permanent_dispatch_events[event_name](event_data) + + def register_event_listener(self, event_name, callback, transient=True): + """ + Register an event listener from webengine view + :param event_name: Event name + :param callback: Callback listener when event happens + :param transient: If the event listener should be unregistered after being run + """ + if transient: + events = self._transient_dispatch_events + else: + events = self._permanent_dispatch_events + + events[event_name] = callback + + def unregister_event_listener(self, event_name, transient=True): + """ + Unregisters an event listener from webengine view + :param event_name: Event name + :param transient: If the event listener was registered as transient + """ + if transient: + events = self._transient_dispatch_events + else: + events = self._permanent_dispatch_events + + if event_name in events: + del events[event_name] + + def get_unique_event_name(self): + """ + Generates an unique event name + :returns: Unique event name + """ + event_count = self._event_counter + self._event_counter += 1 + return 'event_' + str(event_count) + class DisplayWindow(QtWidgets.QWidget, RegistryProperties, LogMixin): """ @@ -431,9 +486,11 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties, LogMixin): # Only make visible on single monitor setup if setting enabled. if len(ScreenList()) == 1 and not self.settings.value('core/display on monitor'): return - self.run_javascript('Display.show();') + # Aborting setVisible(False) call in case the display modes are changed quickly + self.display_watcher.unregister_event_listener(TRANSITION_END_EVENT_NAME) if self.isHidden(): self.setVisible(True) + self.run_javascript('Display.show();') self.hide_mode = None def hide_display(self, mode=HideMode.Screen): @@ -447,19 +504,24 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties, LogMixin): # Only make visible on single monitor setup if setting enabled. if len(ScreenList()) == 1 and not self.settings.value('core/display on monitor'): return + # Aborting setVisible(False) call in case the display modes are changed quickly + self.display_watcher.unregister_event_listener(TRANSITION_END_EVENT_NAME) # Update display to the selected mode + if mode != HideMode.Screen: + if self.isHidden(): + self.setVisible(True) if mode == HideMode.Screen: if self.settings.value('advanced/disable transparent display'): - self.setVisible(False) + # Hide window only after all webview CSS ransitions are done + self.display_watcher.register_event_listener(TRANSITION_END_EVENT_NAME, + lambda _: self.setVisible(False)) + self.run_javascript("Display.toTransparent('{}');".format(TRANSITION_END_EVENT_NAME)) else: self.run_javascript('Display.toTransparent();') elif mode == HideMode.Blank: self.run_javascript('Display.toBlack();') elif mode == HideMode.Theme: self.run_javascript('Display.toTheme();') - if mode != HideMode.Screen: - if self.isHidden(): - self.setVisible(True) self.hide_mode = mode def disable_display(self): diff --git a/tests/js/test_display.js b/tests/js/test_display.js index c6c73f4bc..88e3f93af 100644 --- a/tests/js/test_display.js +++ b/tests/js/test_display.js @@ -246,7 +246,92 @@ describe("Transitions", function () { expect(Display._slidesContainer.children[0].children[0].getAttribute("data-transition")).toEqual("convex-vertical-reverse"); expect(Display._slidesContainer.children[0].children[0].getAttribute("data-transition-speed")).toEqual("slow"); }); +}); + +describe("Screen Visibility", function () { + var TRANSITION_TIMEOUT = 2000; + + beforeEach(function() { + window.displayWatcher = jasmine.createSpyObj('DisplayWatcher', ['dispatchEvent', 'setInitialised', 'pleaseRepaint']); + document.body.innerHTML = ""; + var revealDiv = _createDiv({"class": "reveal"}); + var slidesDiv = _createDiv({"class": "slides"}); + var footerDiv = _createDiv({"class": "footer"}); + slidesDiv.innerHTML = "

"; + revealDiv.append(slidesDiv); + revealDiv.append(footerDiv); + document.body.style.transition = "opacity 75ms ease-in-out"; + Display.init({isDisplay: true, doItemTransition: false}); + }); + afterEach(function() { + // Reset theme + Display._theme = null; + }); + + it("should trigger dispatchEvent when toTransparent(eventName) is called with an event parameter", function (done) { + var testEventName = 'event_32'; + displayWatcher.dispatchEvent = function(eventName) { + if (eventName == testEventName) { + done(); + } + }; + + Display.toTransparent(testEventName); + + setTimeout(function() { + fail('dispatchEvent not called'); + done(); + }, TRANSITION_TIMEOUT); + }); + + it("should trigger dispatchEvent when toBlack(eventName) is called with an event parameter", function (done) { + var testEventName = 'event_33'; + displayWatcher.dispatchEvent = function(eventName) { + if (eventName == testEventName) { + done(); + } + }; + + Display.toBlack(testEventName); + + setTimeout(function() { + fail('dispatchEvent not called'); + done(); + }, TRANSITION_TIMEOUT); + }); + + it("should trigger dispatchEvent when toTheme(eventName) is called with an event parameter", function (done) { + var testEventName = 'event_34'; + displayWatcher.dispatchEvent = function(eventName) { + if (eventName == testEventName) { + done(); + } + }; + + Display.toTheme(testEventName); + + setTimeout(function() { + fail('dispatchEvent not called'); + done(); + }, TRANSITION_TIMEOUT); + }); + + it("should trigger dispatchEvent when show(eventName) is called with an event parameter", function (done) { + var testEventName = 'event_35'; + displayWatcher.dispatchEvent = function(eventName) { + if (eventName == testEventName) { + done(); + } + }; + + Display.show(testEventName); + + setTimeout(function() { + fail('dispatchEvent not called'); + done(); + }, TRANSITION_TIMEOUT); + }); }); describe("Display.alert", function () { diff --git a/tests/openlp_core/display/test_window.py b/tests/openlp_core/display/test_window.py index 3f504238f..4154fea5d 100644 --- a/tests/openlp_core/display/test_window.py +++ b/tests/openlp_core/display/test_window.py @@ -33,7 +33,7 @@ from PyQt5 import QtCore # Mock QtWebEngineWidgets sys.modules['PyQt5.QtWebEngineWidgets'] = MagicMock() -from openlp.core.display.window import DisplayWindow, DisplayWatcher +from openlp.core.display.window import TRANSITION_END_EVENT_NAME, DisplayWindow, DisplayWatcher from openlp.core.common import is_win from openlp.core.common.enum import ServiceItemType from openlp.core.lib.theme import Theme @@ -602,20 +602,150 @@ def test_hide_display_to_transparent(display_window_env, mock_settings): assert display_window.setVisible.call_count == 0 +def test_display_watcher_dispatches_registered_event(display_window_env, mock_settings): + """ + Test that the display watcher dispatches events to the registered listeners + """ + # GIVEN: Display window and a dummy event + event_name = 'dummy_event' + event_listener = MagicMock() + display_window = DisplayWindow() + display_window.display_watcher.register_event_listener(event_name, event_listener) + + # WHEN: Event is dispatched + display_window.display_watcher.dispatchEvent(event_name, {}) + + # THEN: Events should be called + event_listener.assert_called_once() + + +def test_display_watcher_dispatches_permanent_registered_event(display_window_env, mock_settings): + """ + Test that the display watcher dispatches events to the permanent registered listeners + """ + # GIVEN: Display window and a dummy event + event_name = 'dummy_event' + event_listener = MagicMock() + display_window = DisplayWindow() + display_window.display_watcher.register_event_listener(event_name, event_listener, True) + + # WHEN: Event is dispatched + display_window.display_watcher.dispatchEvent(event_name, {}) + + # THEN: Events should be called + event_listener.assert_called_once() + + +def test_display_watcher_dispatches_transient_and_permanent_registered_event(display_window_env, mock_settings): + """ + Test that the display watcher dispatches events to both transient and permanent registered listeners + """ + # GIVEN: Display window and a dummy event + event_name = 'dummy_event' + event_listener = MagicMock() + event_listener_permanent = MagicMock() + display_window = DisplayWindow() + display_window.display_watcher.register_event_listener(event_name, event_listener, True) + display_window.display_watcher.register_event_listener(event_name, event_listener_permanent, False) + + # WHEN: Event is dispatched + display_window.display_watcher.dispatchEvent(event_name, {}) + + # THEN: Events should be called + event_listener.assert_called_once() + event_listener_permanent.assert_called_once() + + +def test_display_watcher_unregisters_registered_event(display_window_env, mock_settings): + """ + Test that the display watcher unregisters registered listeners + """ + # GIVEN: Display window and a dummy event that is unregistered later + event_name = 'dummy_event' + event_listener = MagicMock() + display_window = DisplayWindow() + display_window.display_watcher.register_event_listener(event_name, event_listener) + display_window.display_watcher.unregister_event_listener(event_name) + + # WHEN: Event is dispatched + display_window.display_watcher.dispatchEvent(event_name, {}) + + # THEN: Events should not be called + event_listener.assert_not_called() + + +def test_display_watcher_unregisters_registered_permanent_event(display_window_env, mock_settings): + """ + Test that the display watcher unregisters registered permanent listeners + """ + # GIVEN: Display window and a dummy event that is unregistered later + event_name = 'dummy_event' + event_listener = MagicMock() + display_window = DisplayWindow() + display_window.display_watcher.register_event_listener(event_name, event_listener, True) + display_window.display_watcher.unregister_event_listener(event_name) + + # WHEN: Event is dispatched + display_window.display_watcher.dispatchEvent(event_name, {}) + + # THEN: Events should not be called + event_listener.assert_not_called() + + +def test_display_watcher_unregisters_registered_permanent_and_transient_event(display_window_env, mock_settings): + """ + Test that the display watcher unregisters registered listeners, both permanent and transient + """ + # GIVEN: Display window and a dummy event that is unregistered later + event_name = 'dummy_event' + event_listener = MagicMock() + event_listener_permanent = MagicMock() + display_window = DisplayWindow() + display_window.display_watcher.register_event_listener(event_name, event_listener) + display_window.display_watcher.register_event_listener(event_name, event_listener_permanent, False) + display_window.display_watcher.unregister_event_listener(event_name) + display_window.display_watcher.unregister_event_listener(event_name, False) + + # WHEN: Event is dispatched + display_window.display_watcher.dispatchEvent(event_name, {}) + + # THEN: Events should not be called + event_listener.assert_not_called() + event_listener_permanent.assert_not_called() + + def test_hide_transparent_to_screen(display_window_env, mock_settings): """ Test that when going transparent, and the disable transparent setting is enabled, the screen mode should be used. """ - # GIVEN: Display window and setting advanced/disable transparent display = True + # GIVEN: Display window, setting advanced/disable transparent display = True and mocked run_javascript display_window = DisplayWindow() display_window.setVisible = MagicMock() + has_ran_event = False + + def set_has_ran_event(_): + nonlocal has_ran_event + has_ran_event = True + + def on_dispatch_event(_): + display_window.display_watcher.register_event_listener(TRANSITION_END_EVENT_NAME, set_has_ran_event, False) + display_window.display_watcher.dispatchEvent(TRANSITION_END_EVENT_NAME, {}) + + display_window.run_javascript = MagicMock(side_effect=on_dispatch_event) mock_settings.value.return_value = True # WHEN: Hide display is run with HideMode.Screen display_window.hide_display(HideMode.Screen) # THEN: Should run setVisible(False) + elapsed_time = 0 + while not has_ran_event: + time.sleep(0.05) + elapsed_time += 0.05 + if elapsed_time > 1: + break + assert has_ran_event is True display_window.setVisible.assert_called_once_with(False)