vg/vgimg: migrate to fogleman/gg

[sbinet]

Fixes #470.

Please take a look.

[sbinet]

it's a WIP:

• I haven't checked whether the coordinates match,
• I haven't adapted the fonts system (we'll probably have to migrate to golang.org/x/image/font.Face somehow)
• handling of DPI
• other stuff I've overlooked.

[sbinet]

PTAL.

it seems the rendering of fonts is somewhat different (a bit fuzzier) than what we had with draw2d...
any idea?

[sbinet]

@fogleman do you know why text would appear fuzzier when using gg (that uses golang/truetype via x/image/font.Face) instead of llgcode/draw2d (that uses directly golang/truetype) ?

see:

• https://github.com/gonum/plot/pull/471/files?short_path=4449153#diff-44491532b0a86c394782d54293efbd1e (i.e. the plot title, the axes' labels)

[fogleman]

I suspect you are translating some non-integer amount before drawing the text.

[fogleman]

(That said, it should be possible to draw nice looking text even in that case, so I will look into this.)

[sbinet]

thx for looking into it!

I think I made progress: it seems to come from different default DPIs.
originally, vg/vgimg used a default DPI of 96 (but that could be modified and communicated to draw2d)

it seems gg uses 72 but doesn't expose any API to modify that.

[sbinet]

here is a simpler reproducer:

package main

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

	"github.com/fogleman/gg"
	"gonum.org/v1/plot/vg"
	"gonum.org/v1/plot/vg/vgimg"
)

const (
	fontName = "Times-Roman"
)

func main() {
	f72()
	f96()
}

func f72() {
	c := vgimg.NewWith(
		vgimg.UseDPI(72),
		vgimg.UseWH(200, 200),
		vgimg.UseBackgroundColor(color.Transparent),
	)

	ft, err := vg.MakeFont(fontName, 12)
	if err != nil {
		log.Fatal(err)
	}
	log.Printf("f2: %v", c.Image().Bounds())

	c.FillString(ft, vg.Point{X: 50, Y: 100}, "Hello 72")

	pp := vgimg.PngCanvas{c}
	o, err := os.Create("o-f2-72.png")
	if err != nil {
		log.Fatal(err)
	}
	pp.WriteTo(o)
	o.Close()
}

func f96() {
	c := vgimg.NewWith(
		vgimg.UseDPI(96),
		vgimg.UseWH(200, 200),
		vgimg.UseBackgroundColor(color.Transparent),
	)

	ft, err := vg.MakeFont(fontName, 12)
	if err != nil {
		log.Fatal(err)
	}
	log.Printf("f2: %v", c.Image().Bounds())

	c.FillString(ft, vg.Point{X: 50, Y: 100}, "Hello 96")

	pp := vgimg.PngCanvas{c}
	o, err := os.Create("o-f2-96.png")
	if err != nil {
		log.Fatal(err)
	}
	pp.WriteTo(o)
	o.Close()
}

[sbinet]

and an even simpler one:

package main

import (
	"log"

	"github.com/fogleman/gg"
	"gonum.org/v1/plot/vg"
)

var (
	pt = vg.Point{X: 20, Y: 60}
)

func main() {
	ctx := gg.NewContext(100, 100)
	ctx.SetRGB(1, 1, 1)
	ctx.Clear()

	ctx.InvertY()

	ctx.SetRGB(0, 0, 0)
	ft, err := vg.MakeFont("Times-Roman", 12)
	if err != nil {
		log.Fatal(err)
	}
	ctx.SetFontFace(ft.FontFace(96))

	x1 := pt.X.Dots(96) // == 26.666666666666668
	y1 := 20.0
	x2 := pt.X.Dots(96)
	y2 := 10.0

	{
		ctx.Push()
		ctx.InvertY()
		ctx.DrawString("Hello+96", x1, y1)
		ctx.Pop()
	}

	{
		ctx.Push()
		ctx.Translate(x2, y2)
		ctx.Scale(1, -1)
		ctx.DrawString("Hello-96", 0, 0)
		ctx.Pop()
	}

	ctx.SavePNG("out.png")
}

with the following output:

so, indeed something related with non-integer-ness.

I've modified the vgimg.Canvas.FillString method like so:

