nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-01 07:21:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(artwork): shape the blurhash placeholder to the artwork's aspect ratio

Browse Source 
The UI decoded every blurhash into a 32x32 bitmap and stretched it to fill its
container, so a non-square cover showed a full-box blur that collapsed into a
letterboxed image the moment it loaded. On the album detail page the
placeholder overhung the image by a third of the box height.

A blurhash string carries no aspect ratio of its own, so the dimensions have to
come from the server. artwork.width/height were already stored and read by
nobody; they now surface on ItemImage as imageWidth/imageHeight, hydrated
through the join that was already in place. Existing rows already carry them,
so no migration or rescan is needed.

A square request is padded rather than cropped, which aspect-fits the content
inside the square the server returns. That made the grid a second instance of
the same bug, so `square` now implies contain for the image as well as the
placeholder, instead of the two renderers reading it differently.
This commit is contained in:
Deluan 2026-07-26 10:21:45 -04:00
parent 18f1236595
commit d732e5419a
10 changed files with 187 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
model/artwork.go
View File

@ -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"`

18
model/artwork_test.go
View File

@ -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() {

10
persistence/artwork_hydration.go
View File

@ -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
}

16
persistence/artwork_hydration_test.go
View File

@ -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() {

13
persistence/artwork_repository.go
View File

@ -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

6
persistence/artwork_repository_test.go
View File

@ -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"]

13
ui/src/common/Artwork.jsx
View File

@ -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 (
<div
@ -85,7 +89,12 @@ export const Artwork = ({
style={{ cursor: handleClick ? 'pointer' : 'default' }}
>
{showBlurHash && (
<BlurHashCanvas hash={record.blurHash} className={classes.fill} />
<BlurHashCanvas
hash={record.blurHash}
ratio={ratio}
fit={effectiveFit}
className={classes.fill}
/>
)}
{imgUrl && (
<img
@ -98,7 +107,7 @@ export const Artwork = ({
instant && classes.imgInstant,
decoded && classes.imgVisible,
)}
style={{ objectFit: fit }}
style={{ objectFit: effectiveFit }}
// Fading on decode, not on mount, keeps the image from ramping up before it can paint.
onLoad={() => setDecoded(true)}
/>

52
ui/src/common/Artwork.test.jsx
View File

@ -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(
<Artwork record={nonSquare} fit="contain" title="Album" />,
)
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(<Artwork record={nonSquare} square />)
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(
<Artwork record={withArt} square fit="cover" title="Album" />,
)
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(<Artwork record={sq} square />)
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(<Artwork record={withArt} />)
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(<Artwork record={withArt} title="Album" />)

37
ui/src/common/BlurHashCanvas.jsx
View File

@ -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 (
<canvas
ref={canvasRef}
width={DECODE_SIZE}
height={DECODE_SIZE}
width={width}
height={height}
className={className}
style={style}
style={{ ...style, objectFit: fit }}
aria-hidden="true"
/>
)
@ -45,6 +63,9 @@ export const BlurHashCanvas = ({ hash, className, style }) => {
BlurHashCanvas.propTypes = {
hash: PropTypes.string,
// Aspect ratio (width / height) of the image this stands in for; square when omitted or unusable.
ratio: PropTypes.number,
fit: PropTypes.oneOf(['cover', 'contain']),
className: PropTypes.string,
style: PropTypes.object,
}

37
ui/src/common/BlurHashCanvas.test.jsx
View File

@ -38,6 +38,43 @@ describe('BlurHashCanvas', () => {
expect(imageData.data.some((byte) => byte !== 0)).toBe(true)
})
it('decodes into a bitmap shaped like the image, so the blur is not distorted', () => {
const { container } = render(
<BlurHashCanvas hash="LEHV6nWB2yk8pyo0adR*.7kCMdnj" ratio={1200 / 800} />,
)
// Longest side pinned to the decode size; the other follows the ratio.
expect(ctxMock.createImageData).toHaveBeenCalledWith(32, 21)
const canvas = container.querySelector('canvas')
expect(canvas.width).toBe(32)
expect(canvas.height).toBe(21)
})
it('shapes a portrait ratio the other way round', () => {
render(<BlurHashCanvas hash="LEHV6nWB2yk8pyo0adR*.7kCMdnj" ratio={0.5} />)
expect(ctxMock.createImageData).toHaveBeenCalledWith(16, 32)
})
it('never collapses an extreme ratio to a zero-sized bitmap', () => {
render(<BlurHashCanvas hash="LEHV6nWB2yk8pyo0adR*.7kCMdnj" ratio={200} />)
expect(ctxMock.createImageData).toHaveBeenCalledWith(32, 1)
})
it('falls back to a square when the ratio is unknown or nonsense', () => {
render(<BlurHashCanvas hash="LEHV6nWB2yk8pyo0adR*.7kCMdnj" ratio={0} />)
expect(ctxMock.createImageData).toHaveBeenCalledWith(32, 32)
})
it('applies the object-fit it is given, so it lands where the image will', () => {
const { container } = render(
<BlurHashCanvas
hash="LEHV6nWB2yk8pyo0adR*.7kCMdnj"
ratio={1.5}
fit="contain"
/>,
)
expect(container.querySelector('canvas').style.objectFit).toBe('contain')
})
it('renders a canvas without throwing on a malformed hash, and draws nothing', () => {
const spy = vi.spyOn(console, 'error').mockImplementation(() => {})
const { container } = render(<BlurHashCanvas hash="!!!not-a-blurhash!!!" />)