deadsy / sdfx

A simple CAD package using signed distance functions
MIT License
533 stars 52 forks source link

Redesigning Vec family API #48

Closed soypat closed 2 years ago

soypat commented 2 years ago

Using the method approach to managing vectors feels clunky and less readable than defining functions that take all vectors needed as arguments.

Compare

center := a.f(t) 
n := a.f(t + dt2)
n = n.Sub(a.f(t - dt2))
pv := p.Sub(center) 

with

center := a.f(t)
n := sdfv.Sub(a.f(t + dt2),  a.f(t - dt2))
pv := sdfv.Sub(p, center) 

If this were to be done it could be moved to a subpackage and the methods be left for backwards compatibility

deadsy commented 2 years ago

So: func Sub(a, b V3) V3

Instead of: func (a V3) Sub(b V3) V3

The advantage of the second form is that it leaves "Sub" free in the namespace to be used for other types. E.g. V2, integer vectors, etc.

I think that's why I selected it. Blame a lack of function templates in go.

soypat commented 2 years ago

What you are saying is reasonable. This namespace constraint however can be solved by creating a package for each type much like gonum does.

deadsy commented 2 years ago

This namespace constraint however can be solved by creating a package for each type

Done.

https://github.com/deadsy/sdfx/commit/10ae73b8be68640d4062e39ba22f5aef11900893

I haven't created an c = op(a, b) functions at this time, but it is trivial to do so.

soypat commented 2 years ago

Awesome! I guess as soon as those functions are written we can close this issue. I am unavailable to contribute this feature.

aprice2704 commented 2 years ago

@soypat you could however do:

n := a.f(t + dt2).Sub(a.f(t - dt2))`

couldn't you?

soypat commented 2 years ago

@aprice2704 my issue is not with the amount of lines it takes to express an operation but rather how readable the code is. I dont think methods as operations lend themeselves to readable code, in my experience

aprice2704 commented 2 years ago

I understand but respectfully disagree :) I find the method-operator version to be clearer, since it enforces the order in which operators are applied rather than relying on conventions from natural languages. sub(a,b) results in a-b or b-a? a.sub(b) must be a-b. I have been writing a fairly complex set of geometry manipulations and found the operator-method to be quite clear. :) In any event, the methods being present do not constrain you from adding sub(a,b) and friends to your own packages if you wish; however, adding methods requires forking the package, so I would much prefer @deadsy leaves them in place :)

deadsy commented 2 years ago

At this point the vector packages have been split out into individual packages for each vector type. The rest of the code has been converted to use the new packages.

At the moment the a.Op(b) for is still the main way of doing things.

The Op(a,b) form is possible without leading to name space problems. E.g. v2.Add(a,b)

Still- I haven't created these functions, because I don't feel the need.