diff --git a/vg/vgimg/vgimg.go b/vg/vgimg/vgimg.go
index a12a464..098b6a4 100644
--- a/vg/vgimg/vgimg.go
+++ b/vg/vgimg/vgimg.go
@@ -297,9 +297,12 @@ func (c *Canvas) FillString(font vg.Font, pt vg.Point, str string) {
 
        c.ctx.SetFontFace(font.FontFace(c.DPI()))
 
-       c.ctx.Translate(pt.X.Dots(c.DPI()), pt.Y.Dots(c.DPI()))
-       c.ctx.Scale(1, -1)
-       c.ctx.DrawString(str, 0, 0)
+       x := pt.X.Dots(c.DPI())
+       y := pt.Y.Dots(c.DPI())
+       h := c.h.Dots(c.DPI())
+
+       c.ctx.InvertY()
+       c.ctx.DrawString(str, x, h-y)
 }
 
 // DrawImage implements the vg.Canvas.DrawImage method.

it seems to give better results...
(thanks for being my rubber duck :P)

[kortschak]

Does this fix #231?

[sbinet]

Does this fix #231?

here is the plot with draw2d:

and here it is with gg:

I guess I can add Fixes gonum/plot#231 ?

[sbinet]

actually, as I wrote in the last CL:

    vg/vgimg: handle DPI for fogleman/gg backend
    
    This CL adds consistent handling of DPI for the fogleman/gg backend.
    The old backend required to be communicated the current DPI for the
    image.
    This new backend does not, as the DPI is only needed for the font.
    gg handles fonts via golang.org/x/image/font.Face that embeds the DPI
    upon creation of that font.
    
    The vg/vgimg.Canvas carries its own DPI information and propagates it
    correctly upon font registration and creation.
    This evacuates the need to communicate the current DPI to gg.

so we don't even need to modify fogleman/gg.Context's API.

[sbinet]

PTAL.

[kortschak]

#231 talks about dashed line strokes. The gg example doesn't have any dashing AFAICS. Does it support it?

[fogleman]

https://godoc.org/github.com/fogleman/gg#Context.SetDash

[kortschak]

Thanks, @fogleman.

[sbinet]

and here is how I converted to the gg api:

https://github.com/gonum/plot/pull/471/files#diff-3fc76b9160b7dbc0faf71918da9adea0R192

[kortschak]

OK, the offset doesn't have a meaning in the gg SetDash function AFAICS. The copy should just be of the ds slice.

@fogleman Is there a way of specify an offset into the dash sequence?

[fogleman]

There isn't. 😢 But I should be able to add that.

[kortschak]

Thanks. So, @sbinet, in the first instance just drop the offs parameter and copy the ds into dashes.

[sbinet]

ok. except for:

• https://github.com/gonum/plot/pull/471/files?short_path=37f0b2b#diff-37f0b2b717e1613dd655b689667b4eb4
• https://github.com/gonum/plot/pull/471/files?short_path=be63a62#diff-be63a62daebdea2ba8d92058785fcbf8

differences are now somewhat "in the noise."

I've created fogleman/gg#64 to track the dash-offset feature.

[sbinet]

@ctessum would have to comment on whether the new output of the polygon_holes.png is the expected one (the other backends (PDF, SVG) seem to agree with the new PNG rendering.)

[ctessum]

I think the polygon_holes change is fine. As of now, the winding order is backend-dependent, but if this makes all of the backends agree, that is probably an improvement.

[kortschak]

What's the problem with https://github.com/gonum/plot/pull/471/files?short_path=be63a62#diff-be63a62daebdea2ba8d92058785fcbf8?

[kortschak]

Small query then LGTM.

[sbinet]

The rotated text in https://github.com/gonum/plot/pull/471/files?short_path=be63a62#diff-be63a62daebdea2ba8d92058785fcbf8 is a bit fuzzy.

[kortschak]

Ah, yes. Presumably upstream will eventually fix that. I prefer fuzz though to what we have at the moment.

[sbinet]

PTAL

[kortschak]

You may want to rebase onto master to avoid the coveralls failure that will otherwise happen.

[sbinet]

done.

[sbinet]

could you remind me of the rule for "large-ish" PRs like that (that consist of many commits) ?
do we prefer to squash+merge into a single commit, or rebase+merge ?
(perhaps something to pin down in CONTRIBUTING.md...)

[kortschak]

This is all fairly obviously one thing. So I would squash it and tidy up the commit message body and add any notes you think are relevant.