From 9a96740463dbef17693486014296f4d75637c307 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 16 Oct 2026 02:23:18 +0200 Subject: [PATCH] Crop the centre before resizing for fit=cover (closes #282) For fit=cover, the default fit, libvips' thumbnail scaled the whole source to cover the requested size and copied that into memory before cutting out the centre, so an 8192x64 source at 8192x8192 asked for about 24 GiB, and 8192x1 failed with 500. resize now first cuts the centre of the source, widened to whole source pixels, and gives only that to thumbnail, which scales both sides by the same amount as before; the image it holds exceeds the output by less than one scaled source pixel on each side. Output size and scale are unchanged; its position can move by up to one output pixel. Model: opus-5-5 --- TODO.md | 12 + .../imageprocessor/fit_cover_internal_test.go | 268 ++++++++++++++++++ internal/imageprocessor/imageprocessor.go | 36 ++- 3 files changed, 314 insertions(+), 2 deletions(-) create mode 100644 internal/imageprocessor/fit_cover_internal_test.go diff --git a/TODO.md b/TODO.md index 4b27407..cc2a818 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,18 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-15 a thin source requested with `fit=cover` no longer makes libvips + build a huge image (closes #282): libvips' thumbnail scaled the whole source + to cover the requested size and copied that into memory before it cut out the + centre, so an 8192x64 source at 8192x8192 made it allocate about 24 GiB, and + an 8192x1 source answered 500. The image processor now first cuts the centre + of the source, widened to whole source pixels, and thumbnail scales and cuts + only that, by the same amount on both sides as before. What it copies into + memory is then larger than the output by less than one scaled source pixel on + each side. With the runtime image's libvips and JPEG output at 8192x8192, + resident memory peaked at about 242 MiB for the 8192x64 source and 431 MiB for + the 8192x1 one. The output can sit a fraction of an output pixel from where it + was. - 2026-10-15 a `/v1/image/` URL whose source path or source query holds a `#`, written as `%23`, is refused with 400 (closes #286): the source URL built from it ended at the `#` when fetched, so each text after it fetched the same diff --git a/internal/imageprocessor/fit_cover_internal_test.go b/internal/imageprocessor/fit_cover_internal_test.go new file mode 100644 index 0000000..0b01211 --- /dev/null +++ b/internal/imageprocessor/fit_cover_internal_test.go @@ -0,0 +1,268 @@ +package imageprocessor + +import ( + "bytes" + "image" + "image/color" + "image/png" + "math" + "testing" +) + +// TestImageProcessor_FitCoverServesThinSource verifies that FitCover serves +// a thin source at the requested size. An 8192x1 source scaled to cover +// 8192x8192 is 8192 times as long as the output on its long side. The centre +// of an 8192x1 source with the proportions of 1x8192 is 1/8192 of a pixel +// wide. Each case is also turned on its side. +func TestImageProcessor_FitCoverServesThinSource(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + inputWidth, inputHeight int + size Size + }{ + {"8192x1 at 8192x8192", 8192, 1, Size{Width: 8192, Height: 8192}}, + {"8192x1 at 1x8192", 8192, 1, Size{Width: 1, Height: 8192}}, + {"1x8192 at 8192x8192", 1, 8192, Size{Width: 8192, Height: 8192}}, + {"1x8192 at 8192x1", 1, 8192, Size{Width: 8192, Height: 1}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + input := createTestJPEG(t, tt.inputWidth, tt.inputHeight) + + result, err := New(Params{}).Process(t.Context(), + bytes.NewReader(input), + &Request{Size: tt.size, Format: FormatJPEG, FitMode: FitCover}) + if err != nil { + t.Fatalf("Process() error = %v", err) + } + + defer func() { _ = result.Content.Close() }() + + t.Logf("Process() made %dx%d", result.Width, result.Height) + + if result.Width != tt.size.Width || result.Height != tt.size.Height { + t.Errorf("Process() made %dx%d, want %dx%d", + result.Width, result.Height, tt.size.Width, tt.size.Height) + } + }) + } +} + +// createBandsPNG creates a PNG of three equal bands, red, green and blue, +// side by side when it is wider than tall and top to bottom otherwise. +func createBandsPNG(t *testing.T, width, height int) []byte { + t.Helper() + + bands := []color.RGBA{ + {R: 255, A: 255}, + {G: 255, A: 255}, + {B: 255, A: 255}, + } + + img := image.NewRGBA(image.Rect(0, 0, width, height)) + for y := range height { + for x := range width { + band := 3 * y / height + if width > height { + band = 3 * x / width + } + + img.Set(x, y, bands[band]) + } + } + + return encodePNG(t, img) +} + +// TestImageProcessor_FitCoverCropsTheCentre verifies that FitCover keeps the +// centre of the source: a 300x100 source of red, green and blue bands side +// by side, or a 100x300 one with them top to bottom, comes out at 50x50 as +// its green band. +func TestImageProcessor_FitCoverCropsTheCentre(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + inputWidth, inputHeight int + }{ + {"bands side by side", 300, 100}, + {"bands top to bottom", 100, 300}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + input := createBandsPNG(t, tt.inputWidth, tt.inputHeight) + + result, err := New(Params{}).Process(t.Context(), + bytes.NewReader(input), + &Request{ + Size: Size{Width: 50, Height: 50}, + Format: FormatPNG, + FitMode: FitCover, + }) + if err != nil { + t.Fatalf("Process() error = %v", err) + } + + defer func() { _ = result.Content.Close() }() + + output, err := png.Decode(result.Content) + if err != nil { + t.Fatalf("failed to decode output: %v", err) + } + + bounds := output.Bounds() + t.Logf("Process() made %dx%d", bounds.Dx(), bounds.Dy()) + + if bounds.Dx() != 50 || bounds.Dy() != 50 { + t.Fatalf("Process() made %dx%d, want 50x50", + bounds.Dx(), bounds.Dy()) + } + + // Resampling may blend a little of the bands beside the + // green one into the output's edges. + for y := bounds.Min.Y; y < bounds.Max.Y; y++ { + for x := bounds.Min.X; x < bounds.Max.X; x++ { + r, g, b, _ := output.At(x, y).RGBA() + if r>>8 > 64 || g>>8 < 192 || b>>8 > 64 { + t.Fatalf("output pixel %d,%d is %d,%d,%d, want green", + x, y, r>>8, g>>8, b>>8) + } + } + } + }) + } +} + +// The greys of createSquarePNG, and the grey halfway between them. +const ( + backgroundGrey = 192 + squareGrey = 64 + halfwayGrey = (backgroundGrey + squareGrey) / 2 +) + +// createSquarePNG creates a grey PNG side pixels square with a darker grey +// square squareSide pixels wide in its centre. +func createSquarePNG(t *testing.T, side, squareSide int) []byte { + t.Helper() + + first := (side - squareSide) / 2 + last := first + squareSide - 1 + + img := image.NewGray(image.Rect(0, 0, side, side)) + for y := range side { + for x := range side { + grey := uint8(backgroundGrey) + if x >= first && x <= last && y >= first && y <= last { + grey = squareGrey + } + + img.SetGray(x, y, color.Gray{Y: grey}) + } + } + + return encodePNG(t, img) +} + +// squareLength returns how long the square is along greys, the greys of a +// row or column of the output that crosses it: the distance between the +// first and the last point where greys passes halfwayGrey, or 0 when there +// is no such point. Resampling blurs the square's edges over several +// pixels, and each point is placed between the two pixels on either side +// of it in proportion to their greys, so the length is measured to a +// fraction of a pixel. +func squareLength(greys []float64) float64 { + first, last := -1.0, -1.0 + + for i := 1; i < len(greys); i++ { + before, after := greys[i-1], greys[i] + if (before > halfwayGrey) == (after > halfwayGrey) { + continue + } + + point := float64(i-1) + (before-halfwayGrey)/(before-after) + if first < 0 { + first = point + } + + last = point + } + + return last - first +} + +// TestImageProcessor_FitCoverKeepsProportions verifies that FitCover scales +// both sides of the source by the same amount: a square in the centre of +// the source comes out square. Its width and height are measured along the +// output's middle row and column. +func TestImageProcessor_FitCoverKeepsProportions(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + side, squareSide int + size Size + }{ + {"16x16 at 100x60", 16, 8, Size{Width: 100, Height: 60}}, + {"16x16 at 60x100", 16, 8, Size{Width: 60, Height: 100}}, + {"10x10 at 100x35", 10, 2, Size{Width: 100, Height: 35}}, + {"10x10 at 35x100", 10, 2, Size{Width: 35, Height: 100}}, + {"64x64 at 300x100", 64, 16, Size{Width: 300, Height: 100}}, + {"64x64 at 100x300", 64, 16, Size{Width: 100, Height: 300}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + input := createSquarePNG(t, tt.side, tt.squareSide) + + result, err := New(Params{}).Process(t.Context(), + bytes.NewReader(input), + &Request{Size: tt.size, Format: FormatPNG, FitMode: FitCover}) + if err != nil { + t.Fatalf("Process() error = %v", err) + } + + defer func() { _ = result.Content.Close() }() + + output, err := png.Decode(result.Content) + if err != nil { + t.Fatalf("failed to decode output: %v", err) + } + + bounds := output.Bounds() + if bounds.Dx() != tt.size.Width || bounds.Dy() != tt.size.Height { + t.Fatalf("Process() made %dx%d, want %dx%d", + bounds.Dx(), bounds.Dy(), tt.size.Width, tt.size.Height) + } + + row := make([]float64, bounds.Dx()) + for x := range row { + r, _, _, _ := output.At(x, bounds.Dy()/2).RGBA() + row[x] = float64(r >> 8) + } + + column := make([]float64, bounds.Dy()) + for y := range column { + r, _, _, _ := output.At(bounds.Dx()/2, y).RGBA() + column[y] = float64(r >> 8) + } + + width, height := squareLength(row), squareLength(column) + t.Logf("square is %.2f wide and %.2f high", width, height) + + if width == 0 || math.Abs(width-height) > 0.5 { + t.Errorf("square is %.2f wide and %.2f high, want it square", + width, height) + } + }) + } +} diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index 2a1fbf5..716b9ad 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -475,8 +475,8 @@ func (p *ImageProcessor) resize( ) error { switch fit { case FitCover, "": - // Resize and crop to fill exact dimensions (default) - return img.Thumbnail(width, height, vips.InterestingCentre) + // Crop the centre and resize to fill exact dimensions (default) + return cropCentreAndResize(img, width, height) case FitContain: // Resize to fit within dimensions, maintaining aspect ratio @@ -530,6 +530,38 @@ func (p *ImageProcessor) resize( } } +// cropCentreAndResize scales img by the same amount on both sides, the +// least that makes it cover width x height, and cuts the centre width x +// height out of it, as libvips' thumbnail does with InterestingCentre. +// thumbnail alone scales the whole image and copies the result into memory +// before it cuts, and for a thin image that is far larger than the image or +// the output: 8192x64 at 8192x8192 is 1048576x8192, about 24 GiB. So the +// centre is cut from img first, widened to whole pixels of img, which adds +// less than one pixel of img on each side, and thumbnail scales and cuts +// only that. +func cropCentreAndResize(img *vips.ImageRef, width, height int) error { + imgW, imgH := img.Width(), img.Height() + + // An image wider than the requested proportions keeps its height and + // loses width down to imgH*width/height, any other keeps its width and + // loses height down to imgW*height/width. Half of what is lost comes + // off each side, rounded down to whole pixels, so the part kept holds + // all of the centre. + left, top := 0, 0 + if imgW*height > imgH*width { + left = (imgW*height - imgH*width) / (2 * height) //nolint:mnd // each side + } else { + top = (imgH*width - imgW*height) / (2 * width) //nolint:mnd // each side + } + + err := img.ExtractArea(left, top, imgW-2*left, imgH-2*top) + if err != nil { + return err + } + + return img.Thumbnail(width, height, vips.InterestingCentre) +} + const defaultQuality = 85 // encode encodes an image to the specified format. govips gives pixa no way