From 35d6c36aba492c9bd398fa4d5473d2a59450eae8 Mon Sep 17 00:00:00 2001 From: Fabian Meyer <3982806+meyfa@users.noreply.github.com> Date: Sun, 22 Jan 2023 15:25:36 +0100 Subject: [PATCH] fix: Allow renderer short-circuit if unable to render anything (#200) The text renderer would previously throw in some circumstances if no font was available. With this patch, its `prepareRenderParams()` method can return `null` in these cases, aborting the render for that text element without causing a crash. This also simplifies RectRenderer, which doesn't need an "empty" state anymore. --- .../Renderers/EllipseRenderer.php | 2 +- src/Rasterization/Renderers/LineRenderer.php | 2 +- .../Renderers/MultiPassRenderer.php | 17 +++++---- src/Rasterization/Renderers/PathRenderer.php | 2 +- .../Renderers/PolygonRenderer.php | 2 +- src/Rasterization/Renderers/RectRenderer.php | 13 +------ src/Rasterization/Renderers/TextRenderer.php | 6 ++- .../Renderers/TextRendererTest.php | 37 +++++++++++++++++++ 8 files changed, 58 insertions(+), 23 deletions(-) create mode 100644 tests/Rasterization/Renderers/TextRendererTest.php diff --git a/src/Rasterization/Renderers/EllipseRenderer.php b/src/Rasterization/Renderers/EllipseRenderer.php index 065d4b2..e96d06c 100644 --- a/src/Rasterization/Renderers/EllipseRenderer.php +++ b/src/Rasterization/Renderers/EllipseRenderer.php @@ -19,7 +19,7 @@ class EllipseRenderer extends MultiPassRenderer /** * @inheritdoc */ - protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): array + protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): ?array { $cx = $options['cx']; $cy = $options['cy']; diff --git a/src/Rasterization/Renderers/LineRenderer.php b/src/Rasterization/Renderers/LineRenderer.php index ba0e676..dcb4309 100644 --- a/src/Rasterization/Renderers/LineRenderer.php +++ b/src/Rasterization/Renderers/LineRenderer.php @@ -19,7 +19,7 @@ class LineRenderer extends MultiPassRenderer /** * @inheritdoc */ - protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): array + protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): ?array { $x1 = $options['x1']; $y1 = $options['y1']; diff --git a/src/Rasterization/Renderers/MultiPassRenderer.php b/src/Rasterization/Renderers/MultiPassRenderer.php index e0b94fb..8876f81 100644 --- a/src/Rasterization/Renderers/MultiPassRenderer.php +++ b/src/Rasterization/Renderers/MultiPassRenderer.php @@ -25,6 +25,9 @@ public function render(SVGRasterizer $rasterizer, array $options, SVGNode $conte $transform = $rasterizer->getCurrentTransform(); $params = $this->prepareRenderParams($options, $transform, $rasterizer->getFontRegistry()); + if (!isset($params)) { + return; + } $paintOrder = self::getPaintOrder($context); foreach ($paintOrder as $paint) { @@ -75,19 +78,19 @@ private function paintFill(SVGRasterizer $rasterizer, SVGNode $context, $params) } /** - * Converts the options array into a new parameters array that the render - * methods can make more sense of. + * Converts the options array into a new parameters array that the render methods can make more sense of. + * + * Specifically, the intention is to allow subclasses to outsource coordinate translation, approximation of curves + * and the like to this method rather than dealing with it in the render methods. This shall encourage single passes + * over the input data (for performance reasons). * - * Specifically, the intention is to allow subclasses to outsource - * coordinate translation, approximation of curves and the like to this - * method rather than dealing with it in the render methods. This shall - * encourage single passes over the input data (for performance reasons). + * If this method determines that rendering isn't possible (e.g. because the shape is empty), it shall return null. * * @param array $options The associative array of raw options. * @param Transform $transform The coordinate transform to apply, to go from user to output coordinates. * @param FontRegistry|null $fontRegistry The font registry to use for text rendering. * - * @return array The new associative array of computed render parameters. + * @return array|null The new associative array of computed render parameters, if there is something to render. */ abstract protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry); diff --git a/src/Rasterization/Renderers/PathRenderer.php b/src/Rasterization/Renderers/PathRenderer.php index 2e48d72..c9f5250 100644 --- a/src/Rasterization/Renderers/PathRenderer.php +++ b/src/Rasterization/Renderers/PathRenderer.php @@ -21,7 +21,7 @@ class PathRenderer extends MultiPassRenderer /** * @inheritdoc */ - protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): array + protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): ?array { $approximator = new PathApproximator($transform); $approximator->approximate($options['commands']); diff --git a/src/Rasterization/Renderers/PolygonRenderer.php b/src/Rasterization/Renderers/PolygonRenderer.php index 0e4e0a5..6bbb59e 100644 --- a/src/Rasterization/Renderers/PolygonRenderer.php +++ b/src/Rasterization/Renderers/PolygonRenderer.php @@ -19,7 +19,7 @@ class PolygonRenderer extends MultiPassRenderer /** * @inheritdoc */ - protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): array + protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): ?array { $points = []; foreach ($options['points'] as $point) { diff --git a/src/Rasterization/Renderers/RectRenderer.php b/src/Rasterization/Renderers/RectRenderer.php index f646c25..caaf943 100644 --- a/src/Rasterization/Renderers/RectRenderer.php +++ b/src/Rasterization/Renderers/RectRenderer.php @@ -21,14 +21,14 @@ class RectRenderer extends MultiPassRenderer /** * @inheritdoc */ - protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): array + protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): ?array { $w = $options['width']; $h = $options['height']; $transform->resize($w, $h); if ($w <= 0 || $h <= 0) { - return ['empty' => true]; + return null; } $x1 = $options['x']; @@ -54,7 +54,6 @@ protected function prepareRenderParams(array $options, Transform $transform, ?Fo } return [ - 'empty' => false, 'x1' => $x1, 'y1' => $y1, 'x2' => $x1 + $w - 1, @@ -69,10 +68,6 @@ protected function prepareRenderParams(array $options, Transform $transform, ?Fo */ protected function renderFill($image, $params, int $color): void { - if ($params['empty']) { - return; - } - if ($params['rx'] != 0 || $params['ry'] != 0) { $this->renderFillRounded($image, $params, $color); return; @@ -133,10 +128,6 @@ private function renderFillRounded($image, array $params, int $color): void */ protected function renderStroke($image, $params, int $color, float $strokeWidth): void { - if ($params['empty']) { - return; - } - imagesetthickness($image, round($strokeWidth)); if ($params['rx'] != 0 || $params['ry'] != 0) { diff --git a/src/Rasterization/Renderers/TextRenderer.php b/src/Rasterization/Renderers/TextRenderer.php index b605e5d..886f03b 100644 --- a/src/Rasterization/Renderers/TextRenderer.php +++ b/src/Rasterization/Renderers/TextRenderer.php @@ -23,7 +23,7 @@ class TextRenderer extends MultiPassRenderer /** * @inheritdoc */ - protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): array + protected function prepareRenderParams(array $options, Transform $transform, ?FontRegistry $fontRegistry): ?array { // this assumes there is no rotation or skew, but that's fine, we can't deal with that anyway $size1 = $options['fontSize']; @@ -41,6 +41,10 @@ protected function prepareRenderParams(array $options, Transform $transform, ?Fo } } + if (!isset($fontPath)) { + return null; + } + // text-anchor $anchorOffset = 0; if ($options['anchor'] === 'middle' || $options['anchor'] === 'end') { diff --git a/tests/Rasterization/Renderers/TextRendererTest.php b/tests/Rasterization/Renderers/TextRendererTest.php new file mode 100644 index 0000000..88a6ef6 --- /dev/null +++ b/tests/Rasterization/Renderers/TextRendererTest.php @@ -0,0 +1,37 @@ +getMockForAbstractClass('\SVG\Nodes\SVGNode'); + + $rasterizer = new SVGRasterizer('40px', '80px', null, 4, 8); + $obj->render($rasterizer, [ + 'x' => 10, + 'y' => 10, + 'fontFamily' => 'Roboto', + 'fontWeight' => 'normal', + 'fontStyle' => 'normal', + 'fontSize' => 16, + 'anchor' => 'middle', + 'text' => 'foo', + ], $context); + $img = $rasterizer->finish(); + + $this->assertThat($img, new GDSimilarityConstraint('./tests/images/empty-4x8.png')); + } +}