From 1cb618e5152dfd3ef48970517ff29d37d85be77f Mon Sep 17 00:00:00 2001 From: tannevaled Date: Thu, 10 Sep 2026 22:21:18 +0200 Subject: [PATCH 1/2] Draw a Lab colour instead of a grey of its lightness labSpace read three numbers and used only the first, drawing a grey of the right weight. Its own comment said so and called it honest, which it was -- but it drops the two axes that carry the colour, and Lab(50, 20, -30) is a mid purple, not a mid grey. It is now the real conversion, through gfx/color v0.23.0's LabToSRGBWP: the CIE inverse under the space's own white point, adapted to D65. Two things came with it. /Range is read and kept on the space, because an image decodes against it. Lab is the ONE space in the format whose default /Decode is not [0 1] per component -- it is [0 100 amin amax bmin bmax] -- so a Lab image with no /Decode of its own was having its lightness read as a hundredth of what it said, and came out black. A Lab space with no /WhitePoint falls back to the equal-energy point (1, 1, 1) rather than to a device space, because Lab has no device namesake. That makes the white-point multiplication the identity, which is the most conservative reading of a file that said nothing, and it is also what GfxLabColorSpace's constructor does, so a malformed file is read the same way by both. This was blocked on a misreading, now corrected in the file's own comment. GfxLabColorSpace::getXYZ does not multiply by the white point, and reading that function alone says poppler disagrees with ISO 32000-2 8.6.5.4. It does not: ::getRGB multiplies immediately after calling it. A hand-built four-pixel Lab document run through pdfimages settles it -- the specification's formula matches 4 of 4 pixels within one level, the no-white-point formula is 13 levels out on a neutral mid tone. Every expected value here is poppler's, taken the same way: it draws Lab(50, 20, -30) with no white point as (131, 109, 171), and we draw (131, 108, 170). --- calibrated.go | 55 +++++++++++++++++++++---- calibrated_test.go | 100 +++++++++++++++++++++++++++++++++++++++++++++ go.mod | 2 +- go.sum | 2 + image.go | 13 ++++++ space.go | 16 +++----- space_test.go | 7 +++- 7 files changed, 174 insertions(+), 21 deletions(-) diff --git a/calibrated.go b/calibrated.go index 78fcafd..f6790a5 100644 --- a/calibrated.go +++ b/calibrated.go @@ -6,16 +6,18 @@ import ( "image/color" ) -// This file reads the two calibrated spaces: CalGray and CalRGB. Each +// This file reads the three CIE-based spaces: CalGray, CalRGB and Lab. Each // says what colour its numbers name by giving a white point and a rule for -// reaching CIE XYZ, so neither is its device namesake and neither can be read -// by passing its numbers through. +// reaching CIE XYZ, so none of them is its device namesake and none can be +// read by passing its numbers through. // -// Lab is the third CIE space and is NOT here. poppler's GfxLabColorSpace does -// not multiply by the white point where ISO 32000-2 8.6.5.4 says to -// (X = Xw*g(M)), so a correct Lab and the judge this repository measures -// against would disagree for a reason that is not ours. That wants an -// experiment of its own before either is changed. +// Lab is the third and is here too. It was left out at first on the strength +// of a misreading: GfxLabColorSpace::getXYZ does not multiply by the white +// point where ISO 32000-2 8.6.5.4 says to, which looked like poppler +// disagreeing with the format. It does not -- ::getRGB multiplies immediately +// after calling it. A four-pixel Lab document run through pdfimages settles +// it: the specification's formula matches 4 of 4 pixels within one level, and +// the no-white-point formula is 13 levels out on a neutral mid tone. // // The arithmetic lives in gfx/color; what is here is reading a dictionary and // carrying its defaults. @@ -98,3 +100,40 @@ func (r *renderer) calRGBSpace(arr reader.Array) *space { return color.RGBA{R: byteOf(red), G: byteOf(green), B: byteOf(blue), A: 255} }} } + +// labRangeDefault is the default /Range: the two opponent axes run from -100 +// to 100 unless the space narrows them. Lightness is always 0 to 100 and is +// not part of /Range. +var labRangeDefault = [4]float64{-100, 100, -100, 100} + +// labSpace reads a Lab space: a white point and the range of its two opponent +// axes. +// +// The range is carried on the space because an image decodes against it. Lab +// is the one space in the format whose default /Decode is not [0 1] per +// component: it is [0 100 amin amax bmin bmax], so an image that gives no +// /Decode of its own would otherwise have its lightness read as a hundredth of +// what it says. +func (r *renderer) labSpace(arr reader.Array) *space { + d := r.calDict(arr) + white, ok := r.calWhitePoint(d) + if !ok { + // Lab has no device namesake to fall back to, so unlike CalGray and + // CalRGB there is nothing to decline into. (1, 1, 1) is the equal- + // energy point, which makes the white-point multiplication the + // identity -- the most conservative reading of a file that said + // nothing -- and it is also what GfxLabColorSpace's constructor uses + // when /WhitePoint is missing, so a malformed file is read the same + // way by both. + white = gfxcolor.WhitePoint{X: 1, Y: 1, Z: 1} + } + rng := labRangeDefault + if v, ok := r.calFloats(d, "Range", 4); ok && v[0] <= v[1] && v[2] <= v[3] { + copy(rng[:], v) + } + return &space{name: "Lab", components: 3, labRange: &rng, convert: func(v []float64) color.RGBA { + red, green, blue := gfxcolor.LabToSRGBWP( + gfxcolor.Lab{L: at(v, 0), A: at(v, 1), B: at(v, 2)}, white) + return color.RGBA{R: byteOf(red), G: byteOf(green), B: byteOf(blue), A: 255} + }} +} diff --git a/calibrated_test.go b/calibrated_test.go index 1db356e..35c70d0 100644 --- a/calibrated_test.go +++ b/calibrated_test.go @@ -303,3 +303,103 @@ func TestACalibratedSpaceArrayWithNoDictionaryAtAll(t *testing.T) { } } } + +// labImage draws a 1x1 Lab image of one sample triple through the given space +// array and returns the pixel. Lab is the one space whose samples do not +// decode over [0, 1], so an image is the only way to exercise that. +func labImage(t *testing.T, spaceArr reader.Object, decode reader.Object, samples []byte) color.RGBA { + t.Helper() + w := reader.NewWriter("1.7") + pagesRef := w.Reserve() + dict := reader.Dict{ + "Type": reader.Name("XObject"), "Subtype": reader.Name("Image"), + "Width": reader.Integer(1), "Height": reader.Integer(1), + "ColorSpace": spaceArr, "BitsPerComponent": reader.Integer(8)} + if decode != nil { + dict["Decode"] = decode + } + img := w.Add(&reader.Stream{Dict: dict, Raw: samples}) + pageRef := w.Add(reader.Dict{"Type": reader.Name("Page"), "Parent": pagesRef, + "MediaBox": nums(0, 0, 1, 1), + "Resources": reader.Dict{"XObject": reader.Dict{"I": img}}, + "Contents": w.Add(&reader.Stream{Dict: reader.Dict{}, + Raw: []byte("q 1 0 0 1 0 0 cm /I Do Q")})}) + w.Put(pagesRef, reader.Dict{"Type": reader.Name("Pages"), + "Kids": reader.Array{pageRef}, "Count": reader.Integer(1)}) + out, err := w.Finish(reader.Dict{"Root": w.Add(reader.Dict{ + "Type": reader.Name("Catalog"), "Pages": pagesRef})}) + if err != nil { + t.Fatal(err) + } + d, err := reader.Open(out) + if err != nil { + t.Fatal(err) + } + pic, err := Page(d, 1, Options{Scale: 1}) + if err != nil { + t.Fatal(err) + } + r, g, b, a := pic.At(0, 0).RGBA() + return color.RGBA{R: uint8(r >> 8), G: uint8(g >> 8), B: uint8(b >> 8), A: uint8(a >> 8)} +} + +func TestALabImageWithNoDecodeArrayUsesLabsOwnDefault(t *testing.T) { + // Lab is the one space in the format whose default /Decode is not [0 1] + // per component: it is [0 100 amin amax bmin bmax]. Without that, a + // lightness of 50 would be read as 0.5 and the picture would be black. + space := reader.Array{reader.Name("Lab"), reader.Dict{"WhitePoint": d65}} + // 0x80 0x99 0x59 decodes to L=50.2, a=20.0, b=-30.2 under the default. + samples := []byte{0x80, 0x99, 0x59} + implied := labImage(t, space, nil, samples) + spelled := labImage(t, space, nums(0, 100, -100, 100, -100, 100), samples) + if implied != spelled { + t.Errorf("the implied default (%v) and the same array written out (%v) differ", implied, spelled) + } + if implied.R < 100 || implied.B < 100 { + t.Errorf("a mid-lightness Lab sample came out as %v; the decode was read over [0, 1]", implied) + } +} + +func TestALabSpaceNarrowsItsAxesWithRange(t *testing.T) { + // /Range moves what a sample means, so the same bytes name a different + // colour. A malformed range -- a maximum below its minimum -- is not a + // narrower space, it is a file that got it wrong, and the default stands. + wide := reader.Array{reader.Name("Lab"), reader.Dict{"WhitePoint": d65}} + narrow := reader.Array{reader.Name("Lab"), + reader.Dict{"WhitePoint": d65, "Range": nums(-20, 20, -20, 20)}} + backwards := reader.Array{reader.Name("Lab"), + reader.Dict{"WhitePoint": d65, "Range": nums(20, -20, -20, 20)}} + samples := []byte{0x80, 0xff, 0x00} + a, b, c := labImage(t, wide, nil, samples), labImage(t, narrow, nil, samples), labImage(t, backwards, nil, samples) + if a == b { + t.Errorf("narrowing /Range changed nothing: %v", a) + } + if c != a { + t.Errorf("a backwards /Range was used (%v); the default should stand (%v)", c, a) + } +} + +func TestALabSpaceWithoutAWhitePointUsesTheEqualEnergyPoint(t *testing.T) { + // Lab has no device namesake to decline into. (1, 1, 1) makes the + // white-point multiplication the identity and is what poppler's own + // constructor uses, so a malformed file reads the same way in both. + // poppler draws Lab(50, 20, -30) in such a space as (131, 109, 171). + got := labImage(t, reader.Array{reader.Name("Lab"), reader.Dict{}}, nil, []byte{0x80, 0x99, 0x59}) + for i, pair := range [][2]uint8{{got.R, 131}, {got.G, 109}, {got.B, 171}} { + if d := int(pair[0]) - int(pair[1]); d > 2 || d < -2 { + t.Errorf("channel %d = %d, poppler says %d", i, pair[0], pair[1]) + } + } +} + +func TestADecodeArrayWithSomethingThatIsNotANumber(t *testing.T) { + // A /Decode entry that is a name rather than a number: the array cannot be + // read, so the samples fall back to fractions of their range. + space := reader.Array{reader.Name("Lab"), reader.Dict{"WhitePoint": d65}} + bad := reader.Array{reader.Real(0), reader.Real(100), reader.Real(-100), reader.Real(100), + reader.Real(-100), reader.Name("oops")} + got := labImage(t, space, bad, []byte{0x80, 0x99, 0x59}) + if got.A != 255 { + t.Errorf("a malformed /Decode dropped the picture: %v", got) + } +} diff --git a/go.mod b/go.mod index 76bf65f..741194a 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/go-pdfkit/render go 1.26.4 require ( - github.com/go-gfx/gfx v0.22.0 + github.com/go-gfx/gfx v0.23.0 github.com/go-opentype/fonts v0.9.0 github.com/go-opentype/opentype v0.12.0 github.com/go-pdfkit/reader v0.6.0 diff --git a/go.sum b/go.sum index b2fac18..bd69262 100644 --- a/go.sum +++ b/go.sum @@ -4,6 +4,8 @@ github.com/ajroetker/go-jpeg2000 v0.0.2 h1:ni8brffZrci4Kacx3nM5d92ipmTDfak84KgHY github.com/ajroetker/go-jpeg2000 v0.0.2/go.mod h1:7ld88W47lZy0x8gRQesRGAonDPOpr6ev8rckjCAfbzE= github.com/go-gfx/gfx v0.22.0 h1:ZyATWI4zs9lM+GF7FSJqinsvYhMriqdU7miSFLaArok= github.com/go-gfx/gfx v0.22.0/go.mod h1:5wUl8qCvaooyJ9Ly0oS2TLlawlvAp3kosrxh5kfxVTo= +github.com/go-gfx/gfx v0.23.0 h1:PdxREtW4NrMYEGzcnbILCLnE3xNwReXU4bFxQ02lKO4= +github.com/go-gfx/gfx v0.23.0/go.mod h1:5wUl8qCvaooyJ9Ly0oS2TLlawlvAp3kosrxh5kfxVTo= github.com/go-opentype/fonts v0.9.0 h1:slB6OB3riLyUPrOxqXe0s6/AzdenF1TDvCN8N87hhQk= github.com/go-opentype/fonts v0.9.0/go.mod h1:C6yQL2apHItfEZ5hztpsHF0S5mlX/hklLlq/Z5fRG/g= github.com/go-opentype/opentype v0.12.0 h1:wBlcDi+3ZaNZXEt5z+Ixr11/cYYwi5W+jX6yTl/qr1I= diff --git a/image.go b/image.go index f782b44..63a84ac 100644 --- a/image.go +++ b/image.go @@ -252,6 +252,19 @@ func (r *renderer) decodeArray(dict reader.Dict, sp *space, bpc int) func(c int, // An indexed image's samples are row numbers, not fractions. return func(_ int, raw uint32, _ int) float64 { return float64(raw) } } + if sp.labRange != nil { + // Lab is the one space whose default decode is not [0 1] per + // component: lightness runs to 100 and the two opponent axes over + // the space's own /Range. + lo := [3]float64{0, sp.labRange[0], sp.labRange[2]} + hi := [3]float64{100, sp.labRange[1], sp.labRange[3]} + return func(c int, raw uint32, _ int) float64 { + if c < 0 || c > 2 { + return float64(raw) / maxValue + } + return lo[c] + float64(raw)*(hi[c]-lo[c])/maxValue + } + } return func(_ int, raw uint32, _ int) float64 { return float64(raw) / maxValue } } bounds := make([]float64, 2*sp.components) diff --git a/space.go b/space.go index 47519a9..07efca9 100644 --- a/space.go +++ b/space.go @@ -25,6 +25,10 @@ type space struct { // under, for a Pattern space, is the space an uncoloured pattern's own // colour is given in. under *space + // labRange, for a Lab space, is the range of its two opponent axes. It is + // kept because an image with no /Decode array of its own decodes against + // it; see decodeArray. + labRange *[4]float64 } // The device spaces, which every file may use without saying anything first. @@ -133,7 +137,7 @@ func (r *renderer) colourSpaceArray(family reader.Name, arr reader.Array, resour case "CalGray": return r.calGraySpace(arr) case "Lab": - return labSpace() + return r.labSpace(arr) case "Indexed": return r.indexedSpace(arr, resources, depth) case "Separation", "DeviceN": @@ -179,16 +183,6 @@ func byComponents(n int) *space { return deviceRGB } -// labSpace reads three numbers as lightness and two opponent axes. Only the -// lightness is used, which is a grey of the right weight rather than the right -// colour — enough not to lose the mark, and honest about what it is. -func labSpace() *space { - return &space{name: "Lab", components: 3, convert: func(v []float64) color.RGBA { - g := byteOf(at(v, 0) / 100) - return color.RGBA{R: g, G: g, B: g, A: 255} - }} -} - // indexedSpace reads one number as a row of a table of colours. func (r *renderer) indexedSpace(arr reader.Array, resources reader.Dict, depth int) *space { if len(arr) < 4 { diff --git a/space_test.go b/space_test.go index b3255aa..64a7511 100644 --- a/space_test.go +++ b/space_test.go @@ -27,7 +27,12 @@ func TestNamedColourSpaces(t *testing.T) { // Full key ink on paper, which is not absolute black: (35,31,32) is // the SWOP key primary, and it is what poppler draws for 0 0 0 1 too. {"the device spaces by array", reader.Array{reader.Name("DeviceCMYK")}, "0 0 0 1 sc", color.RGBA{35, 31, 32, 255}}, - {"lightness and two axes", reader.Array{reader.Name("Lab"), reader.Dict{}}, "50 20 -30 sc", color.RGBA{128, 128, 128, 255}}, + // A positive a* is toward magenta and a negative b* toward blue, so + // this is a mid purple and NOT the grey of the same lightness the old + // stand-in drew. The value is poppler's: a one-pixel Lab image of + // exactly this colour, in a space with no /WhitePoint just as here, + // comes out of pdfimages as (131, 109, 171). + {"lightness and two axes", reader.Array{reader.Name("Lab"), reader.Dict{}}, "50 20 -30 sc", color.RGBA{131, 109, 171, 255}}, {"a spot colour at full strength", reader.Array{reader.Name("Separation"), reader.Name("Spot"), reader.Name("DeviceGray"), reader.Dict{}}, "1 sc", color.RGBA{0, 0, 0, 255}}, {"a spot colour at none", reader.Array{reader.Name("Separation"), reader.Name("Spot"), From 3331a6749c6aafebf2b34932ca5e6451b31bb2ba Mon Sep 17 00:00:00 2001 From: tannevaled Date: Thu, 10 Sep 2026 22:23:11 +0200 Subject: [PATCH 2/2] Drop an unreachable guard in Lab's decode decodeArray is called with the space's own component count, so the closure never sees an index outside the three it has. The rounded total read 100.0% while the function stood at 96.0%, which is why the check has to be per function -- as the gate does it -- and not on the summary line. --- image.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/image.go b/image.go index 63a84ac..8964639 100644 --- a/image.go +++ b/image.go @@ -256,12 +256,11 @@ func (r *renderer) decodeArray(dict reader.Dict, sp *space, bpc int) func(c int, // Lab is the one space whose default decode is not [0 1] per // component: lightness runs to 100 and the two opponent axes over // the space's own /Range. + // Three components, and the caller only ever asks for one of + // them: decodeArray is called with the space's own count. lo := [3]float64{0, sp.labRange[0], sp.labRange[2]} hi := [3]float64{100, sp.labRange[1], sp.labRange[3]} return func(c int, raw uint32, _ int) float64 { - if c < 0 || c > 2 { - return float64(raw) / maxValue - } return lo[c] + float64(raw)*(hi[c]-lo[c])/maxValue } }