-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Does not compile on latest nightly (SIMD vector's only field must be an array
)
#96
Comments
Thanks for reporting this. I could probably write an essay about how much I hate this change, but what really matters are the actions I take. My knee jerk reaction is : at this point the most sensible thing to do is for And that's it. The alternative ("solution 2") is to still support Your proposal to remove But man... This is going to be a pain. Granted, we're in unstable territory, but it's still something that's suddenly coming up out of nowhere to eat my time, and there's nothing in it for me. Just a free waste of time. Gotta love it |
Thanks for the quick update. I wonder if there's a way to fight this change. It seems this whole change, both the proposal and implementation, was done by one person. And although the justification is about simplifying the code and the chance affects many files, most of them are tests and the change to real code is small. Finally, they never even considered that people could be using named fields, all they talked about was tuple structs. Correct me if i am wrong but this feature was the only way to have sane notation like If so, this is a major usability downgrade. One of rust's massive flaws that hit me repeatedly in gamedev is its lack of fields in traits and here we run into it again because the best interface you can achieve now is What do you think about writing a post on https://internals.rust-lang.org/ to argue for undoing this change? |
There are unfortunately a lot of counterarguments, and because I know them, I can't go out there and stand my ground in good faith:
But perhaps there's another way 🤔 I just wrote a small experiment with It remains to be seen whether that could still make "going from repr_c to repr_simd" non-seamless, but we're already past that point. |
My workaround wouldn't allow for construction by members or destructuration : let v = simd::Vec4 { x: ... ... };
let simd::Vec4 { x, y, ... } = v; the most "direct" equivalent would be to use let v: simd::Vec4 = c::Vec4 { x: ... ... }.into();
let c::Vec4 { x, y, ... } = v.into(); A slight inconvenience... Could be worse though |
The proposal, yes, but the implementation was only merged now and i found the issue as soon as it was merged because i use nightly on CI. I agree it's weird nobody else has noticed though.
This is death by a thousand papercuts for gamedev. Having so much extra special characters as noise in a complex expression is painful and i really wish Rust lang designers understood that.
That's a fair point, i've never benchmarked my game because TBH perf is rarely an issue in (rust) gamedev. Getting things done is :)
Good idea, that could work. I also "abuse"
Papercuts. Though in these particular cases, I don't mind and it's a good tradeoff given the choices.
Understandable, I don't have time to work on my game now either, i only noticed because I set CI to run weekly even with no commits. And everything runs perfectly OK without SIMD as well. |
The feature
repr_simd
no longer supports structs with multiple fields.The fix is to convert vek's vectors to use arrays internally. I haven't looked at how much work it would be.
The workaround is to disable vek's default features and only use the ones you need - e.g.
vek = { version = "0.17.0", default-features = false, features = ["rgba", "rgb", "std"] }
.I propose that a quick temporary solution would be to publish 0.17.1 with
repr_simd
removed from the list of default features.The text was updated successfully, but these errors were encountered: