From 414c0104c32cae046d4026f2255ded73ce7091fb Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Sun, 31 Oct 2021 23:13:58 +0530 Subject: [PATCH] Rework Joy-Con Vibration Conversion The guest -> host vibration conversion code was entirely broken as it didn't set the vibration `start`/`end` timestamps correctly for a cycle nor did it subtract from the `totalAmplitude` (`currentAmplitude` now) when it a cycle ended due to an incorrect `if` statement and contents. It would just end up saturating the amplitude as much as possible by adding more and more to `totalAmplitude` on every cycle while never subtracting which is entirely wrong and led to a very noticeable drop in amplitude when a vibration was repeated. It's been entirely reworked to fix all the issues listed above and remove a lot of code that had no understandable purpose. More comments have also been added to the code to make it more readable with better variable and function naming alongside it. --- .../main/cpp/skyline/input/npad_device.cpp | 115 +++++++++--------- app/src/main/cpp/skyline/input/npad_device.h | 14 ++- .../cpp/skyline/services/hid/IHidServer.cpp | 4 +- 3 files changed, 68 insertions(+), 65 deletions(-) diff --git a/app/src/main/cpp/skyline/input/npad_device.cpp b/app/src/main/cpp/skyline/input/npad_device.cpp index 9c94bed8..bd05913f 100644 --- a/app/src/main/cpp/skyline/input/npad_device.cpp +++ b/app/src/main/cpp/skyline/input/npad_device.cpp @@ -359,8 +359,8 @@ namespace skyline::input { struct VibrationInfo { jlong period; jint amplitude; - jlong start; - jlong end; + jlong start; //!< The timestamp to (re)start the vibration at + jlong end; //!< The timestamp to end the vibration at VibrationInfo(float frequency, float amplitude) : period(static_cast(MsInSecond / frequency)), @@ -369,22 +369,12 @@ namespace skyline::input { }; template - void VibrateDevice(const std::shared_ptr &jvm, i8 index, std::array vibrations) { - jlong totalTime{}; - std::sort(vibrations.begin(), vibrations.end(), [](const VibrationInfo &a, const VibrationInfo &b) { - return a.period < b.period; - }); - - jlong totalPeriod{}; + void VibrateDevice(const std::shared_ptr &jvm, i8 deviceIndex, std::array vibrations) { jint totalAmplitude{}; - for (const auto &vibration : vibrations) { - totalPeriod += vibration.period; + for (const auto &vibration : vibrations) totalAmplitude += vibration.amplitude; - } - - // If this vibration is essentially null then we don't play rather clear any running vibrations - if (totalPeriod == 0 || totalAmplitude == 0) { - jvm->ClearVibrationDevice(index); + if (totalAmplitude == 0) { + jvm->ClearVibrationDevice(deviceIndex); // If a null vibration was submitted then we just clear vibrations on the device return; } @@ -393,47 +383,75 @@ namespace skyline::input { std::array amplitudes; // We are essentially unrolling the bands into a linear sequence, due to the data not being always linearizable there will be inaccuracies at the ends unless there's a pattern that's repeatable which will happen when all band's frequencies are factors of each other - size_t timingIndex{}; - for (; timingIndex < timings.size(); timingIndex++) { - jlong time{}; + jint currentAmplitude{}; //!< The accumulated amplitude from adding up and subtracting the amplitude of individual bands + jlong currentTime{}; //!< The accumulated time passed by adding up all the periods prior to the current vibration cycle + size_t index{}; + for (; index < timings.size(); index++) { + jlong cyclePeriod{}; //!< The length of this cycle, calculated as the largest period with the same amplitude + size_t bandStartCount{}; //!< The amount of bands that start their vibration cycles in this time slot - size_t startCycleCount{}; for (size_t vibrationIndex{}; vibrationIndex < vibrations.size(); vibrationIndex++) { - auto &vibration{vibrations[vibrationIndex]}; - if (totalTime <= vibration.start) { - vibration.start = vibration.end + vibration.period; - totalAmplitude += vibration.amplitude; - time = std::max(vibration.period, time); - startCycleCount++; - } else if (totalTime <= vibration.start) { + // Iterate over every band to calculate the amplitude for this time slot + VibrationInfo &vibration{vibrations[vibrationIndex]}; + if (currentTime <= vibration.start) { + // If the time to start has arrived then start the vibration vibration.end = vibration.start + vibration.period; - totalAmplitude -= vibration.amplitude; - time = std::max(vibration.period, time); + currentAmplitude += vibration.amplitude; + auto vibrationPeriodLeft{vibration.end - currentTime}; + cyclePeriod = cyclePeriod ? std::min(vibrationPeriodLeft, cyclePeriod) : vibrationPeriodLeft; + + bandStartCount++; + } else if (currentTime <= vibration.end) { + // If the time to end the vibration has arrived then end it + vibration.start = vibration.end + vibration.period; + currentAmplitude -= vibration.amplitude; + auto vibrationPeriodLeft{vibration.start - currentTime}; + cyclePeriod = cyclePeriod ? std::min(vibrationPeriodLeft, cyclePeriod) : vibrationPeriodLeft; } } - // If all bands start again at this point then we can end the pattern here as a loop to the front will be flawless - if (timingIndex && startCycleCount == vibrations.size()) - break; + if (index && bandStartCount == vibrations.size()) + break; // If all bands start again at this point then we can end the pattern here and just loop over the pattern - timings[timingIndex] = time; - totalTime += time; + currentTime += cyclePeriod; + timings[index] = cyclePeriod; - amplitudes[timingIndex] = std::min(totalAmplitude, AmplitudeMax); + amplitudes[index] = std::min(currentAmplitude, AmplitudeMax); } - jvm->VibrateDevice(index, span(timings.begin(), timings.begin() + timingIndex), span(amplitudes.begin(), amplitudes.begin() + timingIndex)); + jvm->VibrateDevice(deviceIndex, span(timings.begin(), timings.begin() + index), span(amplitudes.begin(), amplitudes.begin() + index)); } void VibrateDevice(const std::shared_ptr &jvm, i8 index, const NpadVibrationValue &value) { std::array vibrations{ VibrationInfo{value.frequencyLow, value.amplitudeLow * (AmplitudeMax / 2)}, - {value.frequencyHigh, value.amplitudeHigh * (AmplitudeMax / 2)}, + VibrationInfo{value.frequencyHigh, value.amplitudeHigh * (AmplitudeMax / 2)}, }; VibrateDevice(jvm, index, vibrations); } - void NpadDevice::Vibrate(bool isRight, const NpadVibrationValue &value) { + void NpadDevice::Vibrate(const NpadVibrationValue &left, const NpadVibrationValue &right) { + if (vibrationLeft == left && vibrationRight && (*vibrationRight) == right) + return; + + vibrationLeft = left; + vibrationRight = right; + + if (partnerIndex == NpadDevice::NullIndex) { + std::array vibrations{ + VibrationInfo{left.frequencyLow, left.amplitudeLow * (AmplitudeMax / 4)}, + VibrationInfo{left.frequencyHigh, left.amplitudeHigh * (AmplitudeMax / 4)}, + VibrationInfo{right.frequencyLow, right.amplitudeLow * (AmplitudeMax / 4)}, + VibrationInfo{right.frequencyHigh, right.amplitudeHigh * (AmplitudeMax / 4)}, + }; + VibrateDevice(manager.state.jvm, index, vibrations); + } else { + VibrateDevice(manager.state.jvm, index, left); + VibrateDevice(manager.state.jvm, partnerIndex, right); + } + } + + void NpadDevice::VibrateSingle(bool isRight, const NpadVibrationValue &value) { if (isRight) { if (vibrationRight && (*vibrationRight) == value) return; @@ -449,25 +467,4 @@ namespace skyline::input { else VibrateDevice(manager.state.jvm, index, value); } - - void NpadDevice::Vibrate(const NpadVibrationValue &left, const NpadVibrationValue &right) { - if (vibrationLeft == left && vibrationRight && (*vibrationRight) == right) - return; - - vibrationLeft = left; - vibrationRight = right; - - if (partnerIndex == NpadDevice::NullIndex) { - std::array vibrations{ - VibrationInfo{left.frequencyLow, left.amplitudeLow * (AmplitudeMax / 4)}, - {left.frequencyHigh, left.amplitudeHigh * (AmplitudeMax / 4)}, - {right.frequencyLow, right.amplitudeLow * (AmplitudeMax / 4)}, - {right.frequencyHigh, right.amplitudeHigh * (AmplitudeMax / 4)}, - }; - VibrateDevice(manager.state.jvm, index, vibrations); - } else { - VibrateDevice(manager.state.jvm, index, left); - VibrateDevice(manager.state.jvm, partnerIndex, right); - } - } } diff --git a/app/src/main/cpp/skyline/input/npad_device.h b/app/src/main/cpp/skyline/input/npad_device.h index 9e61baf1..74a3b26f 100644 --- a/app/src/main/cpp/skyline/input/npad_device.h +++ b/app/src/main/cpp/skyline/input/npad_device.h @@ -114,7 +114,7 @@ namespace skyline::input { private: NpadManager &manager; //!< The manager responsible for managing this NpadDevice NpadSection §ion; //!< The section in HID shared memory for this controller - NpadControllerInfo *controllerInfo; //!< The NpadControllerInfo for this controller's type + NpadControllerInfo *controllerInfo{}; //!< The NpadControllerInfo for this controller's type u64 globalTimestamp{}; //!< An incrementing timestamp that's common across all sections /** @@ -134,7 +134,7 @@ namespace skyline::input { static constexpr i8 NullIndex{-1}; //!< The placeholder index value when there is no device present i8 index{NullIndex}; //!< The index of the device assigned to this player i8 partnerIndex{NullIndex}; //!< The index of a partner device, if present - NpadVibrationValue vibrationLeft; //!< Vibration for the left Joy-Con (Handheld/Pair), left LRA in a Pro-Controller or individual Joy-Cons + NpadVibrationValue vibrationLeft{}; //!< Vibration for the left Joy-Con (Handheld/Pair), left LRA in a Pro-Controller or individual Joy-Cons std::optional vibrationRight; //!< Vibration for the right Joy-Con (Handheld/Pair) or right LRA in a Pro-Controller NpadControllerType type{}; NpadConnectionState connectionState{}; @@ -174,8 +174,14 @@ namespace skyline::input { */ void SetAxisValue(NpadAxisId axis, i32 value); - void Vibrate(bool isRight, const NpadVibrationValue &value); - + /** + * @brief Sets the vibration for both the Joy-Cons to the specified vibration values + */ void Vibrate(const NpadVibrationValue &left, const NpadVibrationValue &right); + + /** + * @brief Sets the vibration for either the left or right Joy-Con to the specified vibration value + */ + void VibrateSingle(bool isRight, const NpadVibrationValue &value); }; } diff --git a/app/src/main/cpp/skyline/services/hid/IHidServer.cpp b/app/src/main/cpp/skyline/services/hid/IHidServer.cpp index 9b2791ea..17ddd7c5 100644 --- a/app/src/main/cpp/skyline/services/hid/IHidServer.cpp +++ b/app/src/main/cpp/skyline/services/hid/IHidServer.cpp @@ -148,7 +148,7 @@ namespace skyline::service::hid { if (device.type == handle.GetType()) { const auto &value{request.Pop()}; state.logger->Debug("Vibration - Handle: 0x{:02X} (0b{:05b}), Vibration: {:.2f}@{:.2f}Hz, {:.2f}@{:.2f}Hz", static_cast(handle.id), static_cast(handle.type), value.amplitudeLow, value.frequencyLow, value.amplitudeHigh, value.frequencyHigh); - device.Vibrate(handle.isRight, value); + device.VibrateSingle(handle.isRight, value); } return {}; @@ -172,7 +172,7 @@ namespace skyline::service::hid { } else { const auto &value{values[i]}; state.logger->Debug("Vibration #{} - Handle: 0x{:02X} (0b{:05b}), Vibration: {:.2f}@{:.2f}Hz, {:.2f}@{:.2f}Hz", i, static_cast(handle.id), static_cast(handle.type), value.amplitudeLow, value.frequencyLow, value.amplitudeHigh, value.frequencyHigh); - device.Vibrate(handle.isRight, value); + device.VibrateSingle(handle.isRight, value); } } }