Closed jams2 closed 1 year ago
~@jams2 any chance you can rebase and run the linters (easiest is via pre-commit. but you could do it directly with black and ruff)~
edit: actually we need to keep this for v1.5.1
Merged in f03a3f21c5380e657f0b93007e0ee0ca791bddd9 + parents on the main branch, 623699704d54789bfe853c08acbed4e37bc0017a + parents in stable/1.5.x
and released version 1.5.1
Fixes #112
112 reports that a particular SVG is skewed when being cropped to its original dimensions by Wagtail. The SVG in question has the following relevant attributes:
The reason for the skewing is incorrect maths in Willow's transformation of an input bounding rect into the user (internal SVG) coordinate space. I'll go through the incorrect steps to show where the errors were.
This affects SVGs whose viewBox aspect ratio does not match the viewport aspect ratio.
The problem
1. Initial translation by viewBox offset
An "original" crop on this SVG has an input rect of:
input_rect := (0, 0, 1732.4, 800)
(left, top, right, bottom).
The old code applied an offset to the rect before applying a transformation into user space. This offset is the min-x and min-y of the viewBox, applied here. The resulting bounding rect is:
input_rect := (6600, 3500, 8332.4, 4300)
(the element-wise addition of (6600, 3500, 6600, 3500) to the initial rect).
We've now got a bounding rect that mixes values from the viewport space: the min-x and min-y values of 6600 and 3500, in the user space, have been added to the original rect, which is in the viewport space. This is the cause of the error. Apart from being incorrect, it's confusing and error-prone to have the code apply translation to the bounding rect in more than one place.
2. Calculating the transformation
Next we calculate a transformation to be applied to
input_rect
, here. The transformation is(translate_x, translate_y, scale_x, scale_y)
.We calculate the scaling factors along the x and y axes, ratios of viewport:viewBox.
The SVG has no explicit
preserveAspectRatio
value, so we treat it with the default value ofxMidYMid meet
. All values forpreserveAspectRatio
, other thannone
, require uniform scaling along the x and y axes. We take the minimum ofscale_x
andscale_y
, due to themeet
directive.Next, we determine the translation that is required to position the viewBox within the viewport, according to the
preserveAspectRatio
value. ForxMidYMid
, the formulae are:This gives us a final transformation of:
3. Applying the transformation
The transformation is applied here
The resulting of applying the transformation to our incorrect bounding rect
input_rect := (6600, 3500, 8332.4, 4300)
is:We translate this into a viewBox value (subtract left and top from right and bottom to yield width and height values):
The min-x and min-y values are incorrect, see the next section for the expected viewBox value.
4. Expected value
The expected viewBox is:
This can be confirmed by inspecting the following representative SVGs. The yellow area indicates the viewBox. The first image is the original:
Its top-level attributes match those on the example in the error report.
The next image is the same SVG, cropped by Willow to its original viewport dimensions:
Note that these two SVGs appear identical - the cropping is being applied correctly.
The solution
I've moved the calculation of the viewBox offset into the function that calculates the viewport to user space transform, here, and applied the scaling factors to them so that they are in the correct coordinate space (the viewport coordinate space).
I've also flipped the sign of the translation factors, as it is conventional to add a translation vector rather than to subtract one. This doesn't affect any calculations.
Testing
I've removed some tests, and added some tests that check actual SVG files rather than just doing the calculations. I believe this makes it easier to verify the correctness of the cropping procedures at a glance, and should make the process a little clearer for others working on this code.
The SVGs all display a yellow rectangle which covers the space of the viewBox exactly (although the entire viewBox will not always be visible). I have mapped the viewport into the SVG space with a blue outline. Where additional crops are to be applied, I have marked a further blue outline, to make it evident that the output SVG is correct.
To generate a HTML report that allows original SVGs to be compared with their expected results side by side, I have been using this script. It can be run in the project root. I will attach an up to date report (as .txt, as github won't allow HTML upload):
svg-report.txt
The SVG test files do not cover every possible combination of
preserveAspectRatio
and viewport to viewBox ratio, but the selection is, I believe, representative of the important cases and thorough enough to catch issues.I've also run a selection of SVGs through Wagtail with my branch installed, and behaviour is as expected:
SVG test.pdf