Skip to content

Hd/fix input registering domain reload #3373

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

Merged
merged 7 commits into from
Feb 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions com.unity.render-pipelines.core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

### Changed
- Volume Gizmo Color setting is now under Colors->Scene->Volume Gizmo
- Optimized InputRegistering computational time.

## [11.0.0] - 2020-10-21

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ internal float GetAction(DebugAction action)

void RegisterInputs()
{
#if UNITY_EDITOR
#if UNITY_EDITOR && !USE_INPUT_SYSTEM
var inputEntries = new List<InputManagerEntry>
{
new InputManagerEntry { name = kEnableDebugBtn1, kind = InputManagerEntry.Kind.KeyOrButton, btnPositive = "left ctrl", altBtnPositive = "joystick button 8" },
Expand Down
157 changes: 118 additions & 39 deletions com.unity.render-pipelines.core/Runtime/Inputs/InputRegistering.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,45 +11,36 @@ public enum Kind { KeyOrButton, Mouse, Axis }
public enum Axis { X, Y, Third, Fourth, Fifth, Sixth, Seventh, Eigth }
public enum Joy { All, First, Second }

public string name = "";
public string desc = "";
public string btnNegative = "";
public string btnPositive = "";
public string altBtnNegative = "";
public string altBtnPositive = "";
public float gravity = 0.0f;
public float deadZone = 0.0f;
public float sensitivity = 0.0f;
public bool snap = false;
public bool invert = false;
public Kind kind = Kind.Axis;
public Axis axis = Axis.X;
public Joy joystick = Joy.All;
public string name = "";
public string desc = "";
public string btnNegative = "";
public string btnPositive = "";
public string altBtnNegative = "";
public string altBtnPositive = "";
public float gravity = 0.0f;
public float deadZone = 0.0f;
public float sensitivity = 0.0f;
public bool snap = false;
public bool invert = false;
public Kind kind = Kind.Axis;
public Axis axis = Axis.X;
public Joy joystick = Joy.All;

internal bool IsEqual((string name, InputManagerEntry.Kind kind) partialEntry)
=> this.name == partialEntry.name && this.kind == partialEntry.kind;

internal bool IsEqual(InputManagerEntry other)
=> this.name == other.name && this.kind == other.kind;
}

public class InputRegistering
public static class InputRegistering
{
static bool InputAlreadyRegistered(string name, InputManagerEntry.Kind kind, SerializedProperty spAxes)
{
for (var i = 0; i < spAxes.arraySize; ++i)
{
var spAxis = spAxes.GetArrayElementAtIndex(i);
var axisName = spAxis.FindPropertyRelative("m_Name").stringValue;
var kindValue = spAxis.FindPropertyRelative("type").intValue;
if (axisName == name && (int)kind == kindValue)
return true;
}
static List<InputManagerEntry> s_PendingInputsToRegister = new List<InputManagerEntry>();

return false;
}
static bool havePendingOperation => s_PendingInputsToRegister.Count > 0;

static void WriteEntry(SerializedProperty spAxes, InputManagerEntry entry)
static void CopyEntry(SerializedProperty spAxis, InputManagerEntry entry)
{
if (InputAlreadyRegistered(entry.name, entry.kind, spAxes))
return;

spAxes.InsertArrayElementAtIndex(spAxes.arraySize);
var spAxis = spAxes.GetArrayElementAtIndex(spAxes.arraySize - 1);
spAxis.FindPropertyRelative("m_Name").stringValue = entry.name;
spAxis.FindPropertyRelative("descriptiveName").stringValue = entry.desc;
spAxis.FindPropertyRelative("negativeButton").stringValue = entry.btnNegative;
Expand All @@ -66,8 +57,75 @@ static void WriteEntry(SerializedProperty spAxes, InputManagerEntry entry)
spAxis.FindPropertyRelative("joyNum").intValue = (int)entry.joystick;
}

public static void RegisterInputs(List<InputManagerEntry> entries)
static void AddEntriesWithoutCheck(SerializedProperty spAxes, List<InputManagerEntry> newEntries)
{
int endOfCurrentInputList = spAxes.arraySize;
spAxes.arraySize = endOfCurrentInputList + newEntries.Count;

SerializedProperty spAxis = spAxes.GetArrayElementAtIndex(endOfCurrentInputList);
for (int i = 0; i < newEntries.Count; ++i, spAxis.Next(false))
CopyEntry(spAxis, newEntries[i]);
}

// Get a representation of the already registered inputs
static List<(string name, InputManagerEntry.Kind kind)> GetCachedInputs(SerializedProperty spAxes)
{
int size = spAxes.arraySize;
List<(string name, InputManagerEntry.Kind kind)> result = new List<(string name, InputManagerEntry.Kind kind)>(size);

SerializedProperty spAxis = spAxes.GetArrayElementAtIndex(0);
for (int i = 0; i < size; ++i, spAxis.Next(false))
result.Add((spAxis.FindPropertyRelative("m_Name").stringValue, (InputManagerEntry.Kind)spAxis.FindPropertyRelative("type").intValue));
return result;
}

static void MakeUniquePendingInputsToRegister()
{
for (int pendingIndex = s_PendingInputsToRegister.Count - 1; pendingIndex > 0; --pendingIndex)
{
InputManagerEntry pendingEntry = s_PendingInputsToRegister[pendingIndex];
int checkedIndex = pendingIndex - 1;
for (; checkedIndex > -1 && !pendingEntry.IsEqual(s_PendingInputsToRegister[checkedIndex]); --checkedIndex) ;

if (checkedIndex == -1)
continue;

// There is a duplicate entry in PendingInputesToRegister.
// Debug.LogWarning($"Two entries with same name and kind are tryed to be added at same time. Only first occurence is kept. Name:{pendingEntry.name} Kind:{pendingEntry.kind}");

// Keep only first.
// Counting decreasingly will have no impact on index before pendingIndex. So we can safely remove it.
s_PendingInputsToRegister.RemoveAt(pendingIndex);
}
}

static void RemovePendingInputsToAddThatAreAlreadyRegistered(List<(string name, InputManagerEntry.Kind kind)> cachedEntries, List<InputManagerEntry> newEntries)
{
for (int newIndex = newEntries.Count - 1; newIndex >= 0; --newIndex)
{
var newEntry = newEntries[newIndex];
int checkedIndex = cachedEntries.Count - 1;
for (; checkedIndex > -1 && !newEntry.IsEqual(cachedEntries[checkedIndex]); --checkedIndex) ;

if (checkedIndex == -1)
continue;

// There is a already a cached entry that correspond.
// Debug.LogWarning($"Another entry with same name and kind already exist. Skiping this one. Name:{newEntry.name} Kind:{newEntry.kind}");

// Keep only first.
// Counting decreasingly will have no impact on index before pendingIndex. So we can safely remove it.
s_PendingInputsToRegister.RemoveAt(newIndex);
}
}

static void DelayedRegisterInput()
{
// Exit quickly if nothing more to register
// (case when several different class try to register, only first call will do all)
if (!havePendingOperation)
return;

// Grab reference to input manager
var assets = AssetDatabase.LoadAllAssetsAtPath("ProjectSettings/InputManager.asset");
// Temporary fix. This happens some time with HDRP init when it's called before asset database is initialized (probably related to package load order).
Expand All @@ -76,18 +134,39 @@ public static void RegisterInputs(List<InputManagerEntry> entries)

var inputManager = assets[0];

// Wrap in serialized object
// Wrap in serialized object to access c++ fields
var soInputManager = new SerializedObject(inputManager);
var spAxes = soInputManager.FindProperty("m_Axes");

foreach (InputManagerEntry entry in entries)
{
WriteEntry(spAxes, entry);
}
// At this point, we assume that entries in spAxes are already unique.

// Ensure no double entry are tried to be registered (trim early)
MakeUniquePendingInputsToRegister();

// Cache already existing entries to minimaly use serialization
var cachedEntries = GetCachedInputs(spAxes);
// And trim pending entries regarding already cached ones.
RemovePendingInputsToAddThatAreAlreadyRegistered(cachedEntries, s_PendingInputsToRegister);

// Add now unique entries
AddEntriesWithoutCheck(spAxes, s_PendingInputsToRegister);

// Commit
soInputManager.ApplyModifiedProperties();
}

public static void RegisterInputs(List<InputManagerEntry> entries)
{
#if ENABLE_INPUT_SYSTEM && ENABLE_INPUT_SYSTEM_PACKAGE
Debug.LogWarning("Trying to add entry in the legacy InputManager but using InputSystem package. Skiping.");
return;
#else
s_PendingInputsToRegister.AddRange(entries);

//delay the call in order to do only one pass event if several different class register inputs
EditorApplication.delayCall += DelayedRegisterInput;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is replaced by DelayedRegisterInput(); in the test to compare algorithm in same conditions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean we could end up calling this function multiple times in the same frame?
also when do we unregister the call?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it means you can call it multiple time in a frame, it will all be batched in delayed call.
Example:
Frame1, two different classes call register input

RegisterInputs(new List<InputManagerEntry>
      {
            new InputManagerEntry { name = "cmd1",  kind = InputManagerEntry.Kind.KeyOrButton, btnPositive = "left ctrl",   altBtnPositive = "joystick button 8" },
            new InputManagerEntry { name = "cmd2",  kind = InputManagerEntry.Kind.KeyOrButton, btnPositive = "backspace",   altBtnPositive = "joystick button 9" }
      });
      
      
RegisterInputs(new List<InputManagerEntry>
      {
            new InputManagerEntry { name = "cmd3",  kind = InputManagerEntry.Kind.KeyOrButton, btnPositive = "left alt",   altBtnPositive = "joystick button 1" }
      });

Delayed calls:

  • register cmd1, cmd2 and cmd3 at the same time
  • nothing to register, so exit early

So we inspected the already registered inputs only once instead of two time when done at same frame.

And for unregistering, the mechanism where not here prior and was never requested. I assume it is not needed in this case. But it can be implemented in a similar way. I assume this was done to add inputs in the InputManager.asset only and not remove them.

#endif
}
}
#endif
}