Merge lp:~tomasgroth/openlp/fix-theme-thumb into lp:openlp

[Tomas Groth]

Add a webengine view for previewing themes.
Made VLC loading more robust.
A few minor fixes.

[Raoul Snyman (raoul-snyman)]

Linux tests failed, please see https://ci.openlp.io/job/MP-02-Linux_Tests/176/ for more details

[Tim Bentley (trb143)]

review: Approve

[Raoul Snyman (raoul-snyman)]

I like it, but you'll just need to fix the tests.

review: Needs Fixing

[Raoul Snyman (raoul-snyman)]

Linux tests failed, please see https://ci.openlp.io/job/MP-02-Linux_Tests/177/ for more details

[Raoul Snyman (raoul-snyman)]

Linux tests passed!

[Raoul Snyman (raoul-snyman)]

Linting failed, please see https://ci.openlp.io/job/MP-03-Linting/116/ for more details

[Raoul Snyman (raoul-snyman)]

Linux tests passed!

[Raoul Snyman (raoul-snyman)]

Linting passed!

[Tim Bentley (trb143)]

review: Approve

=== modified file 'openlp/core/display/render.py'
--- openlp/core/display/render.py	2019-04-13 13:00:22 +0000
+++ openlp/core/display/render.py	2019-06-09 20:23:18 +0000
@@ -24,6 +24,7 @@
 """
 import html
 import logging
+import mako
 import math
 import os
 import re
@@ -32,8 +33,10 @@
 from PyQt5 import QtWidgets, QtGui
 
 from openlp.core.common import ThemeLevel
+from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import LogMixin, RegistryProperties
 from openlp.core.common.registry import Registry, RegistryBase
+from openlp.core.common.settings import Settings
 from openlp.core.display.screens import ScreenList
 from openlp.core.display.window import DisplayWindow
 from openlp.core.lib import ItemCapabilities
@@ -58,8 +61,10 @@
     '{r}C{/r}{b}h{/b}{bl}i{/bl}{y}l{/y}{g}d{/g}{pk}' \
     'r{/pk}{o}e{/o}{pp}n{/pp} of the Lord\n'
 VERSE_FOR_LINE_COUNT = '\n'.join(map(str, range(100)))
-TITLE = 'Arky Arky (Unknown)'
-FOOTER = ['Public Domain', 'CCLI 123456']
+TITLE = 'Arky Arky'
+AUTHOR = 'John Doe'
+FOOTER_COPYRIGHT = 'Public Domain'
+CCLI_NO = '123456'
 
 
 def remove_tags(text, can_remove_chords=False):
@@ -425,7 +430,7 @@
     return raw_text + ''.join(end_tags), ''.join(start_tags), ''.join(html_tags)
 
 
-class Renderer(RegistryBase, LogMixin, RegistryProperties, DisplayWindow):
+class ThemePreviewRenderer(LogMixin, DisplayWindow):
     """
     A virtual display used for rendering thumbnails and other offscreen tasks
     """
@@ -435,24 +440,6 @@
         """
         super().__init__(*args, **kwargs)
         self.force_page = False
-        for screen in ScreenList():
-            if screen.is_display:
-                self.setGeometry(screen.display_geometry.x(), screen.display_geometry.y(),
-                                 screen.display_geometry.width(), screen.display_geometry.height())
-                break
-        # If the display is not show'ed and hidden like this webegine will not render
-        self.show()
-        self.hide()
-        self.theme_height = 0
-        self.theme_level = ThemeLevel.Global
-
-    def set_theme_level(self, theme_level):
-        """
-        Sets the theme level.
-
-        :param theme_level: The theme level to be used.
-        """
-        self.theme_level = theme_level
 
     def calculate_line_count(self):
         """
@@ -466,7 +453,30 @@
         """
         return self.run_javascript('Display.clearSlides();')
 
-    def generate_preview(self, theme_data, force_page=False):
+    def generate_footer(self):
+        """
+        """
+        footer_template = Settings().value('songs/footer template')
+        # Keep this in sync with the list in songstab.py
+        vars = {
+            'title': TITLE,
+            'authors_none_label': translate('OpenLP.Ui', 'Written by'),
+            'authors_words_label': translate('SongsPlugin.AuthorType', 'Words',
+                                             'Author who wrote the lyrics of a song'),
+            'authors_words': [AUTHOR],
+            'copyright': FOOTER_COPYRIGHT,
+            'ccli_license': Settings().value('core/ccli number'),
+            'ccli_license_label': translate('SongsPlugin.MediaItem', 'CCLI License'),
+            'ccli_number': CCLI_NO,
+        }
+        try:
+            footer_html = mako.template.Template(footer_template).render_unicode(**vars).replace('\n', '')
+        except mako.exceptions.SyntaxException:
+            log.error('Failed to render Song footer html:\n' + mako.exceptions.text_error_template().render())
+            footer_html = 'Dummy footer text'
+        return footer_html
+
+    def generate_preview(self, theme_data, force_page=False, generate_screenshot=True):
         """
         Generate a preview of a theme.
 
@@ -479,14 +489,16 @@
         if not self.force_page:
             self.set_theme(theme_data)
             self.theme_height = theme_data.font_main_height
-            slides = self.format_slide(render_tags(VERSE), None)
+            slides = self.format_slide(VERSE, None)
             verses = dict()
             verses['title'] = TITLE
-            verses['text'] = slides[0]
+            verses['text'] = render_tags(slides[0])
             verses['verse'] = 'V1'
+            verses['footer'] = self.generate_footer()
             self.load_verses([verses])
             self.force_page = False
-            return self.save_screenshot()
+            if generate_screenshot:
+                return self.save_screenshot()
         self.force_page = False
         return None
 
@@ -515,7 +527,7 @@
         if item and item.is_capable(ItemCapabilities.CanWordSplit):
             pages = self._paginate_slide_words(text.split('\n'), line_end)
         # Songs and Custom
-        elif item is None or item.is_capable(ItemCapabilities.CanSoftBreak):
+        elif item is None or (item and item.is_capable(ItemCapabilities.CanSoftBreak)):
             pages = []
             if '[---]' in text:
                 # Remove Overflow split if at start of the text
@@ -722,7 +734,8 @@
         :param text:  The text to check. It may contain HTML tags.
         """
         self.clear_slides()
-        self.run_javascript('Display.addTextSlide("v1", "{text}", "Dummy Footer");'.format(text=text), is_sync=True)
+        self.run_javascript('Display.addTextSlide("v1", "{text}", "Dummy Footer");'
+                            .format(text=text.replace('"', '\\"')), is_sync=True)
         does_text_fits = self.run_javascript('Display.doesContentFit();', is_sync=True)
         return does_text_fits
 
@@ -745,3 +758,33 @@
             pixmap.save(fname, ext)
         else:
             return pixmap
+
+
+class Renderer(RegistryBase, RegistryProperties, ThemePreviewRenderer):
+    """
+    A virtual display used for rendering thumbnails and other offscreen tasks
+    """
+    def __init__(self, *args, **kwargs):
+        """
+        Constructor
+        """
+        super().__init__(*args, **kwargs)
+        self.force_page = False
+        for screen in ScreenList():
+            if screen.is_display:
+                self.setGeometry(screen.display_geometry.x(), screen.display_geometry.y(),
+                                 screen.display_geometry.width(), screen.display_geometry.height())
+                break
+        # If the display is not show'ed and hidden like this webegine will not render
+        self.show()
+        self.hide()
+        self.theme_height = 0
+        self.theme_level = ThemeLevel.Global
+
+    def set_theme_level(self, theme_level):
+        """
+        Sets the theme level.
+
+        :param theme_level: The theme level to be used.
+        """
+        self.theme_level = theme_level
 
=== modified file 'openlp/core/ui/media/vlcplayer.py'
--- openlp/core/ui/media/vlcplayer.py	2019-05-31 20:19:15 +0000
+++ openlp/core/ui/media/vlcplayer.py	2019-06-09 20:23:18 +0000
@@ -28,7 +28,6 @@
 import sys
 import threading
 from datetime import datetime
-import vlc
 
 from PyQt5 import QtWidgets
 
@@ -62,25 +61,27 @@
 
     :return: The "vlc" module, or None
     """
-    if 'vlc' in sys.modules:
-        # If VLC has already been imported, no need to do all the stuff below again
-        is_vlc_available = False
+    # Import the VLC module if not already done
+    if 'vlc' not in sys.modules:
         try:
-            is_vlc_available = bool(sys.modules['vlc'].get_default_instance())
-        except Exception:
-            pass
-        if is_vlc_available:
-            return sys.modules['vlc']
-        else:
+            import vlc  # noqa module is not used directly, but is used via sys.modules['vlc']
+        except ImportError:
             return None
-    else:
-        return vlc
+    # Verify that VLC is also loadable
+    is_vlc_available = False
+    try:
+        is_vlc_available = bool(sys.modules['vlc'].get_default_instance())
+    except Exception:
+        pass
+    if is_vlc_available:
+        return sys.modules['vlc']
+    return None
 
 
 # On linux we need to initialise X threads, but not when running tests.
 # This needs to happen on module load and not in get_vlc(), otherwise it can cause crashes on some DE on some setups
 # (reported on Gnome3, Unity, Cinnamon, all GTK+ based) when using native filedialogs...
-if is_linux() and 'nose' not in sys.argv[0] and get_vlc():
+if is_linux() and 'pytest' not in sys.argv[0] and get_vlc():
     try:
         try:
             x11 = ctypes.cdll.LoadLibrary('libX11.so.6')
 
=== modified file 'openlp/core/ui/themeform.py'
--- openlp/core/ui/themeform.py	2019-05-26 20:53:54 +0000
+++ openlp/core/ui/themeform.py	2019-06-09 20:23:18 +0000
@@ -172,16 +172,14 @@
         if not event:
             event = QtGui.QResizeEvent(self.size(), self.size())
         QtWidgets.QWizard.resizeEvent(self, event)
-        if hasattr(self, 'preview_page') and self.currentPage() == self.preview_page:
-            frame_width = self.preview_box_label.lineWidth()
-            pixmap_width = self.preview_area.width() - 2 * frame_width
-            pixmap_height = self.preview_area.height() - 2 * frame_width
-            aspect_ratio = float(pixmap_width) / pixmap_height
-            if aspect_ratio < self.display_aspect_ratio:
-                pixmap_height = int(pixmap_width / self.display_aspect_ratio + 0.5)
-            else:
-                pixmap_width = int(pixmap_height * self.display_aspect_ratio + 0.5)
-            self.preview_box_label.setFixedSize(pixmap_width + 2 * frame_width, pixmap_height + 2 * frame_width)
+        try:
+            self.display_aspect_ratio = self.renderer.width() / self.renderer.height()
+        except ZeroDivisionError:
+            self.display_aspect_ratio = 1
+        # Make sure we don't resize before the widgets are actually created
+        if hasattr(self, 'preview_area_layout'):
+            self.preview_area_layout.set_aspect_ratio(self.display_aspect_ratio)
+            self.preview_box.set_scale(float(self.preview_box.width()) / self.renderer.width())
 
     def validateCurrentPage(self):
         """
@@ -206,11 +204,17 @@
         self.setOption(QtWidgets.QWizard.HaveCustomButton1, enabled)
         if self.page(page_id) == self.preview_page:
             self.update_theme()
-            frame = self.theme_manager.generate_image(self.theme)
-            frame.setDevicePixelRatio(self.devicePixelRatio())
-            self.preview_box_label.setPixmap(frame)
-            self.display_aspect_ratio = float(frame.width()) / frame.height()
+            self.preview_box.set_theme(self.theme)
+            self.preview_box.clear_slides()
+            self.preview_box.set_scale(float(self.preview_box.width()) / self.renderer.width())
+            try:
+                self.display_aspect_ratio = self.renderer.width() / self.renderer.height()
+            except ZeroDivisionError:
+                self.display_aspect_ratio = 1
+            self.preview_area_layout.set_aspect_ratio(self.display_aspect_ratio)
             self.resizeEvent()
+            self.preview_box.show()
+            self.preview_box.generate_preview(self.theme, False, False)
 
     def on_custom_1_button_clicked(self, number):
         """
@@ -398,6 +402,7 @@
         Handle the display and state of the Preview page.
         """
         self.setField('name', self.theme.theme_name)
