Simplify matrix generation for convex path renderer

Simplify matrix generation for convex path renderer

Addresses precision issues which were leading to asserts in Debug

Committed: http://code.google.com/p/skia/source/detail?r=12494

[mtklein, 2013-12-03 20:52:27]

lgtm

https://codereview.chromium.org/102683002/diff/60001/src/gpu/GrPathUtils.cpp
File src/gpu/GrPathUtils.cpp (right):

https://codereview.chromium.org/102683002/diff/60001/src/gpu/GrPathUtils.cpp#...
src/gpu/GrPathUtils.cpp:255: // order swapped around to minimize stalls
Interesting!  Does this actually generate significantly different code?  I'd
imagine a smart compiler will go in there and do this anyway, but you never
really know how smart they will be.  Is there a bench for this and is it
affected by the order here?

https://codereview.chromium.org/102683002/diff/60001/src/gpu/GrPathUtils.cpp#...
src/gpu/GrPathUtils.cpp:259: adjugate[8] = x0*y1-y0*x1;
Just a nit.  All the others have x*y except this y0*x1.

https://codereview.chromium.org/102683002/diff/60001/src/gpu/GrPathUtils.cpp#...
src/gpu/GrPathUtils.cpp:271: m[SkMatrix::kMSkewY] = (float)(adjugate[6]*scale);
This is again a nit, but it'd help me read with an extra space in front of the =
for the skews, so it all lines up pretty-like.

[bsalomon, 2013-12-03 21:18:14]

lgtm

[jvanverth1, 2013-12-04 19:51:20]

https://codereview.chromium.org/102683002/diff/60001/src/gpu/GrPathUtils.cpp
File src/gpu/GrPathUtils.cpp (right):

https://codereview.chromium.org/102683002/diff/60001/src/gpu/GrPathUtils.cpp#...
src/gpu/GrPathUtils.cpp:255: // order swapped around to minimize stalls
On 2013/12/03 20:52:28, mtklein wrote:
> Interesting!  Does this actually generate significantly different code?  I'd
> imagine a smart compiler will go in there and do this anyway, but you never
> really know how smart they will be.  Is there a bench for this and is it
> affected by the order here?

I looked into this, and it appears that on Linux, if you store the temporary
adjugate matrix in an array, that the order may matter (because of noise it's
inconclusive). However, I removed the matrix and replaced it with multiple
variables, and in that case the order doesn't appear to matter (again, somewhat
inconclusive).

[commit-bot: I haz the power, 2013-12-04 19:51:54]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/jvanverth@google.com/102683002/80001

[commit-bot: I haz the power, 2013-12-04 19:51:55]

Failed to apply patch for expectations/gm/ignored-tests.txt:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file expectations/gm/ignored-tests.txt
  Hunk #1 FAILED at 66.
  1 out of 1 hunk FAILED -- saving rejects to file
expectations/gm/ignored-tests.txt.rej

Patch:       expectations/gm/ignored-tests.txt
Index: expectations/gm/ignored-tests.txt
diff --git a/expectations/gm/ignored-tests.txt
b/expectations/gm/ignored-tests.txt
index
3f49b304515f0470b90a92021f6d4a8bdebf64d9..da0244ac8b97aa03b90233f4f6c655e2a95bb60d
100644
--- a/expectations/gm/ignored-tests.txt
+++ b/expectations/gm/ignored-tests.txt
@@ -66,3 +66,25 @@ filterbitmap_text_10.00pt
 
 # Added by vandebo in https://codereview.chromium.org/92453002/
 xfermodes
+
+# Added by jvanverth in https://codereview.chromium.org/102683002/
+mixed_xfermodes_gpu
+rrect_gpu
+rrect_aa_gpu
+shadertext3_gpu
+shadertext2_gpu
+patheffect_gpu
+path-reverse_gpu
+bezier_quad_effects_gpu
+hairlines_gpu
+getpostextpath_gpu
+dashcubics_gpu
+convexpaths_gpu
+roundrects_gpu
+strokerect_gpu
+rects_gpu
+simpleaaclip_path_gpu
+complexclip_aa_layer_gpu
+complexclip_bw_layer_gpu
+complexclip_aa_gpu
+complexclip_bw_gpu

[mtklein, 2013-12-04 20:40:46]

On 2013/12/04 19:51:20, JimVV wrote:
> https://codereview.chromium.org/102683002/diff/60001/src/gpu/GrPathUtils.cpp
> File src/gpu/GrPathUtils.cpp (right):
> 
>
https://codereview.chromium.org/102683002/diff/60001/src/gpu/GrPathUtils.cpp#...
> src/gpu/GrPathUtils.cpp:255: // order swapped around to minimize stalls
> On 2013/12/03 20:52:28, mtklein wrote:
> > Interesting!  Does this actually generate significantly different code?  I'd
> > imagine a smart compiler will go in there and do this anyway, but you never
> > really know how smart they will be.  Is there a bench for this and is it
> > affected by the order here?
> 
> I looked into this, and it appears that on Linux, if you store the temporary
> adjugate matrix in an array, that the order may matter (because of noise it's
> inconclusive). However, I removed the matrix and replaced it with multiple
> variables, and in that case the order doesn't appear to matter (again,
somewhat
> inconclusive).

Interesting.  Thanks for the followup!  I did wonder if or how a double[9] would
be put into registers.

[commit-bot: I haz the power, 2013-12-04 21:07:08]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/jvanverth@google.com/102683002/100001

[commit-bot: I haz the power, 2013-12-04 21:33:09]

Change committed as 12494