-
Notifications
You must be signed in to change notification settings - Fork 572
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
Fix pairing of functions by type. #5227
base: main
Are you sure you want to change the base?
Conversation
@ShabbyX Can you please review this? |
Thanks for the fix. It looks like this didn't affect the test results so it's not covered by any tests. Could you please add tests to make sure this isn't regressed in the future? |
Converted to draft until it's got test coverage. |
2fd1d5e
to
6226f04
Compare
Hi @jimblandy did you happen to get a chance to get this PR to the finish line? |
6226f04
to
e1a6e1e
Compare
e1a6e1e
to
659cc54
Compare
@ShabbyX I finally got around to writing a test for this (because I ran into it again and debugged it a second time 😢). PTAL. |
Change `Differ::MatchFunctions` to use `GroupIdsAndMatchByMappedId`, instead of calling `GroupIdsAndMatch` with `Differ::GroupIdsHelperGetTypeId`, which is never correct: doing so compares src and dst type ids directly, rather than consulting the established mapping. Fixes KhronosGroup#5226.
659cc54
to
1a734d1
Compare
Change
Differ::MatchFunctions
to useGroupIdsAndMatchByMappedId
, instead of callingGroupIdsAndMatch
withDiffer::GroupIdsHelperGetTypeId
, which is never correct: doing so compares src and dst type ids directly, rather than consulting the established mapping.Depends on #5225.
Fixes #5226.