+        self.preview_box.set_theme(self.theme)
 
     def on_background_combo_box_current_index_changed(self, index):
         """
@@ -558,5 +563,5 @@
             source_path = self.theme.background_filename
         if not self.edit_mode and not self.theme_manager.check_if_theme_exists(self.theme.theme_name):
             return
-        self.theme_manager.save_theme(self.theme, source_path, destination_path)
+        self.theme_manager.save_theme(self.theme, source_path, destination_path, self.preview_box.save_screenshot())
         return QtWidgets.QDialog.accept(self)
 
=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py	2019-05-22 06:47:00 +0000
+++ openlp/core/ui/thememanager.py	2019-06-09 20:23:18 +0000
@@ -476,7 +476,7 @@
         if not theme_paths:
             theme = Theme()
             theme.theme_name = UiStrings().Default
-            self._write_theme(theme)
+            self.save_theme(theme)
             Settings().setValue(self.settings_section + '/global theme', theme.theme_name)
         self.application.set_normal_cursor()
 
@@ -639,24 +639,14 @@
             return False
         return True
 
-    def save_theme(self, theme, image_source_path, image_destination_path):
-        """
-        Called by theme maintenance Dialog to save the theme and to trigger the reload of the theme list
-
-        :param Theme theme: The theme data object.
-        :param Path image_source_path: Where the theme image is currently located.
-        :param Path image_destination_path: Where the Theme Image is to be saved to
-        :rtype: None
-        """
-        self._write_theme(theme, image_source_path, image_destination_path)
-
-    def _write_theme(self, theme, image_source_path=None, image_destination_path=None):
+    def save_theme(self, theme, image_source_path=None, image_destination_path=None, image=None):
         """
         Writes the theme to the disk and handles the background image if necessary
 
         :param Theme theme: The theme data object.
         :param Path image_source_path: Where the theme image is currently located.
         :param Path image_destination_path: Where the Theme Image is to be saved to
+        :param image: The example image of the theme. Optionally.
         :rtype: None
         """
         name = theme.theme_name
@@ -676,7 +666,15 @@
                     shutil.copyfile(image_source_path, image_destination_path)
                 except OSError:
                     self.log_exception('Failed to save theme image')
-        self.generate_and_save_image(name, theme)
+        if image:
+            sample_path_name = self.theme_path / '{file_name}.png'.format(file_name=name)
+            if sample_path_name.exists():
+                sample_path_name.unlink()
+            image.save(str(sample_path_name), 'png')
+            thumb_path = self.thumb_path / '{name}.png'.format(name=name)
+            create_thumb(sample_path_name, thumb_path, False)
+        else:
+            self.generate_and_save_image(name, theme)
 
     def generate_and_save_image(self, theme_name, theme):
         """
 
=== modified file 'openlp/core/ui/themewizard.py'
--- openlp/core/ui/themewizard.py	2019-04-13 13:00:22 +0000
+++ openlp/core/ui/themewizard.py	2019-06-09 20:23:18 +0000
@@ -31,6 +31,8 @@
 from openlp.core.ui.icons import UiIcons
 from openlp.core.widgets.buttons import ColorButton
 from openlp.core.widgets.edits import PathEdit
+from openlp.core.widgets.layouts import AspectRatioLayout
+from openlp.core.display.render import ThemePreviewRenderer
 
 
 class Ui_ThemeWizard(object):
@@ -363,16 +365,13 @@
         self.preview_layout.addLayout(self.theme_name_layout)
         self.preview_area = QtWidgets.QWidget(self.preview_page)
         self.preview_area.setObjectName('PreviewArea')
-        self.preview_area_layout = QtWidgets.QGridLayout(self.preview_area)
-        self.preview_area_layout.setContentsMargins(0, 0, 0, 0)
-        self.preview_area_layout.setColumnStretch(0, 1)
-        self.preview_area_layout.setRowStretch(0, 1)
-        self.preview_area_layout.setObjectName('preview_area_layout')
-        self.preview_box_label = QtWidgets.QLabel(self.preview_area)
-        self.preview_box_label.setFrameShape(QtWidgets.QFrame.Box)
-        self.preview_box_label.setScaledContents(True)
-        self.preview_box_label.setObjectName('preview_box_label')
-        self.preview_area_layout.addWidget(self.preview_box_label)
+        self.preview_area_layout = AspectRatioLayout(self.preview_area, 0.75)  # Dummy ratio, will be update
+        self.preview_area_layout.margin = 8
+        self.preview_area_layout.setSpacing(0)
+        self.preview_area_layout.setObjectName('preview_web_layout')
+        self.preview_box = ThemePreviewRenderer(self)
+        self.preview_box.setObjectName('preview_box')
+        self.preview_area_layout.addWidget(self.preview_box)
         self.preview_layout.addWidget(self.preview_area)
         theme_wizard.addPage(self.preview_page)
         self.retranslate_ui(theme_wizard)
 
