plot icon indicating copy to clipboard operation
plot copied to clipboard

Enable Glyphs to have an outline

Open zaddok opened this issue 9 years ago • 2 comments

screen shot 2016-07-15 at 4 42 05 pm

Just in case anyone is wanting to ask why we would do this, 1. other libraries do it, and 2. In my case I need to make my application create a graph that looks like the current ones (with an outline). Consider for example the javascript library, chart.js

A patch for this could be done without altering the API. Here is my current solution:

--- a/vg/draw/canvas.go
+++ b/vg/draw/canvas.go
@@ -55,6 +55,12 @@ type GlyphStyle struct {
        // Radius specifies the size of the glyph's radius.
        Radius vg.Length

+       // Width of the line around the glyph. If set, LineColour is used to outline the glyph
+       Width vg.Length
+
+       // Outline glyph with this color if Width is set.
+       LineColor color.Color
+
        // Shape draws the shape of the glyph.
        Shape GlyphDrawer
 }
@@ -106,6 +112,11 @@ func (CircleGlyph) DrawGlyph(c *Canvas, sty GlyphStyle, pt vg.Point) {
        p.Arc(pt, sty.Radius, 0, 2*math.Pi)
        p.Close()
        c.Fill(p)
+
+       if sty.Width > 0 {
+               c.SetLineStyle(LineStyle{Color: sty.LineColor, Width: sty.Width})
+               c.Stroke(p)
+       }
 }

 // RingGlyph is a glyph that draws the outline of a circle.

This simply add's two fields, if you want to set an outline, you can set the new Width field, and the new LineColor field.

Patch: https://github.com/gonum/plot/pull/298

zaddok avatar Jul 15 '16 06:07 zaddok

This seems fine. But to be clear, if we do it, the reason isn't that other packages allow it, so we should to; the reason is because we think that it's a good thing to have. I don't want to start adding lots of features just because other plot packages have them.

eaburns avatar Jul 15 '16 13:07 eaburns

Excellent, thanks. I agree libraries should avoid feature bloat. I was providing the reference to provide evidence that this enhancement wasn't just a wacky idea I dreamed up. Not to suggest we should copy or do things just because someone else has. Thanks. I'll fix and submit a new patch tomorrow.

zaddok avatar Jul 15 '16 14:07 zaddok