Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(8)

Issue 23296006: Improve performance of matrix inversion. (Closed)

Created:
6 years, 3 months ago by shawnsingh
Modified:
6 years, 3 months ago
Reviewers:
jvanverth1, reed1
CC:
skia-review_googlegroups.com
Visibility:
Public.

Description

Improve performance of matrix inversion. The inversion of scale+translate matrices was using 6 division operations when it could be using 3. Also, general affine matrices do not need to compute perspective components of the matrix. This patch updates the matrix inversion with those optimizations, and includes benchmark code to exercise those paths.

Patch Set 1 #

Total comments: 3

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Patch Set 4 : patch for landing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+76 lines, -6 lines) Patch
M src/utils/SkMatrix44.cpp View 1 2 3 2 chunks +76 lines, -6 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
shawnsingh
Please take a look. This improves the 3x4 affine inversion as well as scale+translate matrix ...
6 years, 3 months ago (2013-08-19 05:45:00 UTC) #1
jvanverth1
Very nice! Some comments below. https://codereview.chromium.org/23296006/diff/1/bench/Matrix44Bench.cpp File bench/Matrix44Bench.cpp (right): https://codereview.chromium.org/23296006/diff/1/bench/Matrix44Bench.cpp#newcode109 bench/Matrix44Bench.cpp:109: We may want to ...
6 years, 3 months ago (2013-08-19 20:41:44 UTC) #2
shawnsingh
Thanks for the feedback - it worked out great. 1. Benchmarks and unit tests added ...
6 years, 3 months ago (2013-08-20 04:13:59 UTC) #3
jvanverth1
LGTM, just one comment below. If you approve my updated test for the determinant check, ...
6 years, 3 months ago (2013-08-20 14:16:54 UTC) #4
reed1
https://codereview.chromium.org/23296006/diff/12001/src/utils/SkMatrix44.cpp File src/utils/SkMatrix44.cpp (right): https://codereview.chromium.org/23296006/diff/12001/src/utils/SkMatrix44.cpp#newcode520 src/utils/SkMatrix44.cpp:520: if (!(this->getType() & kPerspective_Mask)) { At some point we ...
6 years, 3 months ago (2013-08-20 15:14:13 UTC) #5
shawnsingh
I have been waiting for the new perf benchmarks to show up on the skia ...
6 years, 3 months ago (2013-08-21 18:11:20 UTC) #6
jvanverth1
On 2013/08/21 18:11:20, shawnsingh wrote: > I have been waiting for the new perf benchmarks ...
6 years, 3 months ago (2013-08-21 19:02:49 UTC) #7
shawnsingh
6 years, 3 months ago (2013-08-22 20:42:05 UTC) #8
Commited this as https://code.google.com/p/skia/source/detail?r=10883

While committing the gcl script seemed to crash by fluke so I'm closing this
manually.

Powered by Google App Engine
This is Rietveld 408576698