Fixing LabelEntryMatchCache #60
Conversation
…any entries are reported as matching the first entry.
| /// <inheritdoc/> | ||
| public bool Equals(IdLabelEntry other) | ||
| { | ||
| return label == other.label && id == other.id; |
There was a problem hiding this comment.
Doesn't need to be addressed in this PR, but can we end up with a label that has the same string as another label but a different ID, or same ID but different strings?
There was a problem hiding this comment.
Two entries with the same id but different labels is certainly feasible - for example "Car" and "Truck" mapping to label id '1' in your dataset. "Car" mapping to both '1' and '2' doesn't make as much sense, since the first entry would always match.
From this struct's perspective though that's an implementation detail. For equality of simple data types it's almost always right to just compare all of the data.
| var label = "label"; | ||
| //only way to guarantee registration order is to run frames. | ||
| //We want to ensure labeledPlane is registered before labeledPlane2 so that the cache does not early out | ||
| var labeledPlane = TestHelper.CreateLabeledPlane(label: "foo"); | ||
| AddTestObjectForCleanup(labeledPlane); | ||
| yield return null; | ||
| var labeledPlane2 = TestHelper.CreateLabeledPlane(label: label); |
There was a problem hiding this comment.
kind of an anti-pattern here: one argument gets a variable name but the other doesn't
mrpropellers
left a comment
mrpropellers
left a comment
There was a problem hiding this comment.
Nothing alarming. Left a small nit if you're bored :)
…any entries are reported as matching the first entry. (Unity-Technologies#60)
2 participants
Peer Review Information:
Fixing bug where instance ids that do not match any IdLabelConfig entries are reported as matching the first entry, resulting in bounding boxes being drawn around objects whose labels have no matches in the IdLabelConfig.
Editor / Package versioning:
Editor Version Target (i.e. 19.3, 20.1): 2019.3
Dev Testing:
Tests Added:
Adding tests for LabelEntryMatchCache
Package Tests (Pass/Fail):
[X] - Make sure automation passes
Core Scenario Tested:
At Risk Areas:
Notes + Expectations: