Open Athosvk opened 6 months ago
Even if we were to thread the land of unsafe, I'd bet it's undefined behaviour to read/write into the same m128 at different offsets simultaneously, so fields_mut would never work without potential UB.
Fwiw this is not true, an m128 is trivially type-punnable as a [f32; 4]
and therefore also a
#[repr(C)]
struct XYZW {
x: f32,
y: f32,
z: f32,
w: f32,
}
Which are both then destructurable. That's exactly how the internal implementations of glam's Deref
and DerefMut
impls work
Problem Currently the
glam
support does not include types that are architecture dependent, e.g.Quat
andVec4
. This is due toVec4
either being a[float; 4]
orm128
depending on the target architecture andglam
using wrappers and usingDeref
to allow trivial access to each vector's components.Unfortunately the lack of reflection supports means either skipping those fields with
#[reflect(skip)]
, manually implementingReflect
,FromReflect
andDescribeType
for the type containing these fields in some way, or supporting a wrapper type aroundglam::Vec4
and equivalents which then requires replacements in all of the codebase. Hopefully this can start a discussion on potential ways to tackle this.Preferred Solution As the current comment in
mirror-mirror
'sglam
implementation states, manually re-implementing config checks and hacks that are done byglam
doesn't seem great. Ideally we can still useglam
's types without wrappers (within structs), whether that asStruct
or maybe asScalar
(?).I've tried implementing
Struct
forVec4
and this seems to be almost possible from what I've found, with the only problem beingfields_mut
.fields_mut
ends up requiring a borrow to each field, butVec4
is am128
with SIMD on which is only a single field. Even if we were to thread the land ofunsafe
, I'd bet it's undefined behaviour to read/write into the samem128
at different offsets simultaneously, sofields_mut
would never work without potential UB. The other trait functions are trivial, though this is a manual implementation rather than just use the#[derive]
syntax as one would expect.I'd argue
Vec4
as aScalar
type makes sense (probably withVec2/3
to match), also because the (often) underlyingm128
is scalar-like, but it perhaps does litter the code with[cfg]
s to check ifglam
support should be on. Perhaps there's also limitations that I'm not aware of forScalar
types.Alternatives Maybe an alternative is to just provide a wrapper type for these that easily allow conversions from/to
glam::Vec4/Mat4/Quat
if the above options aren't really feasible.