Open nileshpatra opened 3 years ago
@fogleman please consider to review this. As this fails and results in a regression on arm64 which is important for me to be fixed.
The underlying freetype/raster
package natively supports quadratic bezier curves (but not cubic). The quadratic curves are rendered with those Add2
functions. What issues were you seeing with the original code?
@fogleman the rendered image in arm64 which indirectly uses the QuadraticTo function renders an image with slight deltas. This leads to failing tests for the same. However all other functions render perfectly identical graphics between both the architectures.
I suspect that this is somewhere due to minor rendering issues?
Please let me know. Although I've tested it, but I'd also appreciate if you could let me know if this patch is not a "breaking" change?
Can you post sample code and the output images?
@fogleman Sample code and output with the patch applied
package main
import "./ups/gg"
func main() {
const S = 1024
dc := gg.NewContext(S, S)
dc.SetRGBA(0, 0, 0, 0.1)
for i := 0; i < 360; i += 15 {
dc.Push()
dc.RotateAbout(gg.Radians(float64(i)), S/2, S/2)
dc.DrawEllipse(S/2, S/2, S*7/16, S/8)
dc.Fill()
dc.Pop()
}
if im, err := gg.LoadImage("examples/gopher.png"); err == nil {
dc.DrawImageAnchored(im, S/2, S/2, 0.5, 0.5)
}
dc.SavePNG("out.png")
}
Another one using QuadraticTo
directly
package main
import "./ups/gg"
func main() {
const S = 1000
dc := gg.NewContext(S, S)
dc.SetRGB(1, 1, 1)
dc.Clear()
dc.Translate(S/2, S/2)
dc.Scale(40, 40)
var x0, y0, x1, y1, x2, y2, x3, y3, x4, y4 float64
x0, y0 = -10, 0
x1, y1 = -5, -10
x2, y2 = 0, 0
x3, y3 = 5, 10
x4, y4 = 10, 0
dc.MoveTo(x0, y0)
dc.LineTo(x1, y1)
dc.LineTo(x2, y2)
dc.LineTo(x3, y3)
dc.LineTo(x4, y4)
dc.SetHexColor("FF2D00")
dc.SetLineWidth(8)
dc.Stroke()
dc.MoveTo(x0, y0)
dc.QuadraticTo(x1, y1, x2, y2)
dc.QuadraticTo(x3, y3, x4, y4)
dc.SetHexColor("3E606F")
dc.SetLineWidth(16)
dc.FillPreserve()
dc.SetRGB(0, 0, 0)
dc.Stroke()
dc.DrawCircle(x0, y0, 0.5)
dc.DrawCircle(x1, y1, 0.5)
dc.DrawCircle(x2, y2, 0.5)
dc.DrawCircle(x3, y3, 0.5)
dc.DrawCircle(x4, y4, 0.5)
dc.SetRGB(1, 1, 1)
dc.FillPreserve()
dc.SetRGB(0, 0, 0)
dc.SetLineWidth(4)
dc.Stroke()
dc.LoadFontFace("/Library/Fonts/Arial.ttf", 200)
dc.DrawStringAnchored("g", -5, 5, 0.5, 0.5)
dc.DrawStringAnchored("G", 5, -5, 0.5, 0.5)
dc.SavePNG("out.png")
}
But what does it look like without the fix?
@fogleman
Without fixes on amd64
:
On arm64
:
Though they look identical but their md5sums are different
OK, actually the tests in gg
should probably be changed to permit small, indistinguishable differences since various platforms produce subtly different results. So, instead of using md5sums, comparing images and ensuring that the delta is small.
OK, actually the tests in
gg
should probably be changed to permit small, indistinguishable differences since various platforms produce subtly different results. So, instead of using md5sums, comparing images and ensuring that the delta is small.
@fogleman Thanks for clarifying. Two questions:
For the background: I actually maintain this as a package in Debian. And this fails for archs other than amd64 hence causing a regression
This is actually blocking the migration of many other packages and hence I wished this to be fixed soonish.
In accordance with #113
PS: This also seems to fix failing tests on arm64