From 10142f5bb4b5fde87ccbfd65fe97a0aba0a4d8c0 Mon Sep 17 00:00:00 2001 From: nyanpasu64 Date: Mon, 8 Apr 2019 05:23:01 -0700 Subject: [PATCH] Fix label scaling, rewrite resolution divisor (#264) - Alter DPI (pixel/inch) instead of scaling each UI element individually - Fix matplotlib output dimension mismatches with nonstandard DPIs (which may not have occurred at 96dpi). --- corrscope/outputs.py | 6 ++-- corrscope/renderer.py | 70 +++++++++++++++++++++++++++--------------- tests/test_output.py | 6 ++-- tests/test_renderer.py | 45 +++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 29 deletions(-) diff --git a/corrscope/outputs.py b/corrscope/outputs.py index fa8958b..21f6688 100644 --- a/corrscope/outputs.py +++ b/corrscope/outputs.py @@ -44,7 +44,7 @@ class Output(ABC): rcfg = corr_cfg.render - frame_bytes = rcfg.height * rcfg.width * BYTES_PER_PIXEL + frame_bytes = rcfg.divided_height * rcfg.divided_width * BYTES_PER_PIXEL self.bufsize = frame_bytes * FRAMES_TO_BUFFER def __enter__(self): @@ -116,8 +116,8 @@ class _FFmpegProcess: def ffmpeg_input_video(cfg: "Config") -> List[str]: fps = cfg.render_fps - width = cfg.render.width - height = cfg.render.height + width = cfg.render.divided_width + height = cfg.render.divided_height return [ f"-f rawvideo -pixel_format {PIXEL_FORMAT} -video_size {width}x{height}", diff --git a/corrscope/renderer.py b/corrscope/renderer.py index ae5dd42..771a8a4 100644 --- a/corrscope/renderer.py +++ b/corrscope/renderer.py @@ -115,6 +115,14 @@ class RendererConfig(DumpableAttrs, always_dump="*"): height: int line_width: float = with_units("px", default=1.5) + @property + def divided_width(self): + return round(self.width / self.res_divisor) + + @property + def divided_height(self): + return round(self.height / self.res_divisor) + bg_color: str = "#000000" init_line_color: str = default_color() @@ -148,14 +156,12 @@ class RendererConfig(DumpableAttrs, always_dump="*"): assert isinstance(self.height, (int, float)) def before_preview(self) -> None: - """ Called *once* before preview. Decreases render resolution/etc. """ - self.width = round(self.width / self.res_divisor) - self.height = round(self.height / self.res_divisor) - self.line_width /= self.res_divisor + """ Called *once* before preview. Does nothing. """ + pass def before_record(self) -> None: - """ Called *once* before recording video. Does nothing yet. """ - pass + """ Called *once* before recording video. Eliminates res_divisor. """ + self.res_divisor = 1 @attr.dataclass @@ -177,12 +183,13 @@ class Renderer(ABC): self.cfg = cfg self.lcfg = lcfg - self.w = cfg.width - self.h = cfg.height + self.w = cfg.divided_width + self.h = cfg.divided_height self.nplots = len(dummy_datas) - assert len(dummy_datas[0].shape) == 2, dummy_datas[0].shape + if self.nplots > 0: + assert len(dummy_datas[0].shape) == 2, dummy_datas[0].shape self.wave_nsamps = [data.shape[0] for data in dummy_datas] self.wave_nchans = [data.shape[1] for data in dummy_datas] @@ -223,14 +230,12 @@ class Renderer(ABC): Point = float -px_inch = 96 -pt_inch = 72 - -DPI = px_inch +PX_INCH = 96 +POINT_INCH = 72 def pixels(px: float) -> Point: - return px / px_inch * pt_inch + return px / PX_INCH * POINT_INCH class MatplotlibRenderer(Renderer): @@ -291,10 +296,31 @@ class MatplotlibRenderer(Renderer): cfg = self.cfg self._fig = Figure() - self._fig.set_dpi(DPI) - self._fig.set_size_inches(self.cfg.width / DPI, self.cfg.height / DPI) FigureCanvasAgg(self._fig) + px_inch = PX_INCH / cfg.res_divisor + self._fig.set_dpi(px_inch) + + """ + Requirements: + - px_inch /= res_divisor (to scale visual elements correctly) + - int(set_size_inches * px_inch) == self.w,h + - matplotlib uses int instead of round. Who knows why. + - round(set_size_inches * px_inch) == self.w,h + - just in case matplotlib changes its mind + + Solution: + - (set_size_inches * px_inch) == self.w,h + 0.25 + - set_size_inches == (self.w,h + 0.25) / px_inch + """ + offset = 0.25 + self._fig.set_size_inches( + (self.w + offset) / px_inch, (self.h + offset) / px_inch + ) + + real_dims = self._fig.canvas.get_width_height() + assert (self.w, self.h) == real_dims, [(self.w, self.h), real_dims] + # Setup background self._fig.set_facecolor(cfg.bg_color) @@ -505,7 +531,7 @@ class MatplotlibRenderer(Renderer): ) pos_axes = (xpos.pos_axes, ypos.pos_axes) - offset_px = (xpos.offset_px, ypos.offset_px) + offset_pt = (pixels(xpos.offset_px), pixels(ypos.offset_px)) out: List["Text"] = [] for label_text, ax in zip(labels, self._axes_mono): @@ -516,8 +542,8 @@ class MatplotlibRenderer(Renderer): # Positioning xy=pos_axes, xycoords="axes fraction", - xytext=offset_px, - textcoords="offset pixels", + xytext=offset_pt, + textcoords="offset points", horizontalalignment=xpos.align, verticalalignment=ypos.align, # Cosmetics @@ -546,12 +572,8 @@ class MatplotlibRenderer(Renderer): f"oh shit, cannot read data from {type(canvas)} != FigureCanvasAgg" ) - w = self.cfg.width - h = self.cfg.height - assert (w, h) == canvas.get_width_height() - buffer_rgb = canvas.tostring_rgb() - assert len(buffer_rgb) == w * h * BYTES_PER_PIXEL + assert len(buffer_rgb) == self.w * self.h * BYTES_PER_PIXEL return buffer_rgb diff --git a/tests/test_output.py b/tests/test_output.py index fea9480..9165dce 100644 --- a/tests/test_output.py +++ b/tests/test_output.py @@ -339,6 +339,8 @@ def test_preview_performance(Popen, mocker: "pytest_mock.MockFixture", outputs): cfg = cfg_192x108() corr = CorrScope(cfg, Arguments(".", outputs)) + + # Run corrscope main loop. corr.play() # Check that only before_preview() called. @@ -348,8 +350,8 @@ def test_preview_performance(Popen, mocker: "pytest_mock.MockFixture", outputs): r.assert_not_called() # Check renderer is 128x72 - assert corr.renderer.cfg.width == 128 - assert corr.renderer.cfg.height == 72 + assert corr.renderer.w == 128 + assert corr.renderer.h == 72 # Ensure subfps is enabled (only odd frames are rendered, 1..29). # See CorrScope `should_render` variable. diff --git a/tests/test_renderer.py b/tests/test_renderer.py index 4a769df..618887a 100644 --- a/tests/test_renderer.py +++ b/tests/test_renderer.py @@ -1,14 +1,17 @@ from typing import Optional, TYPE_CHECKING, List +import hypothesis.strategies as hs import matplotlib.colors import numpy as np import pytest +from hypothesis import given from corrscope.channel import ChannelConfig from corrscope.corrscope import CorrScope, default_config, Arguments from corrscope.layout import LayoutConfig from corrscope.outputs import BYTES_PER_PIXEL, FFplayOutputConfig from corrscope.renderer import RendererConfig, MatplotlibRenderer, LabelPosition, Font +from corrscope.util import perr from corrscope.wave import Flatten if TYPE_CHECKING: @@ -233,3 +236,45 @@ def test_stereo_render_integration(mocker: "pytest_mock.MockFixture"): # Make sure it doesn't crash. corr = CorrScope(cfg, Arguments(".", [FFplayOutputConfig()])) corr.play() + + +@pytest.mark.parametrize( + "target_int, res_divisor", [(50, 2.0), (51, 2.0), (100, 1.001)] +) +def test_res_divisor_rounding_fixed(target_int: int, res_divisor: float): + verify_res_divisor_rounding(target_int, res_divisor, speed_hack=False) + + +@given(target_int=hs.integers(1, 10000), res_divisor=hs.floats(1, 100)) +def test_res_divisor_rounding_hypothesis(target_int: int, res_divisor: float, mocker): + verify_res_divisor_rounding(target_int, res_divisor, speed_hack=True, mocker=mocker) + + +def verify_res_divisor_rounding( + target_int: int, + res_divisor: float, + speed_hack: bool, + mocker: "pytest_mock.MockFixture" = None, +): + """Ensure that pathological-case float rounding errors + don't cause inconsistent dimensions and assertion errors.""" + target_dim = target_int + 0.5 + undivided_dim = round(target_dim * res_divisor) + + cfg = RendererConfig(undivided_dim, undivided_dim, res_divisor=res_divisor) + cfg.before_preview() + + if speed_hack: + mocker.patch.object(MatplotlibRenderer, "_save_background") + datas = [] + else: + datas = [RENDER_Y_ZEROS] + + try: + renderer = MatplotlibRenderer(cfg, LayoutConfig(), datas, channel_cfgs=None) + if not speed_hack: + renderer.update_main_lines(datas) + renderer.get_frame() + except Exception: + perr(cfg.divided_width) + raise