ianmackenzie / elm-geometry

2D/3D geometry package for Elm
Mozilla Public License 2.0
183 stars 26 forks source link

WIP: Transformation2d #150

Open gampleman opened 3 years ago

gampleman commented 3 years ago

This is an API sketch for addressing #79.

Obviously a full implementation needs:

The point here is more to start a conversation around API design before I waste more time on building that stuff.

I've went with a fairly powerful design which allows both unit and coordinate system conversion, as well as tracking which kind of transforms are allowed. This is however at the cost of a fairly complex type signature.

ianmackenzie commented 3 years ago

Exciting, thanks for starting the discussion! A few random thoughts to start...I'm sure I'll have more as I think about this =)

Type parameters

For a bit of consistency with other code, I'm wondering about something like

type Transformation2d units coordinates restrictions

where the units and coordinates type parameters will always be function types and restrictions will basically be an extensible record. So you'd have things like

translateBy : 
    Vector2d units coordinates
    -> Transformation2d (units -> units) (coordinates -> coordinates) a

scaleAbout :
    Point2d units coordinates
    -> Float
    -> Transformation2d (units -> units) (coordinates -> coordinates) { a | scale : Allowed }

relativeTo :
    Frame2d units coordinates { defines : localCoordinates }
    -> Transformation2d (units -> units) (coordinates -> localCoordinates) a

at :
    Quantity Float (Rate units1 units2)
    -> Transformation2d (units2 -> units1) (coordinates -> coordinates) { a | scale : Allowed }

-- transformation1 |> Transformation2d.followedBy transformation2
followedBy :
    Transformation2d (units2 -> units3) (coordinates2 -> coordinates3) restrictions
    -> Transformation2d (units1 -> units2) (coordinates1 -> coordinates2) restrictions
    -> Transformation2d (units1 -> units3) (coordinates1 -> coordinates3) restrictions

Then the type parameters of Transformation2d would follow the same pattern as other elm-geometry types (pretty much always units coordinates, and sometimes another type parameter) and restrictions could be written like { a | scale : Allowed } without having to include all the other fields, e.g.

{ a | scale : Allowed, inputUnits : inputUnits, outputUnits : outputUnits, ... }

which I think you'd end up needing with the current design.

Internal representation

I think I might be tempted to use

type Transformation2d units coordinates restrictions
    = Transformation2d { m11 : Float, m12  : Float, ..., m33 : Float }

to be consistent with elm-explorations/linear-algebra - then unsafe/unwrap can be true no-ops and this should allow nicer interop with elm-explorations/linear-algebra (which is likely to move to just using plain records as the primary representation at some point, so you could actually have zero-overhead interop).

gampleman commented 3 years ago

Regarding representation: elm-explorations/linear-algebra doesn't really have a Mat3 type. I agree that for Transformation3d using that record based representation makes sense, but using 16 Floats for 2d transforms seems wasteful when just 6 are enough.

gampleman commented 3 years ago

OK, I've changed the types to be as you suggested. I think it's a good improvement, as the function types intuitively communicate the idea of transformation better than the input/output prefixes.

Also note that there is a failing test:

↓ Tests.Transformation2d
↓ sanity checks
✗ scaleAbout

Given (Point2d { x = 0.000001, y = 0.000001 },0.000001,Point2d { x = 0.000001, y = 0.000001 })

    Expected Point2d { x = -9.99998e-7, y = -9.99998e-7 }, got Point2d { x = 0.000001, y = 0.000001 }

Any idea why that might be? Is that just floating point arithmetic drift?

ianmackenzie commented 3 years ago

I realize that linear-algebra doesn't have a Mat3 type, but it's also quite possible that it might at some point in the future (I was talking to @w0rm and he mentioned it was weird that Mat3 wasn't there, so I think there's a good chance that when that package gets redesigned it might gain one). So I was thinking of basically using a 9-field record as the internal representation of a Transformation2d so it would be consistent with a future Mat3 type. Also, if we have Transformation3d use a 16-field record as its internal representation, then I think it does make sense to be consistent for Transformation2d.

ianmackenzie commented 3 years ago

For the failing test, I think the sign is incorrect in scaleAbout - should be p.x - k * p.x instead of k * p.x - p.x, and likewise for Y.