Rkyv support #223
Rkyv support #223
Conversation
Cargo.toml
Outdated
| @@ -15,6 +15,9 @@ maintenance = { status = "actively-developed" } | |||
|
|
|||
| [features] | |||
| default = ["std"] | |||
| rkyv_size_16 = ["rkyv/size_16"] | |||
| rkyv_size_32 = ["rkyv/size_32"] | |||
| rkyv_size_64 = ["rkyv/size_64"] | |||
There was a problem hiding this comment.
These features are meant to be controlled by a top-level crate. I think that whatever uses glam and rkyv can choose these features and they'll unify all the way down.
src/features/impl_rkyv.rs
Outdated
| #[cfg(not(any(feature = "archive_le", feature = "archive_be")))] | ||
| type Archived = $type; | ||
| #[cfg(feature = "archive_le")] | ||
| type Archived = rend::LittleEndian<$type>; |
There was a problem hiding this comment.
I'm not sure if this will work right. There might be some work to do here if you want glam types to support little- and big-endian, since some of the SIMD types are new primitives. If glam is using a standard crate I can see about getting this work done.
There was a problem hiding this comment.
I removed the endian conversion step for now until support has been added to convert SIMD types. This is mentioned in the readme and can be added in a follow-up PR. Progress can be tracked here.
There was a problem hiding this comment.
Awesome, looks great! I'll be digging into this more comprehensively in rkyv/rend#4
removed rkyv_size features & changed the optional rkyv dependency to version 0.7
Added the rkyv and bytecheck to the optional feature list
hannes-vernooij
force-pushed
the
rkyv_support
branch
from
7d3c49a
to
7bbfcde
Compare
This is currently not supported for SIMD types yet
|
Sorry I've been quite busy the last few days, I will look at this soon. |
src/features/impl_rkyv.rs
Outdated
| }; | ||
|
|
||
| (@archive_deserialize $type:ty) => { | ||
| const _: () = { |
There was a problem hiding this comment.
Looks like scoping for the type alias right below (type Archived = $type). Doesn't look strictly necessary to me.
There was a problem hiding this comment.
Yes, it was to scope the type alias. I removed the scope.
removed a const block to scope a type alias
|
clippy failed on internal rkyv things, which is strange. |
If I understand the error correctly it means that size_16 size_32 or size_64 need to be passed as a feature by the crate using glam. I can look into getting size_32 enabled by default, or would it be better to define these in the top level crate as previously suggested by djkoloski? |
|
This looks like it may be a situation where we want: [dependencies]
rkyv = { version = "0.7", optional = true, default-features = false }
[dev-dependencies]
rkyv = "0.7"However, this looks like it's only supported by the feature resolver version 2, which would bump the MSRV to 1.50. Feature resolver version 1 will incorrectly unify the default features from the dev-dependencies with the regular dependency features. |
|
I would be OK with switching to feature resolver version 2, it would address some other problems I hit in glam. That can happen separate to this PR though. |
|
@djkoloski incidentally it looks like rkyv's MSRV is 1.53 for use of or patterns (the readme says 1.52). |
|
Perfect, it's not an issue for rkyv users then. I'll probably follow suit and switch to feature resolver 2 (I believe there are some feature issues currently). |
|
Hmm, even with feature resolver 2 I'm still getting the clippy build fail if I disable default features on rkyv as suggested here #223 (comment) |
|
Hmm, that's not the result I got when I did some testing. I'll take a look as soon as I have a chance. |
Added rkyv support