dangmoody / HLML

Auto-generated maths library for C and C++ based on HLSL/Cg
24 stars 1 forks source link

"cannot convert from 'int2' to 'float2'" - explicit conversion constructors needed to match HLSL #58

Closed fstrugar closed 1 year ago

fstrugar commented 2 years ago

Hi! Thanks for the great lib. Just trying to compile some HLSL code, stumbled upon this one.

dangmoody commented 2 years ago

Hi there, thanks for the kind words! =)

Ah ok, looks like I haven't got conversion ctors for things like this. Whoops! I'll get to adding that.

Thanks for highlighting this.

fstrugar commented 2 years ago

Awesome, thanks :)

dangmoody commented 1 year ago

Hi, starting work on this one now.

Is there a list of all the allowed conversions in HLSL/Cg? I tried searching for something but I found nothing and I'd rather not guess my way through all the permutations if I can help it.

fstrugar commented 1 year ago

Thanks! I don't know of a list - but I can try out all the permutations and let you know (one of these days when I have a chunk of time). I think dxc HLSL pretty much lets you convert anything to anything (although it will complain if you >> shift a const that has unambiguous sign and few other things) but you could, to be on a safe side, always use 'explicit' whenever converting with potential data loss. It would be more restrictive than HLSL but for me that would be a feature (helps avoid accidental conversions) and would force one to make HLSL code safer? :)

dangmoody commented 1 year ago

Ok, fair enough. 🙂

I think you've found the perfect middleground for me with this. Explicit ctors are the way to go.

If it ends up annoying people that much then I imagine someone will complain at some point and make an issue for it stating their case and then we can review it and have that conversation then, but this definitely seems like The Right Thing to do. I'd rather have safety over convenience in this instance.

dangmoody commented 1 year ago

I've submitted explicit ctors for converting from between every vector with the same number of components. I've also added this for matrices while I was there. No official release yet but I've attached a preview release (including the new swizzle convention) at the bottom of this comment.

I haven't done anything for converting between vectors of different sizes yet, but the plan for converting from a smaller vector to a bigger one is to ONLY touch the components that the smaller vector cares about.

For example, if doing this:

float2 smaller = { 1.0f, 2.0f };
float3 bigger = float3( smaller );

Only x and y of bigger are going to get set and z will be whatever it was before.

I've seen some maths libraries do it where the z component would get set to 0 in this case but I don't like that because it could potentially overwrite pre-existing data if it gets written to before converting from a smaller vector. IIRC HLSL does the same.

Let me know if you have any disagreements with this. I'm always interested to hear others thoughts. =)

hlml_with_converstion_ctors.zip

dangmoody commented 1 year ago

Got the above working for both vectors and matrices. So unless we have any gripes with what I've put in the above comment I'll mark this as resolved when it goes out into a release.

fstrugar commented 1 year ago

Hey, sorry I'm slow with answering - I'm in the middle of a house move! :) HLSL actually doesn't let you do this smaller->bigger or bigger->smaller conversions at all!

<code file> error: cannot convert from 'float2' to 'float3'
    float3 bigger = float3( smaller );
                    ^

You have to explicitly do

float3 bigger = float3( smaller, 0.0f ); (or bigger.xy = smaller)

That seems the best to me, then the next best would be as you say to not touch the empty - initializing to 0 seems bad/unexpected.

dangmoody commented 1 year ago

Oh yeah, I forgot about the ctors with vector and scalar mixes.

I'll change it to that, I prefer that too.

Good luck with the house move. =)

dangmoody commented 1 year ago

Hey there!

I've attached a version of HLML with all of the new ctors in their current form. I've still got to write the tests for them yet, but everything seems fine with them so far. Let me know what you think.

I hope the house move went/is going well. =)

hlml_new_ctors.zip

fstrugar commented 1 year ago

Hi! Thanks and sorry for the silence on my end, I've been preoccupied with packing, carrying stuff, driving the van, carrying stuff, unpacking, rinse & repeat :) Just wanted to let you know that I'll try it out when I settle and manage to get my main PC working - might take a while longer but it's first on TODO list!

dangmoody commented 1 year ago

Hey there! Don't worry about it! I get what it's like with house moves. =)

I've gotten all the functionality and tests written for this issue as well as #59 so the preview .zips I uploaded are ready whenever you are. There's no rush.

Hope you're settling into the new place! =)

fstrugar commented 1 year ago

Hi there! I'm sorry for the delays but this is the first week that I managed to get my home PC working and start looking into non-work things again.

Is that hlml_new_ctors.zip still the latest with regards to this? :) I saw there was a 2.1.2 release in the meantime. Cheers! :)

dangmoody commented 1 year ago

Hey there!

No worries. Welcome back. =D

The archive above is still latest in the context of this, yes (there's also one for #59, also up to date). v2.1.2 was just a hot-fix for a bug we found in the quaternion slerp functions. 🙂

Please give those zips a go and let me know what you think. I'm happy with them (minus the compile time increase that the new swizzle functionality has introduced) so pending your approval I can definitely get the new conversion ctors in v2.2.0 at the very least.

fstrugar commented 1 year ago

Hi! Thanks :D

Constructors look great, I tested a couple of scenarios and it all seems to work nicely! :)

I'll go and test the swizzling ones now. I'll test compile times on my project too!

dangmoody commented 1 year ago

Fixed in v2.2.0.