Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): smarter info window hiding logic #482

Merged
merged 12 commits into from
Dec 11, 2024
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
(unreleased)=
## [Unreleased](https://github.com/jeertmans/manim-slides/compare/v5.1.10...HEAD)

(unreleased-changed)=
### Changed

- The info window is now only shown in presentations when there
are multiple monitors. However, the `--show-info-window` option
was added to `manim-slides present` to force the info window.
When there are multiple monitors, the info window will no longer
be on the same monitor as the main window, unless overridden.
[@PeculiarProgrammer](https://github.com/PeculiarProgrammer)
[#482](https://github.com/jeertmans/manim-slides/pull/482)

(v5.1.10)=
## [v5.1.10](https://github.com/jeertmans/manim-slides/compare/v5.1.9...v5.1.10)

Expand Down
44 changes: 33 additions & 11 deletions manim_slides/present/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import signal
import sys
from pathlib import Path
from typing import Optional
from typing import Literal, Optional

import click
from click import Context, Parameter
Expand Down Expand Up @@ -222,20 +222,28 @@
)
@click.option(
"--hide-info-window",
is_flag=True,
help="Hide info window.",
flag_value="always",
help="Hide info window. By default, hide the info window if there is only one screen.",
)
@click.option(
"--show-info-window",
"hide_info_window",
flag_value="never",
help="Force to show info window.",
)
@click.option(
"--info-window-screen",
"info_window_screen_number",
metavar="NUMBER",
type=int,
default=None,
help="Put info window on the given screen (a.k.a. display).",
help="Put info window on the given screen (a.k.a. display). "
"If there is more than one screen, it will by default put the info window "
"on a different screen than the main player.",
)
@click.help_option("-h", "--help")
@verbosity_option
def present(
def present( # noqa: C901
scenes: list[str],
config_path: Path,
folder: Path,
Expand All @@ -251,7 +259,7 @@
screen_number: Optional[int],
playback_rate: float,
next_terminates_loop: bool,
hide_info_window: bool,
hide_info_window: Optional[Literal["always", "never"]],
info_window_screen_number: Optional[int],
) -> None:
"""
Expand Down Expand Up @@ -294,22 +302,36 @@
app = qapp()
app.setApplicationName("Manim Slides")

screens = app.screens()

def get_screen(number: int) -> Optional[QScreen]:
try:
return app.screens()[number]
return screens[number]
except IndexError:
logger.error(
f"Invalid screen number {number}, "
f"allowed values are from 0 to {len(app.screens())-1} (incl.)"
f"allowed values are from 0 to {len(screens)-1} (incl.)"
)
return None

should_hide_info_window = False

if hide_info_window is None:
should_hide_info_window = len(screens) == 1
elif hide_info_window == "always":
should_hide_info_window = True

Check warning on line 322 in manim_slides/present/__init__.py

View check run for this annotation

Codecov / codecov/patch

manim_slides/present/__init__.py#L321-L322

Added lines #L321 - L322 were not covered by tests

if should_hide_info_window and info_window_screen_number is not None:
logger.warning(

Check warning on line 325 in manim_slides/present/__init__.py

View check run for this annotation

Codecov / codecov/patch

manim_slides/present/__init__.py#L325

Added line #L325 was not covered by tests
f"Ignoring `--info-window-screen` because `--hide-info-window` is set to `{hide_info_window}`."
)

if screen_number is not None:
screen = get_screen(screen_number)
else:
screen = None

if info_window_screen_number is not None:
if info_window_screen_number is not None and not should_hide_info_window:
info_window_screen = get_screen(info_window_screen_number)
else:
info_window_screen = None
Expand All @@ -333,11 +355,11 @@
screen=screen,
playback_rate=playback_rate,
next_terminates_loop=next_terminates_loop,
hide_info_window=hide_info_window,
hide_info_window=should_hide_info_window,
info_window_screen=info_window_screen,
)

player.show()
player.show(screens)

signal.signal(signal.SIGINT, signal.SIG_DFL)
sys.exit(app.exec())
28 changes: 21 additions & 7 deletions manim_slides/present/player.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
def __init__(
self,
*,
full_screen: bool,
aspect_ratio_mode: Qt.AspectRatioMode,
screen: Optional[QScreen],
) -> None:
Expand All @@ -38,9 +37,6 @@
self.setScreen(screen)
self.move(screen.geometry().topLeft())

if full_screen:
self.setWindowState(Qt.WindowFullScreen)

layout = QHBoxLayout()

# Current slide view
Expand Down Expand Up @@ -243,7 +239,6 @@
self.slide_changed.connect(self.slide_changed_callback)

self.info = Info(
full_screen=full_screen,
aspect_ratio_mode=aspect_ratio_mode,
screen=info_window_screen,
)
Expand Down Expand Up @@ -484,11 +479,28 @@
self.info.next_media_player.setSource(url)
self.info.next_media_player.play()

def show(self) -> None:
def show(self, screens: list[QScreen]) -> None:
"""Screens is necessary to prevent the info window from being shown on the same screen as the main window (especially in full screen mode)."""
super().show()

if not self.hide_info_window:
self.info.show()
if len(screens) > 1 and self.isFullScreen():
self.ensure_different_screens(screens)

Check warning on line 488 in manim_slides/present/player.py

View check run for this annotation

Codecov / codecov/patch

manim_slides/present/player.py#L487-L488

Added lines #L487 - L488 were not covered by tests

if self.isFullScreen():
self.info.showFullScreen()

Check warning on line 491 in manim_slides/present/player.py

View check run for this annotation

Codecov / codecov/patch

manim_slides/present/player.py#L490-L491

Added lines #L490 - L491 were not covered by tests
else:
self.info.show()

Check warning on line 493 in manim_slides/present/player.py

View check run for this annotation

Codecov / codecov/patch

manim_slides/present/player.py#L493

Added line #L493 was not covered by tests

if (

Check warning on line 495 in manim_slides/present/player.py

View check run for this annotation

Codecov / codecov/patch

manim_slides/present/player.py#L495

Added line #L495 was not covered by tests
len(screens) > 1 and self.info.screen() == self.screen()
): # It is better when Qt assigns the location, but if it fails to, this is a fallback
self.ensure_different_screens(screens)

Check warning on line 498 in manim_slides/present/player.py

View check run for this annotation

Codecov / codecov/patch

manim_slides/present/player.py#L498

Added line #L498 was not covered by tests

def ensure_different_screens(self, screens: list[QScreen]) -> None:
target_screen = screens[1] if self.screen() == screens[0] else screens[0]
self.info.setScreen(target_screen)
self.info.move(target_screen.geometry().topLeft())

Check warning on line 503 in manim_slides/present/player.py

View check run for this annotation

Codecov / codecov/patch

manim_slides/present/player.py#L501-L503

Added lines #L501 - L503 were not covered by tests

@Slot()
def close(self) -> None:
Expand Down Expand Up @@ -538,8 +550,10 @@
def full_screen(self) -> None:
if self.windowState() == Qt.WindowFullScreen:
self.setWindowState(Qt.WindowNoState)
self.info.setWindowState(Qt.WindowNoState)

Check warning on line 553 in manim_slides/present/player.py

View check run for this annotation

Codecov / codecov/patch

manim_slides/present/player.py#L553

Added line #L553 was not covered by tests
else:
self.setWindowState(Qt.WindowFullScreen)
self.info.setWindowState(Qt.WindowFullScreen)

Check warning on line 556 in manim_slides/present/player.py

View check run for this annotation

Codecov / codecov/patch

manim_slides/present/player.py#L556

Added line #L556 was not covered by tests

@Slot()
def hide_mouse(self) -> None:
Expand Down
6 changes: 3 additions & 3 deletions uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading