Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plot: y-axis tick label falls off the plot when no plot title #676

Closed
sbinet opened this issue Feb 25, 2021 · 7 comments · Fixed by #708
Closed

plot: y-axis tick label falls off the plot when no plot title #676

sbinet opened this issue Feb 25, 2021 · 7 comments · Fixed by #708

Comments

@sbinet
Copy link
Member

sbinet commented Feb 25, 2021

probably another glyph-box issue.

with latest master (875edf3), the following code gives:

package main

import (
	"image/color"
	"log"

	"gonum.org/v1/plot"
	"gonum.org/v1/plot/plotter"
	"gonum.org/v1/plot/vg"
)

func main() {
	p := plot.New()
	p.X.Min = 0
	p.X.Max = 10
	p.Y.Min = 0
	p.Y.Max = 10

	f1 := plotter.NewFunction(func(x float64) float64 { return 5 })
	f1.LineStyle.Color = color.RGBA{R: 255, A: 255}

	f2 := plotter.NewFunction(func(x float64) float64 { return 6 })
	f2.LineStyle.Color = color.RGBA{B: 255, A: 255}

	p.Add(plotter.NewGlyphBoxes())
	p.Add(f1, f2)
	p.Add(plotter.NewGrid())

	p.Legend.Add("f1", f1)
	p.Legend.Add("f2", f2)
	p.Legend.Top = true

	err := p.Save(20*vg.Centimeter, 15*vg.Centimeter, "box.png")
	if err != nil {
		log.Fatalf("error: %+v", err)
	}
}

box

while with v0.8.1 gives:

box-ref

(notice how the text is chopped off for tick 10 on the y-axis. the f1 legend label as well.)

@sbinet
Copy link
Member Author

sbinet commented Mar 10, 2021

according to this gitsect session:

$> go get github.com/sbinet/img-diff@v0.1.0
$> cat > test-box.sh <<EOF
#!/bin/sh

set -x

REV=`git rev-parse --short HEAD`
go build -o run-box-$REV ./issue-box.go || exit 125
./run-box-$REV $REV

img-diff -batch ./box-$REV.png ./box-ref.png || exit 1
EOF

$> git bisect start 76bbcce v0.8.1
Bisecting: 9 revisions left to test after this (roughly 3 steps)
[b130901b6a489a108668e1b01043c24228ba1d9a] vg/vggio: enable checkplot test

$> git bisect run ./test-box.sh
running ./test-box.sh
++ git rev-parse --short HEAD
+ REV=b130901
+ go build -o run-box-b130901 ./issue-box.go
+ ./run-box-b130901 b130901
saving to "box-b130901.png"...
+ img-diff -batch ./box-b130901.png ./box-ref.png
diff=[1.7976931348623157e+308, 0]
Bisecting: 4 revisions left to test after this (roughly 2 steps)
[4ebc050d3b0eea646a0cd0cffd802edb214b8b13] vg: use x/image/font/{sfnt,opentype}
running ./test-box.sh
++ git rev-parse --short HEAD
+ REV=4ebc050
+ go build -o run-box-4ebc050 ./issue-box.go
+ ./run-box-4ebc050 4ebc050
saving to "box-4ebc050.png"...
+ img-diff -batch ./box-4ebc050.png ./box-ref.png
diff=[1.4348999293084747e-05, 0.9330436790328738]
+ exit 1
Bisecting: 2 revisions left to test after this (roughly 1 step)
[89f7b1173b4b779a0a5b3cf94f2f86031574cea5] all: correctly handle descent in glyphbox of text symbols
running ./test-box.sh
++ git rev-parse --short HEAD
+ REV=89f7b11
+ go build -o run-box-89f7b11 ./issue-box.go
+ ./run-box-89f7b11 89f7b11
saving to "box-89f7b11.png"...
+ img-diff -batch ./box-89f7b11.png ./box-ref.png
diff=[1.26689306058423e-05, 0.9330436790328738]
+ exit 1
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[35049c6da18799d0165f39edd525e2f8777bc484] vg/vgpdf: migrate to phpdave11/gofpdf
running ./test-box.sh
++ git rev-parse --short HEAD
+ REV=35049c6
+ go build -o run-box-35049c6 ./issue-box.go
+ ./run-box-35049c6 35049c6
saving to "box-35049c6.png"...
+ img-diff -batch ./box-35049c6.png ./box-ref.png
diff=[1.7976931348623157e+308, 0]
89f7b1173b4b779a0a5b3cf94f2f86031574cea5 is the first bad commit
commit 89f7b1173b4b779a0a5b3cf94f2f86031574cea5
Author: Sebastien Binet <binet@cern.ch>
Date:   Mon Dec 14 16:40:12 2020 +0100

    all: correctly handle descent in glyphbox of text symbols
    
    Fixes gonum/plot#649.

[...]
bisect run success

$> git bisect reset
Previous HEAD position was 35049c6 vg/vgpdf: migrate to phpdave11/gofpdf
HEAD is now at 76bbcce all: update reference files for x/image/font

89f7b11 is the commit introducing this weird behaviour.

@sbinet
Copy link
Member Author

sbinet commented Mar 10, 2021

actually that should be 4ebc050 (89f7b11 just introduced tighter glyph boxes. 4ebc050 is the one that cuts off the y-labels):

Bisecting: 0 revisions left to test after this (roughly 1 step)
[d53aaabfe205acf797f008fa46f44044c90de589] cmpimg,vg/vggio: implement a YIQ-based image difference
running ./test-box.sh
++ git rev-parse --short HEAD
+ REV=d53aaab
+ go build -o run-box-d53aaab ./issue-box.go
+ ./run-box-d53aaab d53aaab
saving to "box-d53aaab.png"...
+ img-diff -batch ./box-d53aaab.png ./box-ref.png
diff=[1.7976931348623157e+308, 0]
4ebc050d3b0eea646a0cd0cffd802edb214b8b13 is the first bad commit
commit 4ebc050d3b0eea646a0cd0cffd802edb214b8b13
Author: Sebastien Binet <binet@cern.ch>
Date:   Mon Dec 14 17:30:00 2020 +0100

    vg: use x/image/font/{sfnt,opentype}
    
    Updates gonum/plot#613.

 go.mod             |   7 +-
 go.sum             |  19 ++---
 vg/example_test.go |   4 +-
 vg/font.go         | 117 +++++++++++++++++++----------
 vg/font_test.go    | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 307 insertions(+), 53 deletions(-)
 create mode 100644 vg/font_test.go
bisect run success

box-4ebc050

@sbinet
Copy link
Member Author

sbinet commented Mar 10, 2021

using this simple test file:

//go:build ignore
// +build ignore

package main

import (
	t "log"
	"testing"

	. "gonum.org/v1/plot/vg"
)

func main() {
	for _, tc := range []struct {
		font string
		want map[Length]FontExtents
	}{
		{
			font: "Times-Roman",
			want: map[Length]FontExtents{
				10: FontExtents{
					Ascent:  8.9111328125,
					Descent: -2.1630859375,
					Height:  11.4990234375,
				},
				12: FontExtents{
					Ascent:  10.693359375,
					Descent: -2.595703125,
					Height:  13.798828125,
				},
				24: FontExtents{
					Ascent:  21.38671875,
					Descent: -5.19140625,
					Height:  27.59765625,
				},
			},
		},
	} {
		for _, size := range []Length{10, 12, 24} {
			fnt, err := MakeFont(tc.font, size)
			if err != nil {
				t.Fatal(err)
			}

			got := fnt.Extents()
			if got, want := got, tc.want[size]; got != want {
				t.Fatalf(
					"invalid font extents for %q, size=%v:\ngot= %#v\nwant=%#v",
					tc.font, size, got, want,
				)
			}
		}
	}
}

whose values were obtained from:

$> git show 4ebc050d3b0eea646a0cd0cffd802edb214b8b13:vg/font_test.go > tst-fnt.go

and that scaffolding test script:

#!/bin/sh

set -e

REV=$1
echo "checkout ${REV}..."
git checkout $REV
go run ./tst-fnt.go || exit 1

I get:

$>  ./fnt-test.sh v0.8.1
checkout v0.8.1...
HEAD is now at a51a196 plot: fix handling of font descents
2021/03/10 17:26:49 invalid font extents for "Times-Roman", size=10:
got= vg.FontExtents{Ascent:9.814453125, Descent:-3.0322265625, Height:12.8466796875}
want=vg.FontExtents{Ascent:8.9111328125, Descent:-2.1630859375, Height:11.4990234375}
exit status 1

ie: it would seem one of the fundamental core difference boils down to how x/image/font/sfnt and golang/freetype/truetype extract font metrics.

the want metrics were obtained w/ sfnt, the got ones obtained with v0.8.1 (ie: freetype/truetype).

@sbinet
Copy link
Member Author

sbinet commented Mar 10, 2021

the following program:

package main

import (
	"fmt"
	"log"

	"github.com/go-fonts/liberation/liberationsansregular"
	"github.com/golang/freetype/truetype"
	"golang.org/x/image/font"
	"golang.org/x/image/font/opentype"
	"golang.org/x/image/font/sfnt"
	"golang.org/x/image/math/fixed"
)

const (
	defaultHinting = font.HintingNone
	defaultDPI     = 72
	fntSize        = 10.0
)

func main() {
	log.SetFlags(0)
	log.SetPrefix("fnter: ")

	raw := liberationsansregular.TTF

	ttf, err := truetype.Parse(raw)
	if err != nil {
		log.Fatalf("could not parse ttf font: %+v", err)
	}
	otf, err := opentype.Parse(raw)
	if err != nil {
		log.Fatalf("could not parse otf font: %+v", err)
	}

	log.Printf("truetype: units-per-em=%v", ttf.FUnitsPerEm())
	log.Printf("opentype: units-per-em=%v", otf.UnitsPerEm())

	log.Printf("truetype: ext=%#v", ttfExtents(ttf))
	log.Printf("opentype: ext=%#v", otfExtents(otf))

	log.Printf("truetype: bnds=%#v", ttfBounds(ttf))
	log.Printf("opentype: bnds=%#v", otfBounds(otf))

	log.Printf("truetype: met=%#v", ttfMetrics(ttf))
	log.Printf("opentype: met=%#v", otfMetrics(otf))
	log.Printf("delta(t-o):   %#v", deltaMet(ttf, otf))

	log.Printf("truetype: line-gap=%d", ttfLineGap(ttf))
	log.Printf("opentype: line-gap=%d", otfLineGap(otf))
}

type Extents struct {
	Ascent  float64
	Descent float64
	Height  float64
}

func ttfExtents(fnt *truetype.Font) Extents {
	var (
		pem    = fnt.FUnitsPerEm()
		bounds = fnt.Bounds(fixed.Int26_6(pem))
		scale  = fntSize / float64(pem)
	)
	return Extents{
		Ascent:  float64(bounds.Max.Y) * scale,
		Descent: float64(bounds.Min.Y) * scale,
		Height:  float64(bounds.Max.Y-bounds.Min.Y) * scale,
	}
}

func otfExtents(fnt *opentype.Font) Extents {
	var (
		buf  sfnt.Buffer
		ppem = fixed.Int26_6(fnt.UnitsPerEm())
	)

	met, err := fnt.Metrics(&buf, ppem, defaultHinting)
	if err != nil {
		panic(fmt.Errorf("could not extract font extents: %v", err))
	}
	scale := fntSize / float64(ppem)
	return Extents{
		Ascent:  float64(met.Ascent) * scale,
		Descent: float64(met.Descent) * scale,
		Height:  float64(met.Height) * scale,
	}
}

func ttfBounds(fnt *truetype.Font) fixed.Rectangle26_6 {
	var (
		pem    = fnt.FUnitsPerEm()
		bounds = fnt.Bounds(fixed.Int26_6(pem))
	)
	return bounds
}

func otfBounds(fnt *opentype.Font) fixed.Rectangle26_6 {
	var (
		buf  sfnt.Buffer
		ppem = fixed.Int26_6(fnt.UnitsPerEm())
	)

	bounds, err := fnt.Bounds(&buf, ppem, defaultHinting)
	if err != nil {
		panic(fmt.Errorf("could not extract font bounds: %v", err))
	}
	return bounds
}

func ttfMetrics(fnt *truetype.Font) font.Metrics {
	face := truetype.NewFace(fnt, &truetype.Options{
		Size:    fntSize,
		DPI:     defaultDPI,
		Hinting: defaultHinting,
	})
	return face.Metrics()
}

func otfMetrics(fnt *opentype.Font) font.Metrics {
	face, err := opentype.NewFace(fnt, &opentype.FaceOptions{
		Size:    fntSize,
		DPI:     defaultDPI,
		Hinting: defaultHinting,
	})
	if err != nil {
		panic("no face: " + err.Error())
	}
	defer face.Close()
	return face.Metrics()
}

func ttfLineGap(fnt *truetype.Font) fixed.Int26_6 {
	met := ttfMetrics(fnt)
	return met.Height - met.Ascent + met.Descent
}

func otfLineGap(fnt *opentype.Font) fixed.Int26_6 {
	met := otfMetrics(fnt)
	return met.Height - met.Ascent - met.Descent
}

