Fix for degenerate stroke rect

Fix for degenerate stroke rect

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/511684fb52f20862f8552f5009390ff3987b08d8

Committed: https://skia.googlesource.com/skia/+/11edad9098c1abc041e65d28b87d1080c490bc58

[joshualitt, 2015-09-18 19:56:13]

joshualitt@google.com changed reviewers:
+ joshualitt@google.com, robertphillips@google.com

[joshualitt, 2015-09-18 19:56:14]

ptal

[joshualitt, 2015-09-18 20:39:38]

feedback incorporated

[robertphillips, 2015-09-20 15:49:40]

https://codereview.chromium.org/1359453002/diff/20001/src/gpu/batches/GrRectB...
File src/gpu/batches/GrRectBatchFactory.cpp (right):

https://codereview.chromium.org/1359453002/diff/20001/src/gpu/batches/GrRectB...
src/gpu/batches/GrRectBatchFactory.cpp:47: SkRect devInside(devRect);
It seems like we could get cracking with this method.

[joshualitt, 2015-09-21 16:23:38]

ptal, feedback incorporated

[robertphillips, 2015-09-21 17:12:44]

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrAAStr...
File src/gpu/batches/GrAAStrokeRectBatch.cpp (right):

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrAAStr...
src/gpu/batches/GrAAStrokeRectBatch.cpp:399: } else {
// When the interior rect has become degenerate ... ?

SkASSERT(devInside.fLeft == devInside.fRight);
SkASSERT(devInside.fTop == devInside.fBottom);

?

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrAAStr...
src/gpu/batches/GrAAStrokeRectBatch.cpp:422: } else {
// When the interior rect has become degenerate ... ?

asserts ?

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrAAStr...
src/gpu/batches/GrAAStrokeRectBatch.cpp:443: int scale;
This hardens the inner/outer fan too

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrRectB...
File src/gpu/batches/GrRectBatchFactory.cpp (right):

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrRectB...
src/gpu/batches/GrRectBatchFactory.cpp:61: if (degenerate) {
Just use devRect.centerX() ?

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrRectB...
src/gpu/batches/GrRectBatchFactory.cpp:64: 
Just use devRect.centerY() ?

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrRectB...
src/gpu/batches/GrRectBatchFactory.cpp:92: 
Don't we want to get rid of this and pass 'true' to Create ?

[joshualitt, 2015-09-21 19:51:26]

I think I got this, ptal

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrAAStr...
File src/gpu/batches/GrAAStrokeRectBatch.cpp (right):

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrAAStr...
src/gpu/batches/GrAAStrokeRectBatch.cpp:399: } else {
On 2015/09/21 17:12:44, robertphillips wrote:
> // When the interior rect has become degenerate ... ?
> 
> SkASSERT(devInside.fLeft == devInside.fRight);
> SkASSERT(devInside.fTop == devInside.fBottom);
> 
> ?

Acknowledged.

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrAAStr...
src/gpu/batches/GrAAStrokeRectBatch.cpp:422: } else {
On 2015/09/21 17:12:44, robertphillips wrote:
> // When the interior rect has become degenerate ... ?
> 
> asserts ?

Acknowledged.

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrAAStr...
src/gpu/batches/GrAAStrokeRectBatch.cpp:443: int scale;
On 2015/09/21 17:12:44, robertphillips wrote:
> This hardens the inner/outer fan too 

Acknowledged.

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrRectB...
File src/gpu/batches/GrRectBatchFactory.cpp (right):

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrRectB...
src/gpu/batches/GrRectBatchFactory.cpp:61: if (degenerate) {
On 2015/09/21 17:12:44, robertphillips wrote:
> Just use devRect.centerX() ?

Acknowledged.

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrRectB...
src/gpu/batches/GrRectBatchFactory.cpp:64: 
On 2015/09/21 17:12:44, robertphillips wrote:
> Just use devRect.centerY() ?

Acknowledged.

https://codereview.chromium.org/1359453002/diff/40001/src/gpu/batches/GrRectB...
src/gpu/batches/GrRectBatchFactory.cpp:92: 
On 2015/09/21 17:12:44, robertphillips wrote:
> Don't we want to get rid of this and pass 'true' to Create ?

Acknowledged.

[robertphillips, 2015-09-21 19:57:39]

https://codereview.chromium.org/1359453002/diff/90001/src/gpu/batches/GrAAStr...
File src/gpu/batches/GrAAStrokeRectBatch.cpp (right):

https://codereview.chromium.org/1359453002/diff/90001/src/gpu/batches/GrAAStr...
src/gpu/batches/GrAAStrokeRectBatch.cpp:476: } else {
typo degenrate 
can -> need to ?

https://codereview.chromium.org/1359453002/diff/90001/src/gpu/batches/GrAAStr...
src/gpu/batches/GrAAStrokeRectBatch.cpp:479: // TODO use real devRect here
So devOutsideAssist.width() shouldn't appear in this ?

https://codereview.chromium.org/1359453002/diff/90001/src/gpu/batches/GrRectB...
File src/gpu/batches/GrRectBatchFactory.cpp (right):

https://codereview.chromium.org/1359453002/diff/90001/src/gpu/batches/GrRectB...
src/gpu/batches/GrRectBatchFactory.cpp:89: 
false -> devInside.isEmpty() ?

[robertphillips, 2015-09-21 19:58:30]

owise lgtm

[joshualitt, 2015-09-21 21:18:58]

Robert, please take another quick look.  This cl fixes the miscompares

https://codereview.chromium.org/1359453002/diff/90001/src/gpu/batches/GrAAStr...
File src/gpu/batches/GrAAStrokeRectBatch.cpp (right):

https://codereview.chromium.org/1359453002/diff/90001/src/gpu/batches/GrAAStr...
src/gpu/batches/GrAAStrokeRectBatch.cpp:479: // TODO use real devRect here
On 2015/09/21 19:57:39, robertphillips wrote:
> So devOutsideAssist.width() shouldn't appear in this ?

correct, it is always = devrect

[robertphillips, 2015-09-22 16:09:08]

lgtm

[joshualitt, 2015-09-22 16:22:15]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2015-09-22 16:22:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1359453002/110001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1359453002/110001

[commit-bot: I haz the power, 2015-09-22 16:27:08]

Committed patchset #7 (id:110001) as
https://skia.googlesource.com/skia/+/511684fb52f20862f8552f5009390ff3987b08d8

[joshualitt, 2015-09-22 16:47:39]

A revert of this CL (patchset #7 id:110001) has been created in
https://codereview.chromium.org/1356223003/ by joshualitt@google.com.

The reason for reverting is: breaking bots.

[joshualitt, 2015-09-22 17:27:35]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-09-22 17:27:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1359453002/#ps130001
(title: "fix glprograms")

[commit-bot: I haz the power, 2015-09-22 17:27:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1359453002/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1359453002/130001

[commit-bot: I haz the power, 2015-09-22 17:32:31]

Committed patchset #8 (id:130001) as
https://skia.googlesource.com/skia/+/11edad9098c1abc041e65d28b87d1080c490bc58