kvark / mint

Math Interoperability Types
MIT License
256 stars 20 forks source link

Unique Conversion Trait #59

Closed LPGhatguy closed 3 years ago

LPGhatguy commented 4 years ago

It would be nice for mint to provide a trait using an associated type to define a surjective conversion function. This would help crates distinguish normal conversions with From/Into and establish an equivalence relationship between mint types and individual math crates.

The trait I have in mind looks something like this:

trait IntoMint {
    type MintType;

    fn into_mint(&self) -> Self::MintType;
}

You've mentioned before that mint should not have traits. I think I have a compelling use case and I'm not sure how else I can solve it!

Background

I am working on a library named Crevice, which generates std140 versions of structs that are safe to treat as byte slices and upload to the GPU. It depends heavily on mint as the common vector type library and bases all of its conversions on it.

Usage is something like this:

#[derive(AsStd140)]
struct PointLight {
    position: mint::Vector3<f32>,
    color: mint::Vector3<f32>,
    brightness: f32,
}

let light = PointLight {
    position: [1.0, 2.0, 3.0].into(),
    color: [1.0, 1.0, 1.0].into(),
    brightness: 3.0,
};

upload_data_to_gpu(light.as_std140().as_bytes());

One awkward portion of the API is that users need to define their structs using Mint types, Crevice's format-specific types (std140::Vec2, std430::Mat2, etc), or plain arrays. Crevice effectively generates a companion struct that looks like this:

struct PointLightStd140 {
    position: <mint::Vector3<f32> as AsStd140>::Std140Type,
    _pad0: [u8; 4],
    color: <mint::Vector3<f32> as AsStd140>::Std140Type,
    brightness: <f32 as AsStd140>::Std140Type,
}

At proc macro time, it isn't possible for Crevice to tell which mint type should be converted through for a user's type. Ideally, users would be able to write this definition:

struct PointLight {
    position: cgmath::Vector3<f32>,
    color: cgmath::Vector3<f32>,
    brightness: f32,
}

This could work if Crevice could write this blanket impl for AsStd140:

impl<T> AsStd140 for T where T: IntoMint {
    type Std140Type = <<T as IntoMint>::MintType as AsStd140>::Std140Type;
}

Crevice would need to be changed so that this is the only blanket impl for the trait, but I think that would be okay.

Ralith commented 4 years ago

This would also permit the definition of fn convert<T: IntoMint, U: From<T::MintType>>(x: &T) -> U, which is neat.

kvark commented 4 years ago

Thank you for clearly explaining what the feature is! It would be a pretty tough call to make, and I don't think we can make it now. It would need all the mint supporting libraries to be changed... Let's see if there is need for that from other clients as well.

LPGhatguy commented 4 years ago

I think this feature is feasible to prototype in a separate crate. I'll tackle that so that I can verify more concretely that the signature would solve my use cases at least.

benmkw commented 4 years ago

I came across this imgui-inspect crate and it seems to me like it would benefit from the same conversions to make it possible to expose a wide range of types as user controllable inputs.

For this to be easier to extend it would help if displaying and modifying values could be implemented for all values which have a to and from mint conversion.

I have not yet tried implementing it but I believe this is a similar use case because it follows the pattern "do something for all types which can be converted from/ to these mint types"

this is basically the code that would make this possible:

trait Printer {
    fn show(&mut self);
}

impl<T> Printer for T
where
    T: Into<mint::Vector3<f32>>,
    T: From<mint::Vector3<f32>>,
{
    fn show(&mut self) {
        let mut mintvec: mint::Vector3<f32> = self.into(); // ^^^^ the trait `From<&mut T>` is not implemented for `mint::Vector3<f32>
        dbg!("this is a vector {:?}", &mintvec); // show some imgui things
        mintvec.x = 42.0; // modify state thorugh imgui input widgets
        *self = mintvec.into();
    }
}

fn main() {
    let mut vec = cgmath::Vector3::<f32>::new(0.0, 0.0, 0.0);
    vec.show();
    dbg!(&vec);
}

It might be that I'm missing something and this is already possible which would also be nice :)

EDIT:

trait Printer {
    fn show(&mut self);
}

impl<T> Printer for T
where
    T: Into<mint::Vector3<f32>>,
    T: From<mint::Vector3<f32>>,
    T : Clone
{
    fn show(&mut self) {
        let mut mintvec: mint::Vector3<f32> = self.clone().into(); 
        dbg!("this is a vector {:?}", &mintvec); // show some imgui things
        mintvec.x = 42.0; // modify state thorugh imgui input widgets
        *self = mintvec.into();
    }
}

fn main() {
    let mut vec = cgmath::Vector3::<f32>::new(0.0, 0.0, 0.0);
    vec.show();
    dbg!(&vec);
}

this works and one can probably remove the clone somehow so this seemed to be a false positive on my side.

Ralith commented 4 years ago

Note that in your example no other impls for Printer can be defined, which limits its usefulness.

benmkw commented 4 years ago

@Ralith yes I was jut about to start typing away when I realised that, basically seemingly needs specialisation.

cwfitzgerald commented 3 years ago

I just want to +1000 this issue, specifically the definition of a trait:

trait IntoMint {
    type MintType;

    fn into_mint(&self) -> Self::MintType;
}

This came up as I independently arrived at this same solution as @LPGHatguy.

Basically every time I do inter-library interop (most commonly with nalgebra for rapier), I end up rolling my own macro-based solution to the problem that lets me write a single function call to convert. If this one trait existed, we could define this one function:

pub fn convert<I, O>(value: I) -> O
where
    I: IntoMint,
    O: From<<I as IntoMint>::MintType> 
{
    O::from(value.into_mint())
}

which would be able to do interop between any compatible mint types. This would make mint a completely breeze to use, and would make adoption of it much, much more palatable.

I think this is a pretty universally wanted feature, whenever mint comes up, the desire to have a trait like this comes up. While basically every other extension of this trait could be implemented in user space, this one thing would need to be in mint itself, so everyone could implement it.

For reference, this is an example workaround I implemented for dealing with na and glam: https://gist.github.com/cwfitzgerald/c4e6b53e1daec4e4d75679cec33b9bc9

Ralith commented 3 years ago

See also prior discussion in https://github.com/kvark/mint/pull/45, a rejected PR in the same spirit.