=== modified file 'openlp/plugins/media/lib/mediaitem.py'
--- openlp/plugins/media/lib/mediaitem.py	2019-06-01 06:59:45 +0000
+++ openlp/plugins/media/lib/mediaitem.py	2019-06-09 20:23:18 +0000
@@ -173,7 +173,7 @@
             item = self.list_view.currentItem()
             if item is None:
                 return False
-        filename = item.data(QtCore.Qt.UserRole)
+        filename = str(item.data(QtCore.Qt.UserRole))
         # Special handling if the filename is a optical clip
         if filename.startswith('optical:'):
             (name, title, audio_track, subtitle_track, start, end, clip_name) = parse_optical_path(filename)
@@ -259,11 +259,12 @@
         # TODO needs to be fixed as no idea why this fails
         # media.sort(key=lambda file_path: get_natural_key(file_path.name))
         for track in media:
-            track_info = QtCore.QFileInfo(track)
+            track_str = str(track)
+            track_info = QtCore.QFileInfo(track_str)
             item_name = None
-            if track.startswith('optical:'):
+            if track_str.startswith('optical:'):
                 # Handle optical based item
-                (file_name, title, audio_track, subtitle_track, start, end, clip_name) = parse_optical_path(track)
+                (file_name, title, audio_track, subtitle_track, start, end, clip_name) = parse_optical_path(track_str)
                 item_name = QtWidgets.QListWidgetItem(clip_name)
                 item_name.setIcon(UiIcons().optical)
                 item_name.setData(QtCore.Qt.UserRole, track)
@@ -272,22 +273,22 @@
                                                                    end=format_milliseconds(end)))
             elif not os.path.exists(track):
                 # File doesn't exist, mark as error.
-                file_name = os.path.split(str(track))[1]
+                file_name = os.path.split(track_str)[1]
                 item_name = QtWidgets.QListWidgetItem(file_name)
                 item_name.setIcon(UiIcons().error)
                 item_name.setData(QtCore.Qt.UserRole, track)
-                item_name.setToolTip(track)
+                item_name.setToolTip(track_str)
             elif track_info.isFile():
                 # Normal media file handling.
-                file_name = os.path.split(str(track))[1]
+                file_name = os.path.split(track_str)[1]
                 item_name = QtWidgets.QListWidgetItem(file_name)
                 search = file_name.split('.')[-1].lower()
-                if '*.{text}'.format(text=search) in self.media_controller.audio_extensions_list:
+                if search in AUDIO_EXT:
                     item_name.setIcon(UiIcons().audio)
                 else:
                     item_name.setIcon(UiIcons().video)
                 item_name.setData(QtCore.Qt.UserRole, track)
-                item_name.setToolTip(track)
+                item_name.setToolTip(track_str)
             if item_name:
                 self.list_view.addItem(item_name)
 
 
=== modified file 'openlp/plugins/songs/lib/songstab.py'
--- openlp/plugins/songs/lib/songstab.py	2019-04-13 13:00:22 +0000
+++ openlp/plugins/songs/lib/songstab.py	2019-06-09 20:23:18 +0000
@@ -88,7 +88,7 @@
         self.footer_group_box = QtWidgets.QGroupBox(self.left_column)
         self.footer_group_box.setObjectName('footer_group_box')
         self.footer_layout = QtWidgets.QVBoxLayout(self.footer_group_box)
-        self.footer_layout.setObjectName('chords_layout')
+        self.footer_layout.setObjectName('footer_layout')
         self.footer_info_label = QtWidgets.QLabel(self.footer_group_box)
         self.footer_layout.addWidget(self.footer_info_label)
         self.footer_placeholder_info = QtWidgets.QTextEdit(self.footer_group_box)
 
=== modified file 'tests/functional/openlp_core/ui/test_thememanager.py'
--- tests/functional/openlp_core/ui/test_thememanager.py	2019-05-22 06:47:00 +0000
+++ tests/functional/openlp_core/ui/test_thememanager.py	2019-06-09 20:23:18 +0000
@@ -83,7 +83,7 @@
 
     @patch('openlp.core.ui.thememanager.shutil')
     @patch('openlp.core.ui.thememanager.create_paths')
-    def test_write_theme_same_image(self, mocked_create_paths, mocked_shutil):
+    def test_save_theme_same_image(self, mocked_create_paths, mocked_shutil):
         """
         Test that we don't try to overwrite a theme background image with itself
         """
@@ -98,16 +98,16 @@
         mocked_theme.extract_formatted_xml = MagicMock()
         mocked_theme.extract_formatted_xml.return_value = 'fake_theme_xml'.encode()
 
-        # WHEN: Calling _write_theme with path to the same image, but the path written slightly different
+        # WHEN: Calling save_theme with path to the same image, but the path written slightly different
         file_path_1 = RESOURCE_PATH / 'church.jpg'
-        theme_manager._write_theme(mocked_theme, file_path_1, file_path_1)
+        theme_manager.save_theme(mocked_theme, file_path_1, file_path_1)
 
         # THEN: The mocked_copyfile should not have been called
         assert mocked_shutil.copyfile.called is False, 'copyfile should not be called'
 
     @patch('openlp.core.ui.thememanager.shutil')
     @patch('openlp.core.ui.thememanager.create_paths')
-    def test_write_theme_diff_images(self, mocked_create_paths, mocked_shutil):
+    def test_save_theme_diff_images(self, mocked_create_paths, mocked_shutil):
         """
         Test that we do overwrite a theme background image when a new is submitted
         """
@@ -121,15 +121,15 @@
         mocked_theme.theme_name = 'themename'
         mocked_theme.filename = "filename"
 
-        # WHEN: Calling _write_theme with path to different images
+        # WHEN: Calling save_theme with path to different images
         file_path_1 = RESOURCE_PATH / 'church.jpg'
         file_path_2 = RESOURCE_PATH / 'church2.jpg'
-        theme_manager._write_theme(mocked_theme, file_path_1, file_path_2)
+        theme_manager.save_theme(mocked_theme, file_path_1, file_path_2)
 
         # THEN: The mocked_copyfile should not have been called
         assert mocked_shutil.copyfile.called is True, 'copyfile should be called'
 
-    def test_write_theme_special_char_name(self):
+    def test_save_theme_special_char_name(self):
         """
         Test that we can save themes with special characters in the name
         """
@@ -142,8 +142,8 @@
         mocked_theme.theme_name = 'theme 愛 name'
         mocked_theme.export_theme.return_value = "{}"
 
-        # WHEN: Calling _write_theme with a theme with a name with special characters in it
-        theme_manager._write_theme(mocked_theme)
+        # WHEN: Calling save_theme with a theme with a name with special characters in it
+        theme_manager.save_theme(mocked_theme)
 
         # THEN: It should have been created
         assert os.path.exists(os.path.join(self.temp_folder, 'theme 愛 name', 'theme 愛 name.json')) is True, \
 
=== modified file 'tests/openlp_core/ui/test_themeform.py'
--- tests/openlp_core/ui/test_themeform.py	2019-04-13 13:00:22 +0000
+++ tests/openlp_core/ui/test_themeform.py	2019-06-09 20:23:18 +0000
@@ -23,6 +23,7 @@
 Interface tests to test the ThemeWizard class and related methods.
 """
 from unittest import TestCase
+from unittest.mock import patch
 
 from openlp.core.common.registry import Registry
 from openlp.core.ui.themeform import ThemeForm
@@ -39,7 +40,8 @@
         """
         Registry.create()
 
-    def test_create_theme_wizard(self):
+    @patch('openlp.core.display.window.QtWidgets.QVBoxLayout')
+    def test_create_theme_wizard(self, mocked_qvboxlayout):
         """
         Test creating a ThemeForm instance
         """