feat(cli): auto detect resolution (#158)

* feat(cli): auto detect resolution

The `present` command will now read by default the resolution of each presentation, and only change it if specified by the user.

This PR also fixes bugs introduced by #156 and previous PRs, where the transition between two presentation was not correct...

* fix(lib): better to test if not None

Author: jeertmans

manim_slides/config.py

@@ -5,9 +5,9 @@
 import tempfile
 from enum import Enum
 from pathlib import Path
-from typing import Dict, List, Optional, Set, Union
+from typing import Dict, List, Optional, Set, Tuple, Union
 
-from pydantic import BaseModel, FilePath, root_validator, validator
+from pydantic import BaseModel, FilePath, PositiveInt, root_validator, validator
 from PySide6.QtCore import Qt
 
 from .defaults import FFMPEG_BIN
@@ -20,7 +20,7 @@ def merge_basenames(files: List[FilePath]) -> Path:
     """
     logger.info(f"Generating a new filename for animations: {files}")
 
-    dirname = files[0].parent
+    dirname: Path = files[0].parent
     ext = files[0].suffix
 
     basenames = (file.stem for file in files)
@@ -31,7 +31,7 @@ def merge_basenames(files: List[FilePath]) -> Path:
     # https://github.com/jeertmans/manim-slides/issues/123
     basename = hashlib.sha256(basenames_str.encode()).hexdigest()
 
-    return dirname / (basename + ext)
+    return dirname.joinpath(basename + ext)
 
 
 class Key(BaseModel):  # type: ignore
@@ -149,21 +149,22 @@ def slides_slice(self) -> slice:
 class PresentationConfig(BaseModel):  # type: ignore
     slides: List[SlideConfig]
     files: List[FilePath]
+    resolution: Tuple[PositiveInt, PositiveInt] = (1920, 1080)
 
     @root_validator
     def animation_indices_match_files(
         cls, values: Dict[str, Union[List[SlideConfig], List[FilePath]]]
     ) -> Dict[str, Union[List[SlideConfig], List[FilePath]]]:
-        files = values.get("files")
-        slides = values.get("slides")
+        files: List[FilePath] = values.get("files")  # type: ignore
+        slides: List[SlideConfig] = values.get("slides")  # type: ignore
 
         if files is None or slides is None:
             return values
 
         n_files = len(files)
 
         for slide in slides:
-            if slide.end_animation > n_files:  # type: ignore
+            if slide.end_animation > n_files:
                 raise ValueError(
                     f"The following slide's contains animations not listed in files {files}: {slide}"
                 )

manim_slides/convert.py

@@ -24,7 +24,7 @@
 from .present import get_scenes_presentation_config
 
 
-def open_with_default(file: Path):
+def open_with_default(file: Path) -> None:
     system = platform.system()
     if system == "Darwin":
         subprocess.call(("open", str(file)))
@@ -66,7 +66,7 @@ def load_template(self) -> str:
         An empty string is returned if no template is used."""
         return ""
 
-    def open(self, file: Path) -> bool:
+    def open(self, file: Path) -> Any:
         """Opens a file, generated with converter, using appropriate application."""
         raise NotImplementedError
 
@@ -376,7 +376,7 @@ class Config:
         use_enum_values = True
         extra = "forbid"
 
-    def open(self, file: Path) -> bool:
+    def open(self, file: Path) -> None:
         return open_with_default(file)
 
     def convert_to(self, dest: Path) -> None:
@@ -389,7 +389,9 @@ def convert_to(self, dest: Path) -> None:
 
         # From GitHub issue comment:
         # - https://github.com/scanny/python-pptx/issues/427#issuecomment-856724440
-        def auto_play_media(media: pptx.shapes.picture.Movie, loop: bool = False):
+        def auto_play_media(
+            media: pptx.shapes.picture.Movie, loop: bool = False
+        ) -> None:
             el_id = xpath(media.element, ".//p:cNvPr")[0].attrib["id"]
             el_cnt = xpath(
                 media.element.getparent().getparent().getparent(),
@@ -463,7 +465,7 @@ def callback(ctx: Context, param: Parameter, value: bool) -> None:
 
         ctx.exit()
 
-    return click.option(
+    return click.option(  # type: ignore
         "--show-config",
         is_flag=True,
         help="Show supported options for given format and exit.",
@@ -491,7 +493,7 @@ def callback(ctx: Context, param: Parameter, value: bool) -> None:
 
         ctx.exit()
 
-    return click.option(
+    return click.option(  # type: ignore
         "--show-template",
         is_flag=True,
         help="Show the template (currently) used for a given conversion format and exit.",

manim_slides/present.py

@@ -9,6 +9,7 @@
 import click
 import cv2
 import numpy as np
+from click import Context, Parameter
 from pydantic import ValidationError
 from PySide6.QtCore import Qt, QThread, Signal, Slot
 from PySide6.QtGui import QCloseEvent, QIcon, QImage, QKeyEvent, QPixmap, QResizeEvent
@@ -70,8 +71,7 @@ class Presentation:
     """Creates presentation from a configuration object."""
 
     def __init__(self, config: PresentationConfig) -> None:
-        self.slides: List[SlideConfig] = config.slides
-        self.files: List[str] = config.files
+        self.config = config
 
         self.__current_slide_index: int = 0
         self.current_animation: int = self.current_slide.start_animation
@@ -90,23 +90,41 @@ def __init__(self, config: PresentationConfig) -> None:
     def __len__(self) -> int:
         return len(self.slides)
 
+    @property
+    def slides(self) -> List[SlideConfig]:
+        """Returns the list of slides."""
+        return self.config.slides
+
+    @property
+    def files(self) -> List[Path]:
+        """Returns the list of animation files."""
+        return self.config.files
+
+    @property
+    def resolution(self) -> Tuple[int, int]:
+        """Returns the resolution."""
+        return self.config.resolution
+
     @property
     def current_slide_index(self) -> int:
         return self.__current_slide_index
 
     @current_slide_index.setter
-    def current_slide_index(self, value: Optional[int]):
-        if value:
+    def current_slide_index(self, value: Optional[int]) -> None:
+        if value is not None:
             if -len(self) <= value < len(self):
                 self.__current_slide_index = value
                 self.current_animation = self.current_slide.start_animation
+                logger.debug(f"Set current slide index to {value}")
             else:
                 logger.error(
                     f"Could not load slide number {value}, playing first slide instead."
                 )
 
-    def set_current_animation_and_update_slide_number(self, value: Optional[int]):
-        if value:
+    def set_current_animation_and_update_slide_number(
+        self, value: Optional[int]
+    ) -> None:
+        if value is not None:
             n_files = len(self.files)
             if -n_files <= value < n_files:
                 if value < 0:
@@ -116,6 +134,8 @@ def set_current_animation_and_update_slide_number(self, value: Optional[int]):
                     if value < slide.end_animation:
                         self.current_slide_index = i
                         self.current_animation = value
+
+                        logger.debug(f"Playing animation {value}, at slide index {i}")
                         return
 
                 assert (
@@ -159,7 +179,7 @@ def load_animation_cap(self, animation: int) -> None:
 
             self.release_cap()
 
-            file: str = self.files[animation]
+            file: str = str(self.files[animation])
 
             if self.reverse:
                 file = "{}_reversed{}".format(*os.path.splitext(file))
@@ -178,7 +198,7 @@ def current_cap(self) -> cv2.VideoCapture:
 
     def rewind_current_slide(self) -> None:
         """Rewinds current slide to first frame."""
-        logger.debug("Rewinding curring slide")
+        logger.debug("Rewinding current slide")
         if self.reverse:
             self.current_animation = self.current_slide.end_animation - 1
         else:
@@ -216,9 +236,10 @@ def load_next_slide(self) -> None:
 
     def load_previous_slide(self) -> None:
         """Loads previous slide."""
-        logger.debug("Loading previous slide")
+        logger.debug(f"Loading previous slide, current is {self.current_slide_index}")
         self.cancel_reverse()
         self.current_slide_index = max(0, self.current_slide_index - 1)
+        logger.debug(f"Loading slide index {self.current_slide_index}")
         self.rewind_current_slide()
 
     @property
@@ -229,7 +250,8 @@ def fps(self) -> int:
             logger.warn(
                 f"Something is wrong with video file {self.current_file}, as the fps returned by frame {self.current_frame_number} is 0"
             )
-        return max(fps, 1)  # TODO: understand why we sometimes get 0 fps
+        # TODO: understand why we sometimes get 0 fps
+        return max(fps, 1)  # type: ignore
 
     def reset(self) -> None:
         """Rests current presentation."""
@@ -320,6 +342,7 @@ class Display(QThread):  # type: ignore
 
     change_video_signal = Signal(np.ndarray)
     change_info_signal = Signal(dict)
+    change_presentation_sigal = Signal()
     finished = Signal()
 
     def __init__(
@@ -365,10 +388,12 @@ def current_presentation_index(self) -> int:
         return self.__current_presentation_index
 
     @current_presentation_index.setter
-    def current_presentation_index(self, value: Optional[int]):
-        if value:
+    def current_presentation_index(self, value: Optional[int]) -> None:
+        if value is not None:
             if -len(self) <= value < len(self):
                 self.__current_presentation_index = value
+                self.current_presentation.release_cap()
+                self.change_presentation_sigal.emit()
             else:
                 logger.error(
                     f"Could not load scene number {value}, playing first scene instead."
@@ -379,6 +404,11 @@ def current_presentation(self) -> Presentation:
         """Returns the current presentation."""
         return self.presentations[self.current_presentation_index]
 
+    @property
+    def current_resolution(self) -> Tuple[int, int]:
+        """Returns the resolution of the current presentation."""
+        return self.current_presentation.resolution
+
     def run(self) -> None:
         """Runs a series of presentations until end or exit."""
         while self.run_flag:
@@ -413,7 +443,7 @@ def run(self) -> None:
         if self.record_to is not None:
             self.record_movie()
 
-        logger.debug("Closing video thread gracully and exiting")
+        logger.debug("Closing video thread gracefully and exiting")
         self.finished.emit()
 
     def record_movie(self) -> None:
@@ -587,7 +617,6 @@ def __init__(
         *args: Any,
         config: Config = DEFAULT_CONFIG,
         fullscreen: bool = False,
-        resolution: Tuple[int, int] = (1980, 1080),
         hide_mouse: bool = False,
         aspect_ratio: AspectRatio = AspectRatio.auto,
         resize_mode: Qt.TransformationMode = Qt.SmoothTransformation,
@@ -599,7 +628,12 @@ def __init__(
         self.setWindowTitle(WINDOW_NAME)
         self.icon = QIcon(":/icon.png")
         self.setWindowIcon(self.icon)
-        self.display_width, self.display_height = resolution
+
+        # create the video capture thread
+        kwargs["config"] = config
+        self.thread = Display(*args, **kwargs)
+
+        self.display_width, self.display_height = self.thread.current_resolution
         self.aspect_ratio = aspect_ratio
         self.resize_mode = resize_mode
         self.hide_mouse = hide_mouse
@@ -619,9 +653,6 @@ def __init__(
         self.label.setPixmap(self.pixmap)
         self.label.setMinimumSize(1, 1)
 
-        # create the video capture thread
-        kwargs["config"] = config
-        self.thread = Display(*args, **kwargs)
         # create the info dialog
         self.info = Info()
         self.info.show()
@@ -635,6 +666,7 @@ def __init__(
         # connect signals
         self.thread.change_video_signal.connect(self.update_image)
         self.thread.change_info_signal.connect(self.info.update_info)
+        self.thread.change_presentation_sigal.connect(self.update_canvas)
         self.thread.finished.connect(self.closeAll)
         self.send_key_signal.connect(self.thread.set_key)
 
@@ -688,6 +720,14 @@ def update_image(self, cv_img: np.ndarray) -> None:
 
         self.label.setPixmap(QPixmap.fromImage(qt_img))
 
+    @Slot()
+    def update_canvas(self) -> None:
+        """Update the canvas when a presentation has changed."""
+        logger.debug("Updating canvas")
+        self.display_width, self.display_height = self.thread.current_resolution
+        if not self.isFullScreen():
+            self.resize(self.display_width, self.display_height)
+
 
 @click.command()
 @click.option(
@@ -757,7 +797,7 @@ def value_proc(value: Optional[str]) -> List[str]:
     while True:
         try:
             scenes = click.prompt("Choice(s)", value_proc=value_proc)
-            return scenes
+            return scenes  # type: ignore
         except ValueError as e:
             raise click.UsageError(str(e))
 
@@ -785,7 +825,9 @@ def get_scenes_presentation_config(
     return presentation_configs
 
 
-def start_at_callback(ctx, param, values: str) -> Tuple[Optional[int], ...]:
+def start_at_callback(
+    ctx: Context, param: Parameter, values: str
+) -> Tuple[Optional[int], ...]:
     if values == "(None, None, None)":
         return (None, None, None)
 
@@ -838,9 +880,8 @@ def str_to_int_or_none(value: str) -> Optional[int]:
     "--resolution",
     metavar="<WIDTH HEIGHT>",
     type=(int, int),
-    default=(1920, 1080),
+    default=None,
     help="Window resolution WIDTH HEIGHT used if fullscreen is not set. You may manually resize the window afterward.",
-    show_default=True,
 )
 @click.option(
     "--to",
@@ -931,7 +972,7 @@ def present(
     start_paused: bool,
     fullscreen: bool,
     skip_all: bool,
-    resolution: Tuple[int, int],
+    resolution: Optional[Tuple[int, int]],
     record_to: Optional[Path],
     exit_after_last_slide: bool,
     hide_mouse: bool,
@@ -956,9 +997,15 @@ def present(
     if skip_all:
         exit_after_last_slide = True
 
+    presentation_configs = get_scenes_presentation_config(scenes, folder)
+
+    if resolution is not None:
+        for presentation_config in presentation_configs:
+            presentation_config.resolution = resolution
+
     presentations = [
         Presentation(presentation_config)
-        for presentation_config in get_scenes_presentation_config(scenes, folder)
+        for presentation_config in presentation_configs
     ]
 
     if config_path.exists():
@@ -994,7 +1041,6 @@ def present(
         start_paused=start_paused,
         fullscreen=fullscreen,
         skip_all=skip_all,
-        resolution=resolution,
         record_to=record_to,
         exit_after_last_slide=exit_after_last_slide,
         hide_mouse=hide_mouse,