From c915305328052ba3d13baef399626423587bb70b Mon Sep 17 00:00:00 2001
From: Mats Linander <matslina@gmail.com>
Date: Tue, 27 Oct 2020 10:20:40 -0400
Subject: [PATCH] fewer bugs in Plot please

This should improve handling of Y and X limits, as well as make
the translation from data to canvas coordinates more predictable.
Adding a bunch of tests as well.
---
 render/plot.go      |  91 +++++++---
 render/plot_test.go | 393 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 426 insertions(+), 58 deletions(-)

diff --git a/render/plot.go b/render/plot.go
index 7fdff1d094..2bd86c7132 100644
--- a/render/plot.go
+++ b/render/plot.go
@@ -36,17 +36,21 @@ type Plot struct {
 	YLimMin *float64
 	YLimMax *float64
 
-	// If true, also pain surface between line and X-axis
+	// If true, also paint surface between line and X-axis
 	Fill bool
 
 	invThreshold int
 }
 
-// translatePoints() maps the points in X and Y to fit in Width x
-// Height pixels. it sets invThreshold to the height corresponding to
-// Y=0.
-func (p *Plot) translatePoints() []PathPoint {
-	// Find min/max X and Y
+// Computes X and Y limits
+func (p *Plot) computeLimits() (float64, float64, float64, float64) {
+
+	// If all limits are set by user, no computation is required
+	if p.XLimMin != nil && p.XLimMax != nil && p.YLimMin != nil && p.YLimMax != nil {
+		return *p.XLimMin, *p.XLimMax, *p.YLimMin, *p.YLimMax
+	}
+
+	// Otherwise we'll need min/max of X and Y
 	minX, maxX, minY, maxY := p.X[0], p.X[0], p.Y[0], p.Y[0]
 	for i := 1; i < len(p.X); i++ {
 		if p.X[i] < minX {
@@ -63,38 +67,75 @@ func (p *Plot) translatePoints() []PathPoint {
 		}
 	}
 
-	// Xlim and YLim simply overrides actual min/max
+	// Limits not set by user will default to the min/max of the
+	// data, so that it all fits on canvas.
+	xLimMin := minX
+	xLimMax := maxX
+	yLimMin := minY
+	yLimMax := maxY
 	if p.XLimMin != nil {
-		minX = *p.XLimMin
+		xLimMin = *p.XLimMin
 	}
 	if p.XLimMax != nil {
-		maxX = *p.XLimMax
+		xLimMax = *p.XLimMax
 	}
 	if p.YLimMin != nil {
-		minY = *p.YLimMin
+		yLimMin = *p.YLimMin
 	}
 	if p.YLimMax != nil {
-		maxY = *p.YLimMax
+		yLimMax = *p.YLimMax
 	}
 
-	// Add a small epsilon to maxY so normalization interval
-	// becomes half open. This in turn keeps points at the maximum
-	// Y-value from being mapped to p.Height, which would be out
-	// of bounds.
-	epsilon := (maxY - minY) / (float64(p.Height) * 100)
-	maxY += epsilon
+	// The inferred limits can be non-sensical if user provides
+	// only the min or max of a limit. In these cases, we take the
+	// provided limit and add an arbitraty +-0.5 to create limits
+	// that result in all points displayed "off-screen".
+	if xLimMax < xLimMin {
+		if p.XLimMin == nil {
+			xLimMin = xLimMax - 0.5
+		} else {
+			xLimMax = xLimMin + 0.5
+		}
+	}
+	if yLimMax < yLimMin {
+		if p.YLimMin == nil {
+			yLimMin = yLimMax - 0.5
+		} else {
+			yLimMax = yLimMin + 0.5
+		}
+	}
 
-	// Normalize to [0,1) and compute pixel position
+	// If all X or all Y are equal, then the default limits would
+	// have min==max, which is non-sensical.
+	if xLimMin == xLimMax {
+		// Place points furthest left on canvas
+		xLimMin = minX
+		xLimMax = minX + 0.5
+	}
+	if yLimMin == yLimMax {
+		// Place points in vertical center
+		yLimMin = minY - 0.5
+		yLimMax = minY + 0.5
+	}
+
+	return xLimMin, xLimMax, yLimMin, yLimMax
+}
+
+// Maps the points in X and Y to positions on the canvas
+func (p *Plot) translatePoints() []PathPoint {
+	xLimMin, xLimMax, yLimMin, yLimMax := p.computeLimits()
+
+	// Translate
 	points := make([]PathPoint, len(p.X))
 	for i := 0; i < len(p.X); i++ {
-		nX := (p.X[i] - minX) / (maxX - minX)
-		nY := (p.Y[i] - minY) / (maxY - minY)
-		points[i].X = int(math.Floor(nX * float64(p.Width)))
-		points[i].Y = p.Height - 1 - int(math.Floor(nY*float64(p.Height)))
+		nX := (p.X[i] - xLimMin) / (xLimMax - xLimMin)
+		nY := (p.Y[i] - yLimMin) / (yLimMax - yLimMin)
+		points[i] = PathPoint{
+			X: int(math.Round(nX * float64(p.Width-1))),
+			Y: p.Height - 1 - int(math.Round(nY*float64(p.Height-1))),
+		}
 	}
-
-	// In the same way, translate Y=0
-	p.invThreshold = p.Height - 1 - int(math.Floor((0-minY/(maxY-minY))*float64(p.Height)))
+	p.invThreshold = p.Height - 1 - int(math.Round(((0-yLimMin)/(yLimMax-yLimMin))*float64(p.Height-1)))
 
 	return points
 }
diff --git a/render/plot_test.go b/render/plot_test.go
index b223151f20..a11e1108d4 100644
--- a/render/plot_test.go
+++ b/render/plot_test.go
@@ -8,13 +8,208 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-// Warning: The expected results below are really just copy-paste of
-// the actual output of Plot when it was in a state that "looked
-// ok". I tried to get this thing to be super predictable and pixel
-// perfect, but it's really tricky with a) floating point math and b)
-// Bresenham's line drawing algorithm, so this will have to do.
+func TestPlotComputeLimits(t *testing.T) {
+	p := Plot{
+		X: []float64{3.14, 3.56, 3.9},
+		Y: []float64{1.62, 2.7, 2.9},
+	}
+
+	check := func(xMin, xMax, yMin, yMax float64) {
+		xA, xB, yA, yB := p.computeLimits()
+		assert.Equal(t, xMin, xA)
+		assert.Equal(t, xMax, xB)
+		assert.Equal(t, yMin, yA)
+		assert.Equal(t, yMax, yB)
+	}
+
+	// Without any limits set, data's min and max are used
+	check(3.14, 3.9, 1.62, 2.9)
+
+	// XLimMin below, within and above data
+	p.XLimMin = new(float64)
+	*p.XLimMin = 3.0
+	check(3.0, 3.9, 1.62, 2.9)
+	*p.XLimMin = 3.2
+	check(3.2, 3.9, 1.62, 2.9)
+	*p.XLimMin = 4.0
+	check(4.0, 4.5, 1.62, 2.9)
+
+	// XLimMax above, within and below data
+	p.XLimMin = nil
+	p.XLimMax = new(float64)
+	*p.XLimMax = 4.1
+	check(3.14, 4.1, 1.62, 2.9)
+	*p.XLimMax = 3.2
+	check(3.14, 3.2, 1.62, 2.9)
+	*p.XLimMax = -17
+	check(-17.5, -17, 1.62, 2.9)
+
+	// YLimMin below, within and above data
+	p.XLimMax = nil
+	p.YLimMin = new(float64)
+	*p.YLimMin = 1.0
+	check(3.14, 3.9, 1, 2.9)
+	*p.YLimMin = 2.0
+	check(3.14, 3.9, 2.0, 2.9)
+	p.YLimMin = new(float64)
+	*p.YLimMin = 3.0
+	check(3.14, 3.9, 3.0, 3.5)
+
+	// YLimMax above, within and below data
+	p.YLimMin = nil
+	p.YLimMax = new(float64)
+	*p.YLimMax = 3.14
+	check(3.14, 3.9, 1.62, 3.14)
+	*p.YLimMax = 2.0
+	check(3.14, 3.9, 1.62, 2.0)
+	*p.YLimMax = 1.0
+	check(3.14, 3.9, 0.5, 1.0)
+
+	// All limits in conjunction
+	p.XLimMin = new(float64)
+	p.XLimMax = new(float64)
+	p.YLimMin = new(float64)
+	p.YLimMax = new(float64)
+	*p.XLimMin = 3.0
+	*p.XLimMax = 4.0
+	*p.YLimMin = 1.0
+	*p.YLimMax = 3.0
+	check(3.0, 4.0, 1.0, 3.0)
+	*p.XLimMin = 3.3
+	*p.XLimMax = 3.4
+	*p.YLimMin = 2.1
+	*p.YLimMax = 2.2
+	check(3.3, 3.4, 2.1, 2.2)
+
+	// No limits with single Y value centers vertically
+	p.XLimMin = nil
+	p.XLimMax = nil
+	p.YLimMin = nil
+	p.YLimMax = nil
+	p.Y = []float64{3.14, 3.14, 3.14}
+	check(3.14, 3.9, 3.14-0.5, 3.14+0.5)
+
+	// No limits with single X value places points on left hand
+	// side
+	p.X = []float64{2, 2, 2}
+	check(2, 2.5, 3.14-0.5, 3.14+0.5)
+
+}
+
+// Tests of the internal translatePoints() method
+func TestPlotTranslatePoints(t *testing.T) {
+	p := Plot{
+		Width:  10,
+		Height: 10,
+	}
+
+	p.X = []float64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}
+	p.Y = []float64{0, 2, 4, 6, 8, 10, 12, 14, 16, 18}
+	assert.Equal(t, []PathPoint{
+		PathPoint{0, 9},
+		PathPoint{1, 8},
+		PathPoint{2, 7},
+		PathPoint{3, 6},
+		PathPoint{4, 5},
+		PathPoint{5, 4},
+		PathPoint{6, 3},
+		PathPoint{7, 2},
+		PathPoint{8, 1},
+		PathPoint{9, 0},
+	}, p.translatePoints())
+	assert.Equal(t, 9, p.invThreshold)
+
+	// Zoom in with XLim/YLim so that half the points fall outside
+	// of view.
+	p.XLimMin = new(float64)
+	*p.XLimMin = 2
+	p.XLimMax = new(float64)
+	*p.XLimMax = 6
+	p.YLimMin = new(float64)
+	*p.YLimMin = 4
+	p.YLimMax = new(float64)
+	*p.YLimMax = 12
+
+	// The points with X=2,3,4,5,6 will be mapped onto the 10x10
+	// canvas. The lowest falls on 0 and the highest on 9. Since
+	// they're equidistant, the stride between them must be 9/4 = 2.25.
+	assert.Equal(t, []PathPoint{
+		PathPoint{-5, 14}, // -4.5
+		PathPoint{-2, 11}, // -2.25
+		PathPoint{0, 9},   // 0
+		PathPoint{2, 7},   // 2.25
+		PathPoint{5, 4},   // 4.5
+		PathPoint{7, 2},   // 6.75
+		PathPoint{9, 0},   // 9
+		PathPoint{11, -2}, // 11.25
+		PathPoint{14, -5}, // 13.5
+		PathPoint{16, -7}, // 15.75
+	}, p.translatePoints())
+	assert.Equal(t, 14, p.invThreshold)
+}
+
+func TestPlotFlatLine(t *testing.T) {
+	ic := ImageChecker{
+		palette: map[string]color.RGBA{
+			"1": color.RGBA{0xff, 0xff, 0xff, 0xff},
+			".": color.RGBA{0, 0, 0, 0},
+		},
+	}
+
+	// Flatline
+	p := Plot{
+		Width:  10,
+		Height: 5,
+		X:      []float64{0, 9},
+		Y:      []float64{47, 47},
+	}
+	assert.Equal(t, nil, ic.Check([]string{
+		"..........",
+		"..........",
+		"1111111111",
+		"..........",
+		"..........",
+	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
+
+	// Extend view to the eft
+	p.XLimMin = new(float64)
+	*p.XLimMin = -10
+	assert.Equal(t, nil, ic.Check([]string{
+		"..........",
+		"..........",
+		".....11111",
+		"..........",
+		"..........",
+	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
 
-func TestPlot(t *testing.T) {
+}
+
+func TestPlotVerticalLine(t *testing.T) {
+	ic := ImageChecker{
+		palette: map[string]color.RGBA{
+			"1": color.RGBA{0xff, 0xff, 0xff, 0xff},
+			".": color.RGBA{0, 0, 0, 0},
+		},
+	}
+
+	// Flatline
+	p := Plot{
+		Width:  10,
+		Height: 5,
+		X:      []float64{37, 37},
+		Y:      []float64{1, -3},
+	}
+
+	assert.Equal(t, nil, ic.Check([]string{
+		"1.........",
+		"1.........",
+		"1.........",
+		"1.........",
+		"1.........",
+	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
+}
+
+func TestPlotJaggedLine(t *testing.T) {
 	ic := ImageChecker{
 		palette: map[string]color.RGBA{
 			"1": color.RGBA{0xff, 0xff, 0xff, 0xff},
@@ -41,18 +236,162 @@ func TestPlot(t *testing.T) {
 	p.Width = 20
 	p.Height = 10
 	assert.Equal(t, nil, ic.Check([]string{
-		"........1...........",
-		".......11.........11",
+		"........1..........1",
+		".......11.........1.",
 		"......1..1.......1..",
 		".....1...1......1...",
-		".....1...1......1...",
-		"....1.....1....1....",
+		"....1....1.....1....",
+		"...1......1...1.....",
 		"...1......1...1.....",
 		"..1.......1..1......",
 		".1.........11.......",
 		"1..........1........",
 	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
 
+	// Zoom in on the second valley
+	p.XLimMin = new(float64)
+	p.XLimMax = new(float64)
+	p.YLimMin = new(float64)
+	p.YLimMax = new(float64)
+	*p.XLimMin = 5
+	*p.XLimMax = 7
+	*p.YLimMin = 1
+	*p.YLimMax = 5
+
+	assert.Equal(t, nil, ic.Check([]string{
+		"1...................",
+		".1..................",
+		"..1.................",
+		"...1................",
+		"....1...............",
+		".....11.............",
+		".......1............",
+		"........1........111",
+		".........1...1111...",
+		"..........111.......",
+	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
+
+}
+
+func TestPlotFewPoints(t *testing.T) {
+	ic := ImageChecker{
+		palette: map[string]color.RGBA{
+			"1": color.RGBA{0xff, 0xff, 0xff, 0xff},
+			".": color.RGBA{0, 0, 0, 0},
+		},
+	}
+
+	p := Plot{
+		Width:  10,
+		Height: 10,
+		X:      []float64{100, 200, 200, 100, 100, 200, 200, 100},
+		Y:      []float64{-10, -10, -20, -20, -10, -20, -10, -20},
+	}
+
+	assert.Equal(t, nil, ic.Check([]string{
+		"1111111111",
+		"11......11",
+		"1.1....1.1",
+		"1..1..1..1",
+		"1...11...1",
+		"1...11...1",
+		"1..1..1..1",
+		"1.1....1.1",
+		"11......11",
+		"1111111111",
+	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
+
+}
+
+func TestPlotInvertedColor(t *testing.T) {
+	ic := ImageChecker{
+		palette: map[string]color.RGBA{
+			"1": color.RGBA{0x0, 0xff, 0x0, 0xff},
+			"2": color.RGBA{0xff, 0, 0, 0xff},
+			".": color.RGBA{0, 0, 0, 0},
+		},
+	}
+
+	p := Plot{
+		Width:         10,
+		Height:        5,
+		X:             []float64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
+		Y:             []float64{1, 1, 0, 0, -1, -1, 1, 1, 0, 0},
+		Color:         &color.RGBA{0, 0xff, 0, 0xff},
+		ColorInverted: &color.RGBA{0xff, 0, 0, 0xff},
+	}
+	assert.Equal(t, nil, ic.Check([]string{
+		"11....11..",
+		"..1...1.1.",
+		"..11..1.11",
+		"....22....",
+		"....22....",
+	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
+
+	p.Width = 20
+	p.Height = 10
+	assert.Equal(t, nil, ic.Check([]string{
+		"111..........111....",
+		"...1.........1..1...",
+		"...1.........1..1...",
+		"....1.......1....1..",
+		"....111.....1....111",
+		"......2.....2.......",
+		".......2....2.......",
+		".......2...2........",
+		"........2..2........",
+		"........2222........",
+	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
+
+}
+
+func TestPlotSurfaceFill(t *testing.T) {
+	ic := ImageChecker{
+		palette: map[string]color.RGBA{
+			"1": color.RGBA{0x0, 0xff, 0x0, 0xff},
+			",": color.RGBA{0x0, 0xff, 0x0, 0x55},
+			"2": color.RGBA{0xff, 0, 0, 0xff},
+			":": color.RGBA{0xff, 0, 0, 0x55},
+			".": color.RGBA{0, 0, 0, 0},
+		},
+	}
+
+	// Fill with single color
+	p := Plot{
+		Width:  20,
+		Height: 10,
+		X:      []float64{0, 1, 2, 3, 4},
+		Y:      []float64{5, 5, -1, -1, 2},
+		Color:  &color.RGBA{0, 0xff, 0, 0xff},
+		Fill:   true,
+	}
+	assert.Equal(t, nil, ic.Check([]string{
+		"111111..............",
+		",,,,,,1.............",
+		",,,,,,1.............",
+		",,,,,,,1............",
+		",,,,,,,1...........1",
+		",,,,,,,,1.........1,",
+		",,,,,,,,1........1,,",
+		",,,,,,,,,1......1,,,",
+		".........1,,,,,1....",
+		"..........11111.....",
+	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
+
+	// Fil with ColorInverted
+	p.ColorInverted = &color.RGBA{0xff, 0, 0, 0xff}
+	assert.Equal(t, nil, ic.Check([]string{
+		"111111..............",
+		",,,,,,1.............",
+		",,,,,,1.............",
+		",,,,,,,1............",
+		",,,,,,,1...........1",
+		",,,,,,,,1.........1,",
+		",,,,,,,,1........1,,",
+		",,,,,,,,,1......1,,,",
+		".........2:::::2....",
+		"..........22222.....",
+	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
 }
 
 func TestPlotXLim(t *testing.T) {
@@ -75,18 +414,6 @@ func TestPlotXLim(t *testing.T) {
 	// No change when min/max matches that of data
 	*p.XLimMin = 1
 	*p.XLimMax = 10
-	assert.Equal(t, nil, ic.Check([]string{
-		"........1...........",
-		".......11.........11",
-		"......1..1.......1..",
-		".....1...1......1...",
-		".....1...1......1...",
-		"....1.....1....1....",
-		"...1......1...1.....",
-		"..1.......1..1......",
-		".1.........11.......",
-		"1..........1........",
-	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
 
 	// More space on the right
 	*p.XLimMin = 1
@@ -95,9 +422,9 @@ func TestPlotXLim(t *testing.T) {
 		".....1......1.......",
 		".....1......1.......",
 		"....11.....1........",
-		"...1..1....1........",
+		"....1.1....1........",
 		"...1..1...1.........",
-		"..1...1...1.........",
+		"..1...1..1..........",
 		"..1...1..1..........",
 		".1.....11...........",
 		".1.....11...........",
@@ -114,7 +441,7 @@ func TestPlotXLim(t *testing.T) {
 		"........11...1......",
 		".......1.1..1.......",
 		".......1..1.1.......",
-		".......1..1.1.......",
+		"......1...11........",
 		"......1...11........",
 		"......1...11........",
 		".....1....1.........",
@@ -126,13 +453,13 @@ func TestPlotXLim(t *testing.T) {
 	assert.Equal(t, nil, ic.Check([]string{
 		".......11...........",
 		".....11.1...........",
-		"....1....1..........",
-		"..11.....1..........",
-		".1........1.........",
-		"1.........1........1",
-		"...........1.....11.",
-		"...........1...11...",
-		"............111.....",
-		"............1.......",
+		"...11....1..........",
+		".11......1..........",
+		"1........1.........1",
+		"..........1......11.",
+		"..........1.....1...",
+		"..........1...11....",
+		"...........111......",
+		"...........1........",
 	}, p.Paint(image.Rect(0, 0, 100, 100), 0)))
 }