relativty / Relativty

An open source VR headset with SteamVR supports for $200
https://www.relativty.com/
GNU General Public License v3.0
6.65k stars 347 forks source link

Formatting and arrays #74

Open Dudecake opened 3 years ago

Dudecake commented 3 years ago

Notable fixes

Fixed array out-of-range in Normalize

In Normalize an attempt is made te read index 3 of a c-array of length 3

Replaced the c-arrays of std::atomic<float> with std::array<float> guarded by locks

With how the quaternions were copied, race conditions were still possible. Especially in Relativty::HMDDriver::calibrate_quaternion. Relativty::HMDDriver::qconj was only written to in Relativty::HMDDriver::calibrate_quaternion, so locking that is a waste.

Replaced assignments to quaternion arrays with std::copys and a std::transform

The compiler can optionally optimise std::copys to memcpys or vector operations.

Replaced calls to Relativty::HmdQuaternion_Init with braced initialization

Rename Relativty::ServerDriver::HMDDriver to m_HMDDriver

The distinction between the class and the instance couldn't be made otherwise. No idea how MSVC handled that...

Replaced NULLs with either nullptrs or 0s

Some NULLs imply the arguments are pointers when they are ints

Various formatting changes and renamed Relativty::HMDDriver::new_vector_avaiable to Relativty::HMDDriver::new_vector_available

okawo80085 commented 3 years ago

apart from that struct change, everything else looks ok at first glance, didn't check it with actual hardware yet

i recommend you compile it, and at least test is with real hardware for while before merging this

okawo80085 commented 3 years ago

just dropping by, did anyone try this with actual hardware?

Dudecake commented 3 years ago

I haven't had the time to source the components for a unit for me, so I wasn't able to test it. I figured the changes weren't radical enough and the reviewer would still need to test it themselves over trusting me on my (proverbial) blue eyes.