diff --git a/model/artwork.go b/model/artwork.go index 72258b923..dd0f45004 100644 --- a/model/artwork.go +++ b/model/artwork.go @@ -21,6 +21,10 @@ type ItemImage struct { ImageHash string `structs:"-" json:"imageHash,omitempty"` ImageAbsent bool `structs:"-" json:"imageAbsent,omitempty"` BlurHash string `structs:"-" json:"blurHash,omitempty"` + // Dimensions of the original image. A blurhash carries no aspect ratio, so a client + // needs these to decode the placeholder into the shape the real image will occupy. + ImageWidth int `structs:"-" json:"imageWidth,omitempty"` + ImageHeight int `structs:"-" json:"imageHeight,omitempty"` } // ItemArtwork is an entity's resolved artwork state. Hash=="" means known absent. @@ -45,11 +49,25 @@ type ItemArtworkInfo struct { ItemID string Hash string BlurHash string + Width int + Height int } // Absent reports a known-absent artwork state (resolved, no image). func (i ItemArtworkInfo) Absent() bool { return i.Hash == "" } +// Image projects the hydration entry onto the entity-facing struct, so every hydration +// site copies the same set of fields. +func (i ItemArtworkInfo) Image() ItemImage { + return ItemImage{ + ImageHash: i.Hash, + ImageAbsent: i.Absent(), + BlurHash: i.BlurHash, + ImageWidth: i.Width, + ImageHeight: i.Height, + } +} + type ArtworkQueueItem struct { ItemKind string `structs:"item_kind"` ItemID string `structs:"item_id"` diff --git a/model/artwork_test.go b/model/artwork_test.go index 606e45277..5beaca5a7 100644 --- a/model/artwork_test.go +++ b/model/artwork_test.go @@ -23,6 +23,22 @@ var _ = Describe("ItemImage JSON", func() { Expect(out).To(HaveKeyWithValue("blurHash", "LEHV6nWB2yk8")) }) + // Clients decode the blurhash into a bitmap of their choosing, so without the dimensions they + // cannot know the placeholder's shape and default to a square. + It("exposes the image dimensions alongside the blurhash", func() { + al := model.Album{ID: "al-3", Name: "Album"} + al.BlurHash = "LEHV6nWB2yk8" + al.ImageWidth, al.ImageHeight = 1200, 800 + + var out map[string]any + data, err := json.Marshal(al) + Expect(err).ToNot(HaveOccurred()) + Expect(json.Unmarshal(data, &out)).To(Succeed()) + + Expect(out).To(HaveKeyWithValue("imageWidth", BeNumerically("==", 1200))) + Expect(out).To(HaveKeyWithValue("imageHeight", BeNumerically("==", 800))) + }) + It("omits artwork state when the entity has none", func() { var out map[string]any data, err := json.Marshal(model.Album{ID: "al-2", Name: "Album"}) @@ -32,6 +48,8 @@ var _ = Describe("ItemImage JSON", func() { Expect(out).ToNot(HaveKey("imageHash")) Expect(out).ToNot(HaveKey("blurHash")) Expect(out).ToNot(HaveKey("imageAbsent")) + Expect(out).ToNot(HaveKey("imageWidth")) + Expect(out).ToNot(HaveKey("imageHeight")) }) It("exposes known-absent artwork so clients can skip the request", func() { diff --git a/persistence/artwork_hydration.go b/persistence/artwork_hydration.go index ae9564d44..8bc6d579b 100644 --- a/persistence/artwork_hydration.go +++ b/persistence/artwork_hydration.go @@ -70,9 +70,7 @@ func hydrateItemImages(ctx context.Context, db dbx.Builder, kind model.Kind, ids // applyItemImage copies a hydration entry onto img; a missing entry leaves it zero (unresolved). func applyItemImage(infos map[string]model.ItemArtworkInfo, id string, img *model.ItemImage) { if info, ok := infos[id]; ok { - img.ImageHash = info.Hash - img.ImageAbsent = info.Absent() - img.BlurHash = info.BlurHash + *img = info.Image() } } @@ -98,8 +96,7 @@ func hydrateMediaFileArtwork(ctx context.Context, db dbx.Builder, mfs model.Medi eligible := mf.HasCoverArt && conf.Server.EnableMediaFileCoverArt ownInfo, ownResolved := mfInfos[mf.ID] if eligible && ownResolved && !ownInfo.Absent() { - mf.ImageHash = ownInfo.Hash // own resolved art wins - mf.BlurHash = ownInfo.BlurHash + mf.ItemImage = ownInfo.Image() // own resolved art wins continue } ownWontResolve := !eligible || (ownResolved && ownInfo.Absent()) @@ -110,8 +107,7 @@ func hydrateMediaFileArtwork(ctx context.Context, db dbx.Builder, mfs model.Medi // (and a blurhash) belonging to a different image. if album, ok := albumInfos[mf.AlbumID]; ok && !album.Absent() { if mf.DiscNumber == 0 && ownWontResolve { - mf.ImageHash = album.Hash - mf.BlurHash = album.BlurHash + mf.ItemImage = album.Image() } continue } diff --git a/persistence/artwork_hydration_test.go b/persistence/artwork_hydration_test.go index d3fefb343..28678700b 100644 --- a/persistence/artwork_hydration_test.go +++ b/persistence/artwork_hydration_test.go @@ -318,12 +318,12 @@ var _ = Describe("Artwork hydration", func() { Expect(byID["2002"].AlbumImage.ImageAbsent).To(BeTrue()) }) - It("carries the blurhash alongside the hash in both the own-art and inherited branches", func() { + It("carries the blurhash and its dimensions alongside the hash in both the own-art and inherited branches", func() { setCover("1001", true) // eligible, resolves its own art -> own-art-wins branch DeferCleanup(func() { setCover("1001", false) }) - Expect(aw.PutImage(&model.Artwork{Hash: "mfh1001blurxxxxx", Mime: "image/jpeg", BlurHash: "LTRACKblur"})).To(Succeed()) - Expect(aw.PutImage(&model.Artwork{Hash: "alh102blurxxxxxx", Mime: "image/jpeg", BlurHash: "LALBUMblur"})).To(Succeed()) + Expect(aw.PutImage(&model.Artwork{Hash: "mfh1001blurxxxxx", Mime: "image/jpeg", BlurHash: "LTRACKblur", Width: 640, Height: 480})).To(Succeed()) + Expect(aw.PutImage(&model.Artwork{Hash: "alh102blurxxxxxx", Mime: "image/jpeg", BlurHash: "LALBUMblur", Width: 1200, Height: 800})).To(Succeed()) putInfo("mf", "1001", "mfh1001blurxxxxx") putInfo("al", "102", "alh102blurxxxxxx") // 1002's album: single-disc inheritance branch @@ -331,9 +331,13 @@ var _ = Describe("Artwork hydration", func() { Expect(byID["1001"].ImageHash).To(Equal("mfh1001blurxxxxx")) Expect(byID["1001"].BlurHash).To(Equal("LTRACKblur")) + Expect(byID["1001"].ImageWidth).To(Equal(640)) + Expect(byID["1001"].ImageHeight).To(Equal(480)) Expect(byID["1002"].ImageHash).To(Equal("alh102blurxxxxxx")) Expect(byID["1002"].BlurHash).To(Equal("LALBUMblur")) + Expect(byID["1002"].ImageWidth).To(Equal(1200)) + Expect(byID["1002"].ImageHeight).To(Equal(800)) }) It("keeps an eligible file optimistic when its own art is unresolved, even if the album is absent", func() { @@ -773,15 +777,17 @@ var _ = Describe("Artwork hydration", func() { }) Describe("applyItemImage", func() { - It("copies hash, absence and blurhash onto the item", func() { + It("copies hash, absence, blurhash and dimensions onto the item", func() { infos := map[string]model.ItemArtworkInfo{ - "al-1": {ItemID: "al-1", Hash: "0123456789abcdef", BlurHash: "LEHV6nWB2yk8"}, + "al-1": {ItemID: "al-1", Hash: "0123456789abcdef", BlurHash: "LEHV6nWB2yk8", Width: 1200, Height: 800}, } var img model.ItemImage applyItemImage(infos, "al-1", &img) Expect(img.ImageHash).To(Equal("0123456789abcdef")) Expect(img.ImageAbsent).To(BeFalse()) Expect(img.BlurHash).To(Equal("LEHV6nWB2yk8")) + Expect(img.ImageWidth).To(Equal(1200)) + Expect(img.ImageHeight).To(Equal(800)) }) It("marks a hashless entry absent and carries no blurhash", func() { diff --git a/persistence/artwork_repository.go b/persistence/artwork_repository.go index f5ba7be13..2fec7795d 100644 --- a/persistence/artwork_repository.go +++ b/persistence/artwork_repository.go @@ -190,7 +190,8 @@ func (r *artworkRepository) DeleteForItems(kind model.Kind, ids []string) error func (r *artworkRepository) GetInfoForItems(kind model.Kind, ids []string) (map[string]model.ItemArtworkInfo, error) { res := map[string]model.ItemArtworkInfo{} for chunk := range slices.Chunk(ids, artworkBatchSize) { - sel := Select("ia.item_id", "ia.hash", "COALESCE(a.blur_hash, '') as blur_hash"). + sel := Select("ia.item_id", "ia.hash", "COALESCE(a.blur_hash, '') as blur_hash", + "COALESCE(a.width, 0) as width", "COALESCE(a.height, 0) as height"). From(itemArtworkTable + " ia"). LeftJoin("artwork a ON a.hash = ia.hash"). Where(And{ @@ -198,18 +199,12 @@ func (r *artworkRepository) GetInfoForItems(kind model.Kind, ids []string) (map[ Eq{"ia.image_type": model.ImageTypePrimary}, Eq{"ia.item_id": chunk}, }) - var rows []struct { - ItemID string - Hash string - BlurHash string - } + var rows []model.ItemArtworkInfo if err := r.items.queryAll(sel, &rows); err != nil { return nil, err } for _, row := range rows { - res[row.ItemID] = model.ItemArtworkInfo{ - ItemID: row.ItemID, Hash: row.Hash, BlurHash: row.BlurHash, - } + res[row.ItemID] = row } } return res, nil diff --git a/persistence/artwork_repository_test.go b/persistence/artwork_repository_test.go index 5a3c1394e..19b7219fe 100644 --- a/persistence/artwork_repository_test.go +++ b/persistence/artwork_repository_test.go @@ -214,8 +214,8 @@ var _ = Describe("ArtworkRepository", func() { Expect(got.Hash).To(BeEmpty()) }) - It("hydrates a page in one batch, including blurhash and absence", func() { - Expect(repo.PutImage(&model.Artwork{Hash: "h9", Mime: "image/jpeg", BlurHash: "BH9"})).To(Succeed()) + It("hydrates a page in one batch, including blurhash, dimensions and absence", func() { + Expect(repo.PutImage(&model.Artwork{Hash: "h9", Mime: "image/jpeg", BlurHash: "BH9", Width: 1200, Height: 800})).To(Succeed()) Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "al", ItemID: "x1", ImageType: model.ImageTypePrimary, Hash: "h9", Source: "folder"})).To(Succeed()) Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "al", ItemID: "x2", ImageType: model.ImageTypePrimary, Hash: "", Source: ""})).To(Succeed()) @@ -224,6 +224,8 @@ var _ = Describe("ArtworkRepository", func() { Expect(info).To(HaveLen(2)) Expect(info["x1"].Hash).To(Equal("h9")) Expect(info["x1"].BlurHash).To(Equal("BH9")) + Expect(info["x1"].Width).To(Equal(1200)) + Expect(info["x1"].Height).To(Equal(800)) Expect(info["x1"].Absent()).To(BeFalse()) Expect(info["x2"].Absent()).To(BeTrue()) _, unresolved := info["x3"] diff --git a/ui/src/common/Artwork.jsx b/ui/src/common/Artwork.jsx index 1f9159968..4c9398a4e 100644 --- a/ui/src/common/Artwork.jsx +++ b/ui/src/common/Artwork.jsx @@ -77,6 +77,10 @@ export const Artwork = ({ // The blurhash stays mounted under the image until the fade ends. Swapping them the moment the // blob arrives would expose the empty container for the length of the fade. const showBlurHash = !!record.blurHash && !instant && !faded + // A square request is padded, not cropped, so its content is already aspect-fit inside the square + // the server returns; `contain` is what keeps placeholder and image on the same pixels. + const effectiveFit = square ? 'contain' : fit + const ratio = record.imageWidth / record.imageHeight const handleClick = imgUrl && onClick ? onClick : undefined return (
{showBlurHash && ( - + )} {imgUrl && ( setDecoded(true)} /> diff --git a/ui/src/common/Artwork.test.jsx b/ui/src/common/Artwork.test.jsx index 7ced75f4c..7b9117860 100644 --- a/ui/src/common/Artwork.test.jsx +++ b/ui/src/common/Artwork.test.jsx @@ -43,6 +43,58 @@ describe('Artwork', () => { expect(container.querySelector('canvas')).toBeNull() }) + // The placeholder has to land exactly where the image will, or it jumps when the image swaps in. + it('shapes the blurhash like the artwork and fits it like the image', () => { + useImageUrl.mockReturnValue({ imgUrl: null, loading: true }) + const nonSquare = { ...withArt, imageWidth: 1200, imageHeight: 800 } + const { container } = render( + , + ) + const canvas = container.querySelector('canvas') + expect(canvas.width).toBe(32) + expect(canvas.height).toBe(21) + expect(canvas.style.objectFit).toBe('contain') + }) + + // A square request is padded, not cropped, so the artwork still sits letterboxed inside the + // square the server returns and the placeholder has to letterbox with it. + it('letterboxes the blurhash when the server pads a non-square image to a square', () => { + useImageUrl.mockReturnValue({ imgUrl: null, loading: true }) + const nonSquare = { ...withArt, imageWidth: 1200, imageHeight: 800 } + const { container } = render() + const canvas = container.querySelector('canvas') + expect(canvas.width).toBe(32) + expect(canvas.height).toBe(21) + expect(canvas.style.objectFit).toBe('contain') + }) + + // The padded square the server returns is aspect-fit, so cropping it would disagree with the + // placeholder. Both renderers have to read `square` the same way. + it('fits the image itself with contain when the server padded to a square', () => { + useImageUrl.mockReturnValue({ imgUrl: 'blob:abc', loading: false }) + const { container } = render( + , + ) + expect(container.querySelector('img').style.objectFit).toBe('contain') + }) + + it('fills the box for square artwork, the overwhelmingly common case', () => { + useImageUrl.mockReturnValue({ imgUrl: null, loading: true }) + const sq = { ...withArt, imageWidth: 600, imageHeight: 600 } + const { container } = render() + const canvas = container.querySelector('canvas') + expect(canvas.width).toBe(32) + expect(canvas.height).toBe(32) + }) + + it('falls back to a square blurhash when the record has no dimensions', () => { + useImageUrl.mockReturnValue({ imgUrl: null, loading: true }) + const { container } = render() + const canvas = container.querySelector('canvas') + expect(canvas.width).toBe(32) + expect(canvas.height).toBe(32) + }) + it('mounts the image only once its blob is ready', () => { useImageUrl.mockReturnValue({ imgUrl: 'blob:abc', loading: false }) const { container } = render() diff --git a/ui/src/common/BlurHashCanvas.jsx b/ui/src/common/BlurHashCanvas.jsx index 554230385..05bb3b3ba 100644 --- a/ui/src/common/BlurHashCanvas.jsx +++ b/ui/src/common/BlurHashCanvas.jsx @@ -5,8 +5,26 @@ import { decode } from 'blurhash' // A blurhash carries no detail beyond a few dozen pixels; CSS upscales the canvas. const DECODE_SIZE = 32 -export const BlurHashCanvas = ({ hash, className, style }) => { +// bitmapSize shapes the decode target like the source image: a blurhash carries no aspect ratio, +// so a square decode stretched to the box distorts the blur and overpaints where the image won't reach. +const bitmapSize = (ratio) => { + if (!(ratio > 0) || !Number.isFinite(ratio)) { + return { width: DECODE_SIZE, height: DECODE_SIZE } + } + return ratio >= 1 + ? { + width: DECODE_SIZE, + height: Math.max(1, Math.round(DECODE_SIZE / ratio)), + } + : { + width: Math.max(1, Math.round(DECODE_SIZE * ratio)), + height: DECODE_SIZE, + } +} + +export const BlurHashCanvas = ({ hash, ratio, fit, className, style }) => { const canvasRef = useRef(null) + const { width, height } = bitmapSize(ratio) useEffect(() => { if (!hash || !canvasRef.current) { @@ -17,16 +35,16 @@ export const BlurHashCanvas = ({ hash, className, style }) => { return } // Clear first so a hash change that fails to decode never leaves a stale frame. - ctx.clearRect(0, 0, DECODE_SIZE, DECODE_SIZE) + ctx.clearRect(0, 0, width, height) try { - const pixels = decode(hash, DECODE_SIZE, DECODE_SIZE) - const imageData = ctx.createImageData(DECODE_SIZE, DECODE_SIZE) + const pixels = decode(hash, width, height) + const imageData = ctx.createImageData(width, height) imageData.data.set(pixels) ctx.putImageData(imageData, 0, 0) } catch { // A malformed hash simply leaves the canvas blank. } - }, [hash]) + }, [hash, width, height]) if (!hash) { return null @@ -34,10 +52,10 @@ export const BlurHashCanvas = ({ hash, className, style }) => { return (