Suggestion: split out intersection and stroke code into separate sub-packages (and others..)

[rcoreilly]

It would be a major breaking change, but the code associated with path_intersection and path_stroke is not needed by many use-cases, and could usefully be put into separate sub-packages. I just did this in my version of this code in cogent core, and the remaining top-level basic path code was only 1/3 the previous size. Also, the api footprint for the Path type is much smaller, as this requires converting methods to functions in the packages. This is more conventional Go-style, e.g., with strings having all the string functionality instead of putting everything as methods on the type. The same approach could be taken with all the latex, text, etc stuff as well. Anyway, just a suggestion -- it did take a big chunk out of my day and wasn't much fun, but I'm happy with the result :)

[tdewolff]

Thanks for the suggestion, but I'm not clear on the advantages. Does the large API bother you? I don't think binary size is influenced much, or is it? I believe that both path stroking and intersection code are very integral parts of handling paths. Text and LaTeX stuff is already on their separate types, there's some room to move or put methods on the polyline type, and perhaps the path intersection code should work on slices of simple paths []*canvas.Path (as opposed to compound paths *canvas.Path).

There's even the idea of optimising the memory requirements by reducing storage space of paths by almost 50% (for solely LineTo's) by omitting repeated path commands, or by another 50% by using generics to support float32s. This could be complemented by encoding some basic properties of the path in their memory representation (such as being XMonotone or non-overlapping) which may optimise some functionalities.

Going back to your point, I believe path stroking and intersection code are "basic" functionality that should be on the path's type. What's your proposed API for those? Secondly, putting functionality in subpackages would help with dependency reduction, but since those functionalities hardly involve any dependencies, what is the benefit of moving them to subpackages?

[rcoreilly]

I'm just saying that in our use of this code, we didn't need any of the intersection or stroke stuff for the basic GUI functionality (rasterx does the stroking), so that's one example where it could usefully be partitioned. I just find it conceptually easier to see the methods on our version of Path as basic shape drawing and infrastructure, whereas the intersect package has quite a large footprint and I find it easier to understand that all of these functions are more advanced operations on paths..

https://pkg.go.dev/cogentcore.org/core@v0.3.11/paint/ppath
https://pkg.go.dev/cogentcore.org/core@v0.3.11/paint/ppath/intersect

by comparison, the larger canvas footprint has many different things in one package space:

https://pkg.go.dev/github.com/tdewolff/canvas

These things are ultimately a matter of taste, but anyway, I was just happy with the result of partitioning the intersect and stroke code into separate packages so it made things a bit cleaner conceptually..

[tdewolff]

I see what you mean. Perhaps we should hide (lowercase) the following types: Intersections SweepEvents SweepNode SweepPoint SweepStatus. And on a second note, the Path type should be an array of polygons (each a single path). That would solve the need for Paths since that was just an optimisation to avoid merging and splitting (Path.Split) paths. That would remove Paths.

Otherwise, the divide is quite arbitrary of which functions should be on Path and which should be their own function. This make it unclear which functions belong to Path (all of them do) in my opinion. In the end the problem is that we can't categorise methods in GoDoc. We're bound to add more methods anyways! Just to give you an idea of how many functions could be defined on a path: http://paperjs.org/reference/pathitem/

I've noticed you also modified the receptor type to be a value instead of a pointer when it doesn't modify the path. I like that this shows which functions may modify the path (or more accurately, extend the path, you can still modify the values in a path when you have a value receiver that is a slice), but be aware that now you're copying three uint64s (on a 64bit arch) instead of one for every function call. I've noticed that for small functions that are called often this is a (very) slight performance penalty.