func deltaMet(f1 *truetype.Font, f2 *opentype.Font) font.Metrics {
	m1 := ttfMetrics(f1)
	m2 := otfMetrics(f2)
	return font.Metrics{
		Height:     m1.Height - m2.Height,
		Ascent:     m1.Ascent - m2.Ascent,
		Descent:    m1.Descent - m2.Descent,
		XHeight:    m1.XHeight - m2.XHeight,
		CapHeight:  m1.CapHeight - m2.CapHeight,
		CaretSlope: m1.CaretSlope.Sub(m2.CaretSlope),
	}
}

prints:

fnter: truetype: units-per-em=2048
fnter: opentype: units-per-em=2048
fnter: truetype: ext=main.Extents{Ascent:9.7998046875, Descent:-3.0322265625, Height:12.83203125}
fnter: opentype: ext=main.Extents{Ascent:9.052734375, Descent:2.119140625, Height:11.4990234375}
fnter: truetype: bnds=fixed.Rectangle26_6{Min:fixed.Point26_6{X:-1114, Y:-621}, Max:fixed.Point26_6{X:2666, Y:2007}}
fnter: opentype: bnds=fixed.Rectangle26_6{Min:fixed.Point26_6{X:-1114, Y:-2007}, Max:fixed.Point26_6{X:2666, Y:621}}
fnter: truetype: met=font.Metrics{Height:640, Ascent:580, Descent:136, XHeight:0, CapHeight:0, CaretSlope:image.Point{X:0, Y:0}}
fnter: opentype: met=font.Metrics{Height:736, Ascent:579, Descent:136, XHeight:338, CapHeight:440, CaretSlope:image.Point{X:0, Y:1}}
fnter: delta(t-o):   font.Metrics{Height:-96, Ascent:1, Descent:0, XHeight:-338, CapHeight:-440, CaretSlope:image.Point{X:0, Y:-1}}
fnter: truetype: line-gap=196
fnter: opentype: line-gap=21

@sbinet
Copy link
Member Author

sbinet commented Mar 24, 2021

FYI, I've filed golang/go#45176.

we should probably add some instrumentation in gonum/plot to also draw "glyph-boxes" around text in other parts of a plot (ie: not only inside the DataCanvas area but also for the Title, axes' labels, ticks, etc...)


If I am not mistaken (and barring any bug in x/image), what we had before migrating to x/image/font/{opentype,sfnt} was working because we had implicit "extra space" coming from the FontMetrics.
but it seems to me we didn't put any logic to insert explicit space between, say, the bottom of an X-axis tick and the top of the bounding box of a character.

@sbinet
Copy link
Member Author

sbinet commented Jun 25, 2021

circling back to this issue, I've probably found out the root cause of it.

here is my new test program:

package main

import (
	"image/color"
	"log"
	"os"

	"gonum.org/v1/plot"
	"gonum.org/v1/plot/plotter"
	"gonum.org/v1/plot/vg"
	"gonum.org/v1/plot/vg/draw"
	"gonum.org/v1/plot/vg/vgimg"
)

func main() {
	p := plot.New()
	p.X.Min = 0
	p.X.Max = 10
	p.Y.Min = 0
	p.Y.Max = 10

	f1 := plotter.NewFunction(func(x float64) float64 { return 5 })
	f1.LineStyle.Color = color.RGBA{R: 255, A: 255}

	f2 := plotter.NewFunction(func(x float64) float64 { return 6 })
	f2.LineStyle.Color = color.RGBA{B: 255, A: 255}

	p.Add(plotter.NewGlyphBoxes())
	p.Add(f1, f2)
	p.Add(plotter.NewGrid())

	p.Legend.Add("f1", f1)
	p.Legend.Add("f2", f2)
	p.Legend.Top = true

	c := vgimg.PngCanvas{
		Canvas: vgimg.New(20*vg.Centimeter, 15*vg.Centimeter),
	}

	d := draw.New(c)
	p.Draw(d)
	p.DrawGlyphBoxes(&d)

	f, err := os.Create("box.png")
	if err != nil {
		log.Fatalf("error: %+v", err)
	}
	defer f.Close()

	_, err = c.WriteTo(f)
	if err != nil {
		log.Fatalf("error: %+v", err)
	}

	err = f.Close()
	if err != nil {
		log.Fatalf("error: %+v", err)
	}
}

if I add the following to plot.go:

diff --git a/plot.go b/plot.go
index 09c9ec7..9edff00 100644
--- a/plot.go
+++ b/plot.go
@@ -382,6 +382,10 @@ func (p *Plot) GlyphBoxes(*Plot) (boxes []GlyphBox) {
                        boxes = append(boxes, b)
                }
        }
+       if true {
+               y := verticalAxis{p.Y}
+               boxes = append(boxes, y.GlyphBoxes(p)...)
+       }
        return
 }

I get:

gonum/plot-v0.8.1

box

gonum/plot-v0.9

box

(the glyphboxes are shifted to the right b/c they were drawn (presumably) on the data canvas, but that's irrelevant here: we are just interested in their y coordinates)

i.e.: with v0.9, the boxes are shifted toward the bottom.
if one applies:

diff --git a/axis.go b/axis.go
index 3b1ab10..559c451 100644
--- a/axis.go
+++ b/axis.go
@@ -398,6 +404,7 @@ func (a verticalAxis) draw(c draw.Canvas) {
 // GlyphBoxes returns the GlyphBoxes for the tick labels
 func (a verticalAxis) GlyphBoxes(*Plot) []GlyphBox {
        var boxes []GlyphBox
+       ext := a.Tick.Label.FontExtents()
+       desc := ext.Height - ext.Ascent // descent + linegap
        for _, t := range a.Tick.Marker.Ticks(a.Min, a.Max) {
                if t.IsMinor() {
                        continue
@@ -406,6 +413,10 @@ func (a verticalAxis) GlyphBoxes(*Plot) []GlyphBox {
                        Y:         a.Norm(t.Value),
                        Rectangle: a.Tick.Label.Rectangle(t.Label),
                }
+               if true {
+                       box.Rectangle.Min.Y += desc
+                       box.Rectangle.Max.Y += desc
+               }
                boxes = append(boxes, box)
        }
        return boxes

we recover the "original behaviour":
box

I guess it's because somewhere between package plot, package plot/text and others, there's a convention mismatch between whether the coordinates of a given text string are to be interpreted as (0,0) at the baseline, or with descent(+linegap) included.
(or, rather, the coordinates of the returned glyphbox).

@kortschak would you agree with my assessment?
in any case, I think we probably need to clarify a bit the documentation of:

  • text.Handler.{Box,Draw}
  • text.Style.{Height,Rectangle}
  • vg.Canvas.FillString
  • vg/draw.Canvas.FillText
  • others?

to mention the issue of fonts' baseline, linegap, descent and cie.

@kortschak
Copy link
Member

That's a good assessment. It would certainly be worthwhile making an effort to get if not a single coordinate system and least a consistent reference to a single coordinate system in the documentation.

Probably also good to get the glyph boxes to draw in the right place.

sbinet added a commit to sbinet-gonum/plot that referenced this issue Jun 25, 2021
Fixes gonum#676.

Signed-off-by: Sebastien Binet <binet@cern.ch>
sbinet added a commit to sbinet-gonum/plot that referenced this issue Jun 25, 2021
Fixes gonum#676.

Signed-off-by: Sebastien Binet <binet@cern.ch>
sbinet added a commit to sbinet-gonum/plot that referenced this issue Jun 25, 2021
Fixes gonum#676.

Signed-off-by: Sebastien Binet <binet@cern.ch>
sbinet added a commit to sbinet-gonum/plot that referenced this issue Jun 25, 2021
Fixes gonum#676.

Signed-off-by: Sebastien Binet <binet@cern.ch>
sbinet added a commit to sbinet-gonum/plot that referenced this issue Aug 4, 2021
Fixes gonum#676.

Signed-off-by: Sebastien Binet <binet@cern.ch>
sbinet added a commit to sbinet-gonum/plot that referenced this issue Aug 4, 2021
Fixes gonum#676.

Signed-off-by: Sebastien Binet <binet@cern.ch>
sbinet added a commit to sbinet-gonum/plot that referenced this issue Aug 4, 2021
… glyphboxes

Fixes gonum#676.

Signed-off-by: Sebastien Binet <binet@cern.ch>
sbinet added a commit to sbinet-gonum/plot that referenced this issue Aug 6, 2021
… glyphboxes

Fixes gonum#676.

Signed-off-by: Sebastien Binet <binet@cern.ch>
sbinet added a commit that referenced this issue Aug 8, 2021
… glyphboxes

Fixes #676.

Signed-off-by: Sebastien Binet <binet@cern.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants