Add an Angle and RVec (rotation vector) type

bschwind / opencascade-rs

Rust bindings to the OpenCascade CAD Kernel

GNU Lesser General Public License v2.1

118 stars 22 forks source link

Add an Angle and RVec (rotation vector) type #85

Closed bschwind closed 1 year ago

bschwind commented 1 year ago

This is one possible implementation for more strongly-typed angles. Spawned from this discussion

The main goals are:

Calling code should always indicate the unit of rotation (radians vs. degrees vs. something else)
The called code (the OpenCascade bindings) should also explicitly mention which rotation unit it desires when passing an angle to OpenCascade.
The calling code should be ergonomic (easy to construct, easy to read, terse but not to the point where it hurts readability)

I think what we have here so far is nice, but I'm open to suggestions if we want to make it more Rust-y with new types and impl From<_> signatures on functions which take angles.

strohel commented 1 year ago

@PabloMansanet can we nerd-snipe you to try an alternative implementation where Degrees and Radians are separate types?

I think the "2 separate types" would be just slightly more canonical, yet a bit more verbose: APIs would have to accept argument: impl Into<Radians> (which we could alias as impl Angle) rather than argument: Angle.

My gut feeling would be enum { Degrees, Radians } would be more suitable when we want to carry the difference between the 2 variants in runtime. But if we only care about interoperability, compile-time conversion (the 2 separate types) is the way to go.

But both approaches are better than status quo, I'm certainly not against merging this.

PabloMansanet commented 1 year ago

@PabloMansanet can we nerd-snipe you to try an alternative implementation where Degrees and Radians are separate types?

I think the "2 separate types" would be just slightly more canonical, yet a bit more verbose: APIs would have to accept argument: impl Into<Radians> (which we could alias as impl Angle) rather than argument: Angle.

My gut feeling would be enum { Degrees, Radians } would be more suitable when we want to carry the difference between the 2 variants in runtime. But if we only care about interoperability, compile-time conversion (the 2 separate types) is the way to go.

But both approaches are better than status quo, I'm certainly not against merging this.

I like being nerd sniped :) sure thing, I'll get through this today, even if I don't manage to get much more done other than review!