Skip to content

Commit 385b68d

Browse files
JonathanHUnityJonathanHUnity
authored
Fixing LabelEntryMatchCache bug where instance ids that do not match any entries are reported as matching the first entry. (Unity-Technologies#60)
1 parent 1dc0de0 commit 385b68d

File tree

6 files changed

+142
-2
lines changed

6 files changed

+142
-2
lines changed

‎com.unity.perception/Runtime/GroundTruth/GroundTruthLabelSetupSystem.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ protected override void OnUpdate()
5353
{
5454
instanceId = instanceId
5555
});
56+
labeling.SetInstanceId(instanceId);
5657
});
5758
}
5859

‎com.unity.perception/Runtime/GroundTruth/Labeling/IdLabelEntry.cs‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace UnityEngine.Perception.GroundTruth {
55
/// An entry for <see cref="IdLabelConfig"/> mapping a label to an integer id.
66
/// </summary>
77
[Serializable]
8-
public struct IdLabelEntry : ILabelEntry
8+
public struct IdLabelEntry : ILabelEntry, IEquatable<IdLabelEntry>
99
{
1010
string ILabelEntry.label => this.label;
1111
/// <summary>
@@ -16,5 +16,26 @@ public struct IdLabelEntry : ILabelEntry
1616
/// The id to associate with the label.
1717
/// </summary>
1818
public int id;
19+
20+
/// <inheritdoc/>
21+
public bool Equals(IdLabelEntry other)
22+
{
23+
return label == other.label && id == other.id;
24+
}
25+
26+
/// <inheritdoc/>
27+
public override bool Equals(object obj)
28+
{
29+
return obj is IdLabelEntry other && Equals(other);
30+
}
31+
32+
/// <inheritdoc/>
33+
public override int GetHashCode()
34+
{
35+
unchecked
36+
{
37+
return ((label != null ? label.GetHashCode() : 0) * 397) ^ id;
38+
}
39+
}
1940
}
2041
}

‎com.unity.perception/Runtime/GroundTruth/Labeling/LabelEntryMatchCache.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ class LabelEntryMatchCache : IGroundTruthGenerator, IDisposable
1414
const int k_StartingObjectCount = 1 << 8;
1515
NativeList<ushort> m_InstanceIdToLabelEntryIndexLookup;
1616
IdLabelConfig m_IdLabelConfig;
17+
ushort m_DefaultValue;
1718

1819
public LabelEntryMatchCache(IdLabelConfig idLabelConfig)
1920
{
@@ -26,7 +27,7 @@ public bool TryGetLabelEntryFromInstanceId(uint instanceId, out IdLabelEntry lab
2627
{
2728
labelEntry = default;
2829
index = -1;
29-
if (m_InstanceIdToLabelEntryIndexLookup.Length <= instanceId)
30+
if (m_InstanceIdToLabelEntryIndexLookup.Length <= instanceId || m_InstanceIdToLabelEntryIndexLookup[(int)instanceId] == m_DefaultValue)
3031
return false;
3132

3233
index = m_InstanceIdToLabelEntryIndexLookup[(int)instanceId];
@@ -38,9 +39,15 @@ void IGroundTruthGenerator.SetupMaterialProperties(MaterialPropertyBlock mpb, Re
3839
{
3940
if (m_IdLabelConfig.TryGetMatchingConfigurationEntry(labeling, out _, out var index))
4041
{
42+
m_DefaultValue = ushort.MaxValue;
43+
Debug.Assert(index < m_DefaultValue, "Too many entries in the label config");
4144
if (m_InstanceIdToLabelEntryIndexLookup.Length <= instanceId)
4245
{
46+
var oldLength = m_InstanceIdToLabelEntryIndexLookup.Length;
4347
m_InstanceIdToLabelEntryIndexLookup.Resize((int)instanceId + 1, NativeArrayOptions.ClearMemory);
48+
49+
for (int i = oldLength; i < instanceId; i++)
50+
m_InstanceIdToLabelEntryIndexLookup[i] = m_DefaultValue;
4451
}
4552
m_InstanceIdToLabelEntryIndexLookup[(int)instanceId] = (ushort)index;
4653
}

‎com.unity.perception/Runtime/GroundTruth/Labeling/Labeling.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,14 @@ public class Labeling : MonoBehaviour
1717
[FormerlySerializedAs("classes")]
1818
public List<string> labels = new List<string>();
1919

20+
public uint instanceId { get; private set; }
21+
2022
Entity m_Entity;
23+
24+
internal void SetInstanceId(uint instanceId)
25+
{
26+
this.instanceId = instanceId;
27+
}
2128
void Awake()
2229
{
2330
m_Entity = World.DefaultGameObjectInjectionWorld.EntityManager.CreateEntity();

‎com.unity.perception/Tests/Runtime/GroundTruthTests/LabelEntryMatchCacheTests.cs‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
using System.Collections;
2+
using NUnit.Framework;
3+
using UnityEngine.Perception.GroundTruth;
4+
using UnityEngine.TestTools;
5+
6+
namespace GroundTruthTests
7+
{
8+
[TestFixture]
9+
public class LabelEntryMatchCacheTests : GroundTruthTestBase
10+
{
11+
[Test]
12+
public void TryGet_ReturnsFalse_ForInvalidInstanceId()
13+
{
14+
var config = new IdLabelConfig();
15+
using (var cache = new LabelEntryMatchCache(config))
16+
{
17+
Assert.IsFalse(cache.TryGetLabelEntryFromInstanceId(100, out var labelEntry, out var index));
18+
Assert.AreEqual(-1, index);
19+
Assert.AreEqual(default(IdLabelEntry), labelEntry);
20+
}
21+
}
22+
[UnityTest]
23+
public IEnumerator TryGet_ReturnsTrue_ForMatchingLabel()
24+
{
25+
var label = "label";
26+
var labeledPlane = TestHelper.CreateLabeledPlane(label: label);
27+
AddTestObjectForCleanup(labeledPlane);
28+
var config = new IdLabelConfig();
29+
config.Init(new[]
30+
{
31+
new IdLabelEntry()
32+
{
33+
id = 1,
34+
label = label
35+
},
36+
});
37+
using (var cache = new LabelEntryMatchCache(config))
38+
{
39+
//allow label to be registered
40+
yield return null;
41+
Assert.IsTrue(cache.TryGetLabelEntryFromInstanceId(labeledPlane.GetComponent<Labeling>().instanceId, out var labelEntry, out var index));
42+
Assert.AreEqual(0, index);
43+
Assert.AreEqual(config.labelEntries[0], labelEntry);
44+
}
45+
}
46+
[UnityTest]
47+
public IEnumerator TryGet_ReturnsFalse_ForNonMatchingLabel()
48+
{
49+
var label = "label";
50+
var labeledPlane = TestHelper.CreateLabeledPlane(label: label);
51+
AddTestObjectForCleanup(labeledPlane);
52+
var config = new IdLabelConfig();
53+
using (var cache = new LabelEntryMatchCache(config))
54+
{
55+
//allow label to be registered
56+
yield return null;
57+
Assert.IsFalse(cache.TryGetLabelEntryFromInstanceId(labeledPlane.GetComponent<Labeling>().instanceId, out var labelEntry, out var index));
58+
Assert.AreEqual(-1, index);
59+
Assert.AreEqual(default(IdLabelEntry), labelEntry);
60+
}
61+
}
62+
[UnityTest]
63+
public IEnumerator TryGet_ReturnsFalse_ForNonMatchingLabel_WithOtherMatches()
64+
{
65+
var label = "label";
66+
//only way to guarantee registration order is to run frames.
67+
//We want to ensure labeledPlane is registered before labeledPlane2 so that the cache does not early out
68+
var labeledPlane = TestHelper.CreateLabeledPlane(label: "foo");
69+
AddTestObjectForCleanup(labeledPlane);
70+
yield return null;
71+
var labeledPlane2 = TestHelper.CreateLabeledPlane(label: label);
72+
AddTestObjectForCleanup(labeledPlane2);
73+
var config = new IdLabelConfig();
74+
config.Init(new[]
75+
{
76+
new IdLabelEntry()
77+
{
78+
id = 1,
79+
label = label
80+
},
81+
});
82+
using (var cache = new LabelEntryMatchCache(config))
83+
{
84+
//allow label to be registered
85+
yield return null;
86+
Assert.IsFalse(cache.TryGetLabelEntryFromInstanceId(labeledPlane.GetComponent<Labeling>().instanceId, out var labelEntry, out var index));
87+
Assert.AreEqual(-1, index);
88+
Assert.AreEqual(default(IdLabelEntry), labelEntry);
89+
}
90+
}
91+
92+
}
93+
}

‎com.unity.perception/Tests/Runtime/GroundTruthTests/LabelEntryMatchCacheTests.cs.meta‎

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)