Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[release/3.0] fix Matrix4x4 + and - operator bugs (#39838) - #39889

Merged
wtgodbe merged 1 commit into
dotnet:release/3.0from
tannergooding:backport-39838
Aug 7, 2019
Merged

[release/3.0] fix Matrix4x4 + and - operator bugs (#39838)#39889
wtgodbe merged 1 commit into
dotnet:release/3.0from
tannergooding:backport-39838

Conversation

@tannergooding

@tannergooding tannergooding commented Jul 30, 2019

Copy link
Copy Markdown
Member

Port #39838 to release/3.0

Description

#31779 brought a perf improvement to the Matrix4x4 addition and subtraction operators, but also introduced a functional regression due to using the left-hand side variable twice.

Customer Impact

Customers using the addition or subtraction operators on Matrix4x4 will compute the wrong result

Regression?

Yes, as per the description.

Risk

Low. The tests are being updated to use different values for the left and right hand side inputs to validate this stays correct and we don't accidentally introduce bugs like this in the future.

* fix Matrix4x4 + and - operator bugs

* fix Add() and Subtract() tests
@tannergooding

Copy link
Copy Markdown
Member Author

CC. @stephentoub, @danmosemsft

I'm also going to log an issue tracking us reviewing the existing numeric tests to validate we don't have other places where the lhs/rhs inputs are only always the same.

@tannergooding tannergooding added this to the 3.0 milestone Jul 30, 2019

@stephentoub stephentoub left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, Tanner.

@tannergooding

Copy link
Copy Markdown
Member Author

Logged https://github.com/dotnet/corefx/issues/39890 to track auditing the remaining tests.

@leecow

leecow commented Jul 30, 2019

Copy link
Copy Markdown
Member

tactics approved - wait to merge for @wtgodbe go ahead.

@tannergooding

Copy link
Copy Markdown
Member Author

@wtgodbe, do you have an ETA on when this can be merged?

@wtgodbe

wtgodbe commented Jul 31, 2019

Copy link
Copy Markdown
Member

Currently I'm targeting Monday

@karelz karelz changed the title fix Matrix4x4 + and - operator bugs (#39838) [release/3.0] fix Matrix4x4 + and - operator bugs (#39838) Aug 3, 2019
@karelz

karelz commented Aug 3, 2019

Copy link
Copy Markdown
Member

@tannergooding please link the PR you are porting in future (see my edits to top post) - it required some effort to find this one and confirm we didn't lose track of the port request. Thanks!

@danmoseley

Copy link
Copy Markdown

still waiting on branch

@wtgodbe

wtgodbe commented Aug 7, 2019

Copy link
Copy Markdown
Member

Branch is open, merging

@wtgodbe
wtgodbe merged commit 1a11573 into dotnet:release/3.0 Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants