nathanhack / svg

A Scalable Vector Graphics (SVG) helper library for vecty.
MIT License
4 stars 3 forks source link

Add support for higher level abstractions for working with paths #3

Closed seanrmurphy closed 4 years ago

seanrmurphy commented 4 years ago

As discussed in #1 it would be nice to have some kind of higher level abstraction for working with paths.

I did some work in my own fork - the basic logic is here:

https://github.com/seanrmurphy/svg/blob/master/svgelem/svgtypes/types.go

Note that I decided to modify package names in my fork - I did not like the fact that the original repo contained an elem package which had a clash with the vecty package of the same name (obviously); I also introduced the svgtypes package as the home for these higher level abstractions.

If you think this approach is largely ok, I can make a PR with these changes.

Here's some (not v clean) sample code which shows how it works (this is still buggy but shows how the path abstraction works).

    for _, percent := range percentages {
        pathData := svgtypes.Path{}
        // moveTo 0,0
        pathElement := svgtypes.PathElement{
            Mode:       svgtypes.MoveToAbsolute,
            Parameters: []float64{0, 0},
        }
        pathData.Elements = append(pathData.Elements, pathElement)
        // lineTo start
        pathElement = svgtypes.PathElement{
            Mode:       svgtypes.LineToAbsolute,
            Parameters: []float64{rx, 0},
        }
        pathData.Elements = append(pathData.Elements, pathElement)
        x = x + float64(rx)*math.Cos(percent*2*math.Pi)
        y = y + float64(ry)*math.Sin(percent*2*math.Pi)
        pathElement = svgtypes.PathElement{
            Mode:       svgtypes.ArcAbsolute,
            Parameters: []float64{rx, ry, 0.0, 0, 0, x, y},
        }
        pathData.Elements = append(pathData.Elements, pathElement)
        pathElement = svgtypes.PathElement{
            Mode: svgtypes.ClosePath,
        }
        pathData.Elements = append(pathData.Elements, pathElement)
        s := svgtypes.GeneratePathString(pathData)
        log.Printf("s = %v\n", s)
        paths = append(paths, s)
    }

Hmm...now that I read this code, I realize it would be more natural to replace the append with an addElement method on the struct and similarly with the GeneratePathString...will make these clean ups.

Let me know what you think in any case.

seanrmurphy commented 4 years ago

Modified the interface on this again to make it simpler - here's the same code using the newer interface

    for _, percent := range percentages {
        path := svgtypes.Path{}

        // moveTo 0,0
        pe := svgtypes.NewMoveToAbsolute(0, 0)
        path.AddElement(pe)

        // lineTo
        x = float64(rx) * math.Cos(cumulativePercentage*2*math.Pi)
        y = -float64(ry) * math.Sin(cumulativePercentage*2*math.Pi)
        pe = svgtypes.NewLineToAbsolute(x, y)
        path.AddElement(pe)

        // create Arc
        cumulativePercentage += percent
        x = float64(rx) * math.Cos(cumulativePercentage*2*math.Pi)
        y = -float64(ry) * math.Sin(cumulativePercentage*2*math.Pi)
        pe = svgtypes.NewArcAbsolute(rx, ry, 0.0, 0, 0, x, y)
        path.AddElement(pe)

        // close path
        pe = svgtypes.NewClosePath()
        path.AddElement(pe)

        s := path.ToString()
        paths = append(paths, s)
    }
nathanhack commented 4 years ago

Sorry for the delayed response. I've been busy, but I got a chance to give it an earnest look today.

Starting from the top, to comment on the package naming. One of my hopes is this repo be the testing ground for SVG in vecty and eventually some form of it merged into vecty. At which point the names of the packages will most likely change. Until then I wanted to keep them inline with svg type. One of the nice things about golang is the aliasing, so users can rename the package to what ever they want in their programs. Hopefully that help explain what I did and mostly why :-). That said, I'm not completely married to package names or directory structure. Any thoughts on the topic are welcomed, especially when it makes for more readable code.

After looking at your example code above I have come to the following conclusions.
1)I like the way you expect to interact/create lines (or other path commands), it lends itself to be very flexible. In particular it allows procedural construction. This was one of the my hopes/goals.
2) I don't want to limit other options, if possible. 3) I want to align the names with svg labels to make it easy migrate people from svg in html to using svg in vecty.

Considering that wrote up something quickly and I purpose the following examples:

Additionally, I would like to not remove anyone that already passes in a string literal into attr.D() as example

svg.Render(
    svg.SVG(
        attr.ViewBox(0, 0, 100, 100),
        elm.Path(
            attr.Stroke("red"),
            attr.D("M 50 50 h -25"),
        ),
    ),
)

Instead, I would like to add to that, these options (from the first example drawing a heart).

svg.Render(
    svg.SVG(
        attr.ViewBox(0, 0, 100, 100),
        elm.Path(
            attr.Fill("purple"),
            attr.Stroke("red"),
            attr.D("M 10,30",
                "A 20,20 0,0,1 50,30",
                "A 20,20 0,0,1 90,30",
                "Q 90,60 50,90",
                "Q 10,60 10,30 z",
            ),
        ),
    ),
)
svg.Render(
    svg.SVG(
        attr.ViewBox(0, 0, 100, 100),
        elm.Path(
            attr.Fill("purple"),
            attr.Stroke("red"),
            attr.D(
                path.M(10, 30),
                path.A(20, 20, 0, 0, 1, 50, 30),
                path.A(20, 20, 0, 0, 1, 90, 30),
                path.Q(90, 60, 50, 90),
                path.Q(10, 60, 10, 30),
                path.Z(),
            ),
        ),
    ),
)
svg.Render(
    svg.SVG(
        attr.ViewBox(0, 0, 100, 100),
        elm.Path(
            attr.Fill("purple"),
            attr.Stroke("red"),
            attr.D(path.M(10, 30).
                    A(20, 20, 0, 0, 1, 50, 30).
                    A(20, 20, 0, 0, 1, 90, 30).
                    Q(90, 60, 50, 90).
                    Q(10, 60, 10, 30).
                    Z(),
            ),
        ),
    ),
)

and

pathCmds := []path.Cmd{
    path.M(10, 30),
    path.A(20, 20, 0, 0, 1, 50, 30),
    path.A(20, 20, 0, 0, 1, 90, 30),
    path.Q(90, 60, 50, 90),
    path.Q(10, 60, 10, 30),
    path.Z(),
}
...

svg.Render(
    svg.SVG(
        attr.ViewBox(0, 0, 100, 100),
        elm.Path(
            attr.Fill("purple"),
            attr.Stroke("red"),
            attr.D(pathCmds...),
        ),
    ),
)
seanrmurphy commented 4 years ago

Moved discussion on package naming to #4 .

seanrmurphy commented 4 years ago

Nice ideas and I would be willing to contribute although I would need to go through the proposals in more detail; I don't want to completely discard the stuff I have done, as I think there is still space for path elements with more specified names, but perhaps as an extra layer on top which may not be part of the main package, but could be helpful for people who are not so familiar with svg specifics.

An obvious question relates to how to deal with the case in which the number of parameters provided makes no sense eg path.M(p1, p2, p3) but I guess this could be considered a next step.

nathanhack commented 4 years ago

I agree, having more descriptive names is good practice and adds value. I already added the path commands as described above (I should have made a PR but I just added it directly). I have time today so I'll go ahead and update it to contain more descriptive names.

nathanhack commented 4 years ago

@seanrmurphy would you be ok with me using the longer names found in your type.go?

seanrmurphy commented 4 years ago

Of course - no problem at all!