Skip to content

Commit

Permalink
Fixed behavior of config.background_opacity < 1 and duplicated log …
Browse files Browse the repository at this point in the history
…messages (#3680)

* Fixes #3614

Setting background_transparency=0.0 should have the same effect than
setting transparent=True. The output format must be compatible
with transparency (.mov, .webm). Otherwise ffpmeg fails.

This patch ensures that background_transparency=0.0 triggers the
change of the output format. Also a WARNING is raised about
the change in the output format.

I want to thank @SirJamesClarkMaxwell for inspiring me to tackle
this issue.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* test config.background_opacity < 1

* fix config behavior for config.background_opacity < 1

* fix logger messages being emitted twice

* actually cleanup usage of loggers, remove logger-imports; black

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* dry_run fixture

* remove debug print

* applied suggestion from review

* remove custom fixture from test_deprecation, use manim_caplog

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: Benjamin Hackl <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Jul 14, 2024
1 parent 205f746 commit f1ce512
Show file tree
Hide file tree
Showing 13 changed files with 121 additions and 92 deletions.
1 change: 1 addition & 0 deletions manim/_config/logger_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ def make_logger(
logger = logging.getLogger("manim")
logger.addHandler(rich_handler)
logger.setLevel(verbosity)
logger.propagate = False

if not (libav_logger := logging.getLogger()).hasHandlers():
libav_logger.addHandler(rich_handler)
Expand Down
22 changes: 16 additions & 6 deletions manim/_config/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@

__all__ = ["config_file_paths", "make_config_parser", "ManimConfig", "ManimFrame"]

logger = logging.getLogger("manim")


def config_file_paths() -> list[Path]:
"""The paths where ``.cfg`` files will be searched for.
Expand Down Expand Up @@ -799,7 +801,7 @@ def digest_args(self, args: argparse.Namespace) -> Self:
try:
self.upto_animation_number = nflag[1]
except Exception:
logging.getLogger("manim").info(
logger.info(
f"No end scene number specified in -n option. Rendering from {nflag[0]} onwards...",
)

Expand Down Expand Up @@ -1038,7 +1040,7 @@ def verbosity(self, val: str) -> None:
val,
["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"],
)
logging.getLogger("manim").setLevel(val)
logger.setLevel(val)

@property
def format(self) -> str:
Expand All @@ -1054,7 +1056,7 @@ def format(self, val: str) -> None:
)
self.resolve_movie_file_extension(self.transparent)
if self.format == "webm":
logging.getLogger("manim").warning(
logger.warning(
"Output format set as webm, this can be slower than other formats",
)

Expand Down Expand Up @@ -1272,6 +1274,8 @@ def background_opacity(self) -> float:
@background_opacity.setter
def background_opacity(self, value: float) -> None:
self._set_between("background_opacity", value, 0, 1)
if self.background_opacity < 1:
self.resolve_movie_file_extension(is_transparent=True)

@property
def frame_size(self) -> tuple[int, int]:
Expand Down Expand Up @@ -1306,8 +1310,8 @@ def quality(self, value: str | None) -> None:

@property
def transparent(self) -> bool:
"""Whether the background opacity is 0.0 (-t)."""
return self._d["background_opacity"] == 0.0
"""Whether the background opacity is less than 1.0 (-t)."""
return self._d["background_opacity"] < 1.0

@transparent.setter
def transparent(self, value: bool) -> None:
Expand Down Expand Up @@ -1425,6 +1429,7 @@ def window_size(self, value: str) -> None:
self._d.__setitem__("window_size", value)

def resolve_movie_file_extension(self, is_transparent: bool) -> None:
prev_file_extension = self.movie_file_extension
if is_transparent:
self.movie_file_extension = ".webm" if self.format == "webm" else ".mov"
elif self.format == "webm":
Expand All @@ -1433,6 +1438,11 @@ def resolve_movie_file_extension(self, is_transparent: bool) -> None:
self.movie_file_extension = ".mov"
else:
self.movie_file_extension = ".mp4"
if self.movie_file_extension != prev_file_extension:
logger.warning(
f"Output format changed to '{self.movie_file_extension}' "
"to support transparency",
)

@property
def enable_gui(self) -> bool:
Expand Down Expand Up @@ -1778,7 +1788,7 @@ def tex_template_file(self) -> Path:
def tex_template_file(self, val: str) -> None:
if val:
if not os.access(val, os.R_OK):
logging.getLogger("manim").warning(
logger.warning(
f"Custom TeX template {val} not found or not readable.",
)
else:
Expand Down
5 changes: 3 additions & 2 deletions manim/cli/render/global_options.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
from __future__ import annotations

import logging
import re

from cloup import Choice, option, option_group

from ... import logger

__all__ = ["global_options"]

logger = logging.getLogger("manim")


def validate_gui_location(ctx, param, value):
if value:
Expand Down
5 changes: 3 additions & 2 deletions manim/cli/render/render_options.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
from __future__ import annotations

import logging
import re

from cloup import Choice, option, option_group

from manim.constants import QUALITIES, RendererType

from ... import logger

__all__ = ["render_options"]

logger = logging.getLogger("manim")


def validate_scene_range(ctx, param, value):
try:
Expand Down
5 changes: 3 additions & 2 deletions manim/renderer/shader_wrapper.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
from __future__ import annotations

import copy
import logging
import re
from pathlib import Path

import moderngl
import numpy as np

from .. import logger

# Mobjects that should be rendered with
# the same shader will be organized and
# clumped together based on keeping track
Expand All @@ -17,6 +16,8 @@

__all__ = ["ShaderWrapper"]

logger = logging.getLogger("manim")


def get_shader_dir():
return Path(__file__).parent / "shaders"
Expand Down
3 changes: 2 additions & 1 deletion manim/utils/deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@


import inspect
import logging
import re
from collections.abc import Iterable
from typing import Any, Callable

from decorator import decorate, decorator

from .. import logger
logger = logging.getLogger("manim")


def _get_callable_info(callable: Callable) -> tuple[str, str]:
Expand Down
5 changes: 3 additions & 2 deletions manim/utils/testing/_frames_testers.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
from __future__ import annotations

import contextlib
import logging
import warnings
from pathlib import Path

import numpy as np

from manim import logger

from ._show_diff import show_diff_helper

FRAME_ABSOLUTE_TOLERANCE = 1.01
FRAME_MISMATCH_RATIO_TOLERANCE = 1e-5

logger = logging.getLogger("manim")


class _FramesTester:
def __init__(self, file_path: Path, show_diff=False) -> None:
Expand Down
10 changes: 10 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import logging
import sys
from pathlib import Path

Expand Down Expand Up @@ -45,6 +46,15 @@ def pytest_collection_modifyitems(config, items):
item.add_marker(slow_skip)


@pytest.fixture
def manim_caplog(caplog):
logger = logging.getLogger("manim")
logger.propagate = True
caplog.set_level(logging.INFO, logger="manim")
yield caplog
logger.propagate = False


@pytest.fixture
def config():
saved = manim.config.copy()
Expand Down
4 changes: 3 additions & 1 deletion tests/helpers/graphical_units.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@

from __future__ import annotations

import logging
import tempfile
from pathlib import Path

import numpy as np

from manim import logger
from manim.scene.scene import Scene

logger = logging.getLogger("manim")


def set_test_scene(scene_object: type[Scene], module_name: str, config):
"""Function used to set up the test data for a new feature. This will basically set up a pre-rendered frame for a scene. This is meant to be used only
Expand Down
8 changes: 4 additions & 4 deletions tests/module/animation/test_animation.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,20 @@ def test_animation_forbidden_run_time(run_time):
test_scene.play(FadeIn(None, run_time=run_time))


def test_animation_run_time_shorter_than_frame_rate(caplog, config):
def test_animation_run_time_shorter_than_frame_rate(manim_caplog, config):
test_scene = Scene()
test_scene.play(FadeIn(None, run_time=1 / (config.frame_rate + 1)))
assert (
"Original run time of FadeIn(Mobject) is shorter than current frame rate"
in caplog.text
in manim_caplog.text
)


@pytest.mark.parametrize("frozen_frame", [False, True])
def test_wait_run_time_shorter_than_frame_rate(caplog, frozen_frame):
def test_wait_run_time_shorter_than_frame_rate(manim_caplog, frozen_frame):
test_scene = Scene()
test_scene.wait(1e-9, frozen_frame=frozen_frame)
assert (
"Original run time of Wait(Mobject) is shorter than current frame rate"
in caplog.text
in manim_caplog.text
)
5 changes: 2 additions & 3 deletions tests/module/mobject/geometry/test_unit_geometry.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@ def test_get_arc_center():
)


def test_BackgroundRectangle(caplog):
caplog.set_level(logging.INFO)
def test_BackgroundRectangle(manim_caplog):
c = Circle()
bg = BackgroundRectangle(c)
bg.set_style(fill_opacity=0.42)
assert bg.get_fill_opacity() == 0.42
bg.set_style(fill_opacity=1, hello="world")
assert (
"Argument {'hello': 'world'} is ignored in BackgroundRectangle.set_style."
in caplog.text
in manim_caplog.text
)
Loading

0 comments on commit f1ce512

Please sign in to comment.