Fix cropping of SVGs

[jams2]

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:

• width :: 1732.4
• height :: 800
• viewBox :: 6600 3500 12750 8050

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.

scale_x := 0.135874509804 (1732.4 / 12750)
scale_y := 0.0993788819876 (800 / 8050)

The SVG has no explicit preserveAspectRatio value, so we treat it with the default value of xMidYMid meet. All values for preserveAspectRatio, other than none, require uniform scaling along the x and y axes. We take the minimum of scale_x and scale_y, due to the meet directive.

scale_x := scale_y := 0.0993788819876

Next, we determine the translation that is required to position the viewBox within the viewport, according to the preserveAspectRatio value. For xMidYMid, the formulae are:

translate_x = (viewport_width - viewBox_width * scale) / 2
translate_y = (viewport_height - viewBox_height * scale) / 2

translate_x := 232.65962733
translate_y := 0

This gives us a final transformation of:

(translate_x=232.65962733, translate_y=0, scale_x=0.0993788819876, scale_y=0.0993788819876)

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:

(64071.3625, 35218.75, 81503.6375, 43268.75)

We translate this into a viewBox value (subtract left and top from right and bottom to yield width and height values):

64071.3625 35218.75 17432.275 8050

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:

4258.862499999999 3499.9999999999995 17432.275 8050.0

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

[zerolab]

@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

[zerolab]

It took me a while to wrap my head around it, but the changes make sense and, imho, the resulting code is easier to follow 🚀

[zerolab]

Merged in f03a3f2 + parents on the main branch, 6236997 + parents in stable/1.5.x and released version 1.5.1