Skip to content

Commit

Permalink
Dropped strict enforcement of mutual exclusion between virtual contro…
Browse files Browse the repository at this point in the history 
…ller objects when registering for force feedback.
  • Loading branch information
@samuelgr
samuelgr committed Jul 23, 2022
1 parent 779251b commit 2ca5716
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 57 deletions.
35 changes: 21 additions & 14 deletions Include/Xidi/Test/MockPhysicalController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "VirtualController.h"

#include <cstddef>
#include <set>


namespace XidiTest
Expand Down Expand Up @@ -56,8 +57,8 @@ namespace XidiTest
/// Initialized to use a base timestamp of 0.
ForceFeedback::Device forceFeedbackDevice;

/// Virtual controller registered for force feedback.
const VirtualController* forceFeedbackRegistration;
/// Virtual controllers registered for force feedback.
std::set<const VirtualController*> forceFeedbackRegistration;


public:
Expand All @@ -80,6 +81,13 @@ namespace XidiTest
/// Intended to be invoked internally only.
void AdvancePhysicalState(void);

/// Unregisters a virtual controller for force feedback.
/// @param [in] controllerToRegister Pointer to the virtual controller object that should be unregistered for force feedback.
inline void EraseForceFeedbackRegistration(const VirtualController* controllerToUnregister)
{
forceFeedbackRegistration.erase(controllerToUnregister);
}

/// Retrieves and returns the current physical state.
/// @return Current physical state being reported to the test cases that request it.
SPhysicalState GetCurrentPhysicalState(void) const;
Expand All @@ -91,11 +99,18 @@ namespace XidiTest
return forceFeedbackDevice;
}

/// Provides access to the virtual controller object that is registered for force feedback.
/// @return Pointer to the registered virtual controller, or `nullptr` if no virtual controller is registered.
inline const VirtualController* GetForceFeedbackRegistration(void) const
/// Registers a virtual controller for force feedback.
/// @param [in] controllerToRegister Pointer to the virtual controller object that should be registered for force feedback.
inline void InsertForceFeedbackRegistration(const VirtualController* controllerToRegister)
{
forceFeedbackRegistration.insert(controllerToRegister);
}

/// Checks if the specified virtual controller is registered for force feedback.
/// @return `true` if so, `false` if not.
inline bool IsVirtualControllerRegisteredForForceFeedback(const VirtualController* controllerToCheck)
{
return forceFeedbackRegistration;
return forceFeedbackRegistration.contains(controllerToCheck);
}

/// Retrieves and returns the controller identifier associated with this object.
Expand All @@ -115,13 +130,5 @@ namespace XidiTest
/// Requests an advancement to the next physical state.
/// Test will fail due to a test implementation issue if attempting to advance past the end of the physical state array.
void RequestAdvancePhysicalState(void);

/// Sets the virtual controller that is registered for force feedback.
/// Passing `nullptr` clears the registration entirely.
/// @param [in] controllerToRegister Pointer to the virtual controller object that should be registered for force feedback.
inline void SetForceFeedbackRegistration(const VirtualController* controllerToRegister)
{
forceFeedbackRegistration = controllerToRegister;
}
};
}
2 changes: 1 addition & 1 deletion Include/Xidi/VirtualController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ namespace Xidi
static constexpr uint32_t kFfGainMax = 10000;

/// Default value for force feedback gain. No scaling down of effects by default.
static constexpr uint32_t kFfGainDefault = 10000;
static constexpr uint32_t kFfGainDefault = kFfGainMax;


// -------- TYPE DEFINITIONS ----------------------------------- //
Expand Down
30 changes: 14 additions & 16 deletions Source/PhysicalController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@
#include "ForceFeedbackDevice.h"
#include "Globals.h"
#include "ImportApiWinMM.h"
#include "Mapper.h"
#include "Message.h"
#include "PhysicalController.h"
#include "VirtualController.h"

#include <condition_variable>
#include <cstdint>
#include <set>
#include <shared_mutex>
#include <stop_token>
#include <thread>
Expand Down Expand Up @@ -77,7 +79,7 @@ namespace Xidi
static ForceFeedback::Device* physicalControllerForceFeedbackBuffer;

/// Pointers to the virtual controller objects registered for force feedback with each physical controller.
static const VirtualController* physicalControllerForceFeedbackRegistration[kPhysicalControllerCount];
static std::set<const VirtualController*> physicalControllerForceFeedbackRegistration[kPhysicalControllerCount];

/// Mutex objects for protecting against concurrent accesses to the physical controller force feedback registration data.
static std::mutex physicalControllerForceFeedbackMutex[kPhysicalControllerCount];
Expand All @@ -100,15 +102,21 @@ namespace Xidi

if (true == Globals::DoesCurrentProcessHaveInputFocus())
{
ForceFeedback::TEffectValue overallEffectGain = 10000;
ForceFeedback::SPhysicalActuatorComponents physicalActuatorVector = {};
ForceFeedback::TOrderedMagnitudeComponents virtualMagnitudeVector = physicalControllerForceFeedbackBuffer[controllerIdentifier].PlayEffects();

if (kVirtualMagnitudeVectorZero != virtualMagnitudeVector)
{
std::unique_lock lock(physicalControllerForceFeedbackMutex[controllerIdentifier]);

if (nullptr != physicalControllerForceFeedbackRegistration[controllerIdentifier])
physicalActuatorVector = physicalControllerForceFeedbackRegistration[controllerIdentifier]->ForceFeedbackMapVirtualToPhysical(virtualMagnitudeVector);
// Gain is modified downwards by each virtual controller object.
// Typically there would only be one, in which case the properties of that object would be effective.
// Otherwise this loop is essentially modeled as multiple volume knobs connected in sequence, each lowering the volume of the effects by the value of its own device-wide gain property.
for (auto virtualController : physicalControllerForceFeedbackRegistration[controllerIdentifier])
overallEffectGain *= ((ForceFeedback::TEffectValue)virtualController->GetForceFeedbackGain() / ForceFeedback::kEffectModifierMaximum);

physicalActuatorVector = Mapper::GetConfigured(controllerIdentifier)->MapForceFeedbackVirtualToPhysical(virtualMagnitudeVector, overallEffectGain);
}

// Currently the impulse trigger values are ignored.
Expand Down Expand Up @@ -286,14 +294,9 @@ namespace Xidi
}

std::unique_lock lock(physicalControllerForceFeedbackMutex[controllerIdentifier]);
physicalControllerForceFeedbackRegistration[controllerIdentifier].insert(virtualController);

if (nullptr == physicalControllerForceFeedbackRegistration[controllerIdentifier])
{
physicalControllerForceFeedbackRegistration[controllerIdentifier] = virtualController;
return &physicalControllerForceFeedbackBuffer[controllerIdentifier];
}

return nullptr;
return &physicalControllerForceFeedbackBuffer[controllerIdentifier];
}

// --------
Expand All @@ -309,12 +312,7 @@ namespace Xidi
}

std::unique_lock lock(physicalControllerForceFeedbackMutex[controllerIdentifier]);

if (virtualController == physicalControllerForceFeedbackRegistration[controllerIdentifier])
{
physicalControllerForceFeedbackRegistration[controllerIdentifier] = nullptr;
physicalControllerForceFeedbackBuffer[controllerIdentifier].Clear();
}
physicalControllerForceFeedbackRegistration[controllerIdentifier].erase(virtualController);
}

// --------
Expand Down
22 changes: 8 additions & 14 deletions Source/Test/Case/VirtualControllerTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -846,20 +846,20 @@ namespace XidiTest

TEST_ASSERT(false == controller.ForceFeedbackIsRegistered());
TEST_ASSERT(nullptr == controller.ForceFeedbackGetDevice());
TEST_ASSERT(nullptr == physicalController.GetForceFeedbackRegistration());
TEST_ASSERT(false == physicalController.IsVirtualControllerRegisteredForForceFeedback(&controller));

TEST_ASSERT(true == controller.ForceFeedbackRegister());
TEST_ASSERT(kForceFeedbackDeviceAddress == controller.ForceFeedbackGetDevice());
TEST_ASSERT(&controller == physicalController.GetForceFeedbackRegistration());
TEST_ASSERT(true == physicalController.IsVirtualControllerRegisteredForForceFeedback(&controller));

controller.ForceFeedbackUnregister();
TEST_ASSERT(false == controller.ForceFeedbackIsRegistered());
TEST_ASSERT(nullptr == controller.ForceFeedbackGetDevice());
TEST_ASSERT(nullptr == physicalController.GetForceFeedbackRegistration());
TEST_ASSERT(false == physicalController.IsVirtualControllerRegisteredForForceFeedback(&controller));
}

// Verifies that only one virtual controller is allowed to register at a time, even with the same controller identifier.
TEST_CASE(VirtualController_ForceFeedback_MutualExclusion)
// Verifies that multiple virtual controllers are allowed to register at a time.
TEST_CASE(VirtualController_ForceFeedback_MultipleRegistrations)
{
constexpr TControllerIdentifier kControllerIndex = 1;
constexpr SPhysicalState kPhysicalState = {.errorCode = ERROR_SUCCESS, .state = {.dwPacketNumber = 1}};
Expand All @@ -875,12 +875,6 @@ namespace XidiTest

TEST_ASSERT(true == controller.ForceFeedbackRegister());
TEST_ASSERT(true == controller.ForceFeedbackIsRegistered());
TEST_ASSERT(false == controller2.ForceFeedbackRegister());
TEST_ASSERT(false == controller2.ForceFeedbackIsRegistered());

controller.ForceFeedbackUnregister();
TEST_ASSERT(false == controller.ForceFeedbackIsRegistered());

TEST_ASSERT(true == controller2.ForceFeedbackRegister());
TEST_ASSERT(true == controller2.ForceFeedbackIsRegistered());
}
Expand All @@ -900,7 +894,7 @@ namespace XidiTest
TEST_ASSERT(true == controller.ForceFeedbackRegister());

TEST_ASSERT(kForceFeedbackDeviceAddress == controller.ForceFeedbackGetDevice());
TEST_ASSERT(&controller == physicalController.GetForceFeedbackRegistration());
TEST_ASSERT(true == physicalController.IsVirtualControllerRegisteredForForceFeedback(&controller));
}

// Verifies that virtual controllers automatically unregister themselves upon destruction.
Expand All @@ -914,9 +908,9 @@ namespace XidiTest

VirtualController* controller = new VirtualController(kControllerIndex, kTestMapper);
TEST_ASSERT(true == controller->ForceFeedbackRegister());
TEST_ASSERT(controller == physicalController.GetForceFeedbackRegistration());
TEST_ASSERT(true == physicalController.IsVirtualControllerRegisteredForForceFeedback(controller));

delete controller;
TEST_ASSERT(nullptr == physicalController.GetForceFeedbackRegistration());
TEST_ASSERT(false == physicalController.IsVirtualControllerRegisteredForForceFeedback(controller));
}
}
10 changes: 3 additions & 7 deletions Source/Test/MockPhysicalController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,8 @@ namespace Xidi

if (nullptr != mockPhysicalController[controllerIdentifier])
{
if (nullptr == mockPhysicalController[controllerIdentifier]->GetForceFeedbackRegistration())
{
mockPhysicalController[controllerIdentifier]->SetForceFeedbackRegistration(virtualController);
return &mockPhysicalController[controllerIdentifier]->GetForceFeedbackDevice();
}
mockPhysicalController[controllerIdentifier]->InsertForceFeedbackRegistration(virtualController);
return &mockPhysicalController[controllerIdentifier]->GetForceFeedbackDevice();
}

return nullptr;
Expand All @@ -160,8 +157,7 @@ namespace Xidi

if (nullptr != mockPhysicalController[controllerIdentifier])
{
if (virtualController == mockPhysicalController[controllerIdentifier]->GetForceFeedbackRegistration())
mockPhysicalController[controllerIdentifier]->SetForceFeedbackRegistration(nullptr);
mockPhysicalController[controllerIdentifier]->EraseForceFeedbackRegistration(virtualController);
}
}

Expand Down
10 changes: 5 additions & 5 deletions Source/VirtualDirectInputDevice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -880,8 +880,9 @@ namespace Xidi
switch (cooperativeLevel)
{
case ECooperativeLevel::Exclusive:
// In exclusive mode, the virtual controller takes exclusive control over the physical controller's force feedback buffer.
// Acquisition is modeled as registering successfully for such control.
// In exclusive mode, the virtual controller gets access to the physical controller's force feedback buffer.
// Normally DirectInput would enforce mutual exclusion between applications, hence the "exclusive" terminology.
// Xidi does not enforce mutual exclusion at all. However, in the interest of compabitility with DirectInput, Xidi does require exclusive acquisition in order for force feedback to be available.

Message::OutputFormatted(kMethodSeverity, L"Acquiring Xidi virtual controller %u in exclusive mode.", (1 + controller->GetIdentifier()));

Expand All @@ -891,9 +892,8 @@ namespace Xidi
if (true == controller->ForceFeedbackRegister())
LOG_INVOCATION_AND_RETURN(DI_OK, kMethodSeverity);

// Getting to this point means another object has already acquired exclusive access to the physical device.
// DirectInput would normally enforce mutual exclusion between multiple applications, whereas Xidi only does so between multiple objects within the same application.
LOG_INVOCATION_AND_RETURN(DIERR_OTHERAPPHASPRIO, Message::ESeverity::Warning);
// Getting to this point means force feedback registration failed. This should not normally occur.
LOG_INVOCATION_AND_RETURN(DIERR_OTHERAPPHASPRIO, Message::ESeverity::Error);

default:
// No other cooperative level requires any action on Xidi's part for the acquisition to succeed.
Expand Down

0 comments on commit 2ca5716

Please sign in to comment.