Skip to content
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

Optimize barycentric interpolation #1019

Merged
merged 1 commit into from
Feb 25, 2020
Merged

Conversation

cryovat
Copy link
Contributor

@cryovat cryovat commented Feb 25, 2020

Purpose of this PR

  • Description of feature/change.

I noticed during benchmarking math operations that one of the Barycentric overloads was four times slower than the other.

Turns out the slow version contains a naive implementation using operator overloads (leading to a number of unnecessary copies/allocations), while the fast version has an optimized implementation that passes all parameters by reference.

This change replaces the naive implementations with a call to the optimized method instead.

  • Which part of OpenTK does this affect (Math, OpenGL, Platform, Input, etc).

Math

  • Links to screenshots, design docs, user docs, etc.

Se comments for impact of current approach.

Testing status

  • Explanation of what’s tested, how tested and existing or new automation tests.

Ran unit tests, and verified the Barycentric ones all passed. Note that the double variants of the vectors do not have existing test coverage.

Comments

Benchmark of current implementations as of a2e1be1:

Method Runtime Mean Error StdDev Median Ratio RatioSD
'Vector3.BaryCentric(Vector3 a, Vector3 b, Vector3 c, float u, float v)' .NET Core 3.1 29.595 ns 0.0812 ns 0.0720 ns 29.596 ns 0.99 0.00
'Vector3.BaryCentric(ref Vector3 a, ref Vector3 b, ref Vector3 c, float u, float v, out Vector3 result)' .NET Core 3.1 7.048 ns 0.1418 ns 0.1326 ns 7.082 ns 0.96 0.02

@cryovat
Copy link
Contributor Author

cryovat commented Feb 25, 2020

Benchmark with change:

Method Runtime Mean Error StdDev Ratio
'Vector3.BaryCentric(Vector3 a, Vector3 b, Vector3 c, float u, float v)' .NET Core 3.1 15.229 ns 0.0645 ns 0.0572 ns 0.98
'Vector3.BaryCentric(ref Vector3 a, ref Vector3 b, ref Vector3 c, float u, float v, out Vector3 result)' .NET Core 3.1 7.106 ns 0.0357 ns 0.0334 ns 1.04

I was honestly hoping for a bigger improvement, but 2x cost still beats 4x I guess...

@varon varon merged commit 83de7ce into opentk:master Feb 25, 2020
@varon
Copy link
Member

varon commented Feb 25, 2020

Thanks @cryovat - This is a great PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants