Merge branch 'previous-verse-flash-fix' into 'master'

Adjusting display transitions that causes content flashes

Closes #979

See merge request openlp/openlp!429

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;

openlp/core/display/html/display.js

@@ -321,6 +321,8 @@ var Display = {
     width: "100%",
     height: "100%"
   },
+  _lastRequestAnimationFrameHandle: null,
+
   /**
    * Start up reveal and do any other initialisation
    * @param {object} options - The initialisation options:
@@ -415,7 +417,7 @@ var Display = {
       Reveal.slide(0, currentSlide.v);
       Reveal.sync();
       Display._removeLastSection();
-	  Display._skipNextTransition = false;
+      Display._skipNextTransition = false;
     }
   },
   /**
@@ -857,17 +859,37 @@ var Display = {
   /**
    * Blank the screen
   */
-  toBlack: function () {
+  toBlack: function (onFinishedEventName) {
-    var documentBody = $("body")[0];
+    /* Avoid race conditions where display goes to transparent and quickly goes to black */
-    documentBody.style.opacity = 1;
+    Display._abortLastTransitionOperation();
-    if (!Reveal.isPaused()) {
+    /*
-      Reveal.togglePause();
+      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;
@@ -875,31 +897,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 () {
+  toTransparent: function (onFinishedEventName) {
-    Display._slidesContainer.style.opacity = 0;
+    Display._abortLastTransitionOperation();
-    Display._footerContainer.style.opacity = 0;
     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
@@ -1171,11 +1302,11 @@ var Display = {
    * and we don't want any flashbacks to the current slide contents
    */
   finishWithCurrentItem: function () {
-	Display.setTextSlide('');
+    Display.setTextSlide('');
-	var documentBody = $("body")[0];
+    var documentBody = $("body")[0];
     documentBody.style.opacity = 1;
-	Display._skipNextTransition = true;
+    Display._skipNextTransition = true;
-	displayWatcher.pleaseRepaint();
+    displayWatcher.pleaseRepaint();
   },
   /**
    * Return the video types supported by the video tag
@@ -1227,7 +1358,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;
     }

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):

tests/js/test_display.js

@@ -308,7 +308,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 = "<section><section><p></p></section></section>";
+    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 () {

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)