Add gm that tests shaded stroked rectangles.

Add gm that tests shaded stroked rectangles.

Fix GPU handling of previously untested cases.

Move rect->path fallback from SkGpuDevice to GrDrawContext.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2125663003

Committed: https://skia.googlesource.com/skia/+/a7d85ba138ee09739defbd277b04c479fdef82a6

[bsalomon, 2016-07-06 02:33:37]

Description was changed from

==========
Add gm that tests shaded stroked rectangles.

Move rect->path fallback from SkGpuDevice to GrDrawContext.

BUG=skia:
==========

to

==========
Add gm that tests shaded stroked rectangles.

Move rect->path fallback from SkGpuDevice to GrDrawContext.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2125663003
==========

[bsalomon, 2016-07-06 02:45:59]

Description was changed from

==========
Add gm that tests shaded stroked rectangles.

Move rect->path fallback from SkGpuDevice to GrDrawContext.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2125663003
==========

to

==========
Add gm that tests shaded stroked rectangles.

Fix GPU handling of previously untested cases.

Move rect->path fallback from SkGpuDevice to GrDrawContext.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2125663003
==========

[bsalomon, 2016-07-06 02:45:59]

bsalomon@google.com changed reviewers:
+ robertphillips@google.com

[bsalomon, 2016-07-06 02:47:39]

The CQ bit was checked by bsalomon@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-06 02:47:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-06 02:53:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-06 02:53:52]

Dry run: Try jobs failed on following builders:
  Build-Win-MSVC-x86-Debug-Trybot on master.client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86-D...)

[robertphillips, 2016-07-06 15:01:48]

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.cpp
File gm/stroke_rect_shader.cpp (right):

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:1: /*
What's up with the naming of this file?

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:10: 
Are we testing the degenerate rect cases somewhere else ?

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:12: 
// Draw stroked rects (both AA and nonAA) with all the types of joins:
//    bevel, miter, miter-limited-to-bevel, round
// and as a hairline.

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:17: SkPaint paint;
What are your thoughts on the style proposal that we never hide sk_sp in an auto
(since it obscures ownership transactions?

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:23: // for the first rect drawn.
kRect.centerX(), kRect.centerY() ?

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:45: 
Wrong comment ?

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:50: 
// Hairlines, make bevel and round joins appear the same as mitered ones

Do we want to test that ?

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:52: canvas->drawRect(kRect, paint);
Rm this translate ?

https://codereview.chromium.org/2125663003/diff/20001/include/core/SkPaint.h
File include/core/SkPaint.h (right):

https://codereview.chromium.org/2125663003/diff/20001/include/core/SkPaint.h#...
include/core/SkPaint.h:413: };
Have we given up on the whole 'kLast_Cap' thing ?

https://codereview.chromium.org/2125663003/diff/20001/src/gpu/GrDrawContext.cpp
File src/gpu/GrDrawContext.cpp (right):

https://codereview.chromium.org/2125663003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:324: const SkStrokeRec& stroke = style->strokeRec();
Do we still need this code? If so, I think it needs a return statement.

https://codereview.chromium.org/2125663003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:365: stroke.getStyle() ==
SkStrokeRec::kHairline_Style) {
// Handle special case where the stroked rect degenerates to a line or point 

?

https://codereview.chromium.org/2125663003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:377: case SkPaint::kRound_Join:
Isn't that a bug in raster though ?

[bsalomon, 2016-07-06 15:57:54]

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.cpp
File gm/stroke_rect_shader.cpp (right):

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:1: /*
On 2016/07/06 15:01:47, robertphillips wrote:
> What's up with the naming of this file?

I don't understand. What is up with it? The underscores?

$ ls  *_*
all_bitmap_configs.cpp      degenerate_rrect.cpp            gm_json.pyc         
      image_pict.cpp                    repeated_bitmap.cpp
annotated_text.cpp          draw_bitmap_rect_skbug4374.cpp  gradient_matrix.cpp 
      image_shader.cpp                  skbug_257.cpp
clip_strokerect.cpp         glyph_pos_align.cpp            
gradients_2pt_conical.cpp  internal_links.cpp                skbug_4868.cpp
complexclip_blur_tiled.cpp  glyph_pos.cpp                  
gradients_no_texture.cpp   path_stroke_with_zero_length.cpp 
stroke_rect_shader.cpp
convex_all_line_paths.cpp   gm_expectations.h              
hardstop_gradients.cpp     pdf_never_embed.cpp              
tilemodes_scaled.cpp

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:10: 
On 2016/07/06 15:01:47, robertphillips wrote:
> Are we testing the degenerate rect cases somewhere else ?

The strokedrect GM tests more varieties of rects + stroking params including
degenerate rectangles.

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:12: 
On 2016/07/06 15:01:47, robertphillips wrote:
> // Draw stroked rects (both AA and nonAA) with all the types of joins:
> //    bevel, miter, miter-limited-to-bevel, round
> // and as a hairline.

Done.

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:17: SkPaint paint;
On 2016/07/06 15:01:47, robertphillips wrote:
> What are your thoughts on the style proposal that we never hide sk_sp in an
auto
> (since it obscures ownership transactions?

I don't agree with it, but I'm fine abiding by it.

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:23: // for the first rect drawn.
On 2016/07/06 15:01:47, robertphillips wrote:
> kRect.centerX(), kRect.centerY() ?

Done.

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:45: 
On 2016/07/06 15:01:47, robertphillips wrote:
> Wrong comment ?

Done.

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:50: 
On 2016/07/06 15:01:47, robertphillips wrote:
> // Hairlines, make bevel and round joins appear the same as mitered ones
> 
> Do we want to test that ?

If we are going to test that I think it'd be better to do so in a GM focused on
hairline rects  (if we don't have one already). Or it could be done as a unit
test that just asserts that join doesn't affect the generated pixels.

https://codereview.chromium.org/2125663003/diff/20001/gm/stroke_rect_shader.c...
gm/stroke_rect_shader.cpp:52: canvas->drawRect(kRect, paint);
On 2016/07/06 15:01:47, robertphillips wrote:
> Rm this translate ?

Done.

https://codereview.chromium.org/2125663003/diff/20001/include/core/SkPaint.h
File include/core/SkPaint.h (right):

https://codereview.chromium.org/2125663003/diff/20001/include/core/SkPaint.h#...
include/core/SkPaint.h:413: };
On 2016/07/06 15:01:47, robertphillips wrote:
> Have we given up on the whole 'kLast_Cap' thing ?
Done

https://codereview.chromium.org/2125663003/diff/20001/src/gpu/GrDrawContext.cpp
File src/gpu/GrDrawContext.cpp (right):

https://codereview.chromium.org/2125663003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:324: const SkStrokeRec& stroke = style->strokeRec();
On 2016/07/06 15:01:47, robertphillips wrote:
> Do we still need this code? If so, I think it needs a return statement.

Oh no, left it by accident. Removed.

https://codereview.chromium.org/2125663003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:365: stroke.getStyle() ==
SkStrokeRec::kHairline_Style) {
On 2016/07/06 15:01:47, robertphillips wrote:
> // Handle special case where the stroked rect degenerates to a line or point 
> 
> ?

That seems to me a bit like rewriting "if (!width || !height)" in prose. I think
a reader of this code knows that zero width or height means the rect is
degenerate.

https://codereview.chromium.org/2125663003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:377: case SkPaint::kRound_Join:
On 2016/07/06 15:01:47, robertphillips wrote:
> Isn't that a bug in raster though ?

I'm not sure. Will check with reed@

[robertphillips, 2016-07-06 17:08:30]

lgtm

https://codereview.chromium.org/2125663003/diff/60001/src/gpu/batches/GrNonAA...
File src/gpu/batches/GrNonAAStrokeRectBatch.cpp (right):

https://codereview.chromium.org/2125663003/diff/60001/src/gpu/batches/GrNonAA...
src/gpu/batches/GrNonAAStrokeRectBatch.cpp:246: SkScalar strokeWidth =
random->nextBool() ? 0.0f : 1.0f;
Do we care about testing kRound_Join here ?

[bsalomon, 2016-07-06 17:13:40]

https://codereview.chromium.org/2125663003/diff/60001/src/gpu/batches/GrNonAA...
File src/gpu/batches/GrNonAAStrokeRectBatch.cpp (right):

https://codereview.chromium.org/2125663003/diff/60001/src/gpu/batches/GrNonAA...
src/gpu/batches/GrNonAAStrokeRectBatch.cpp:246: SkScalar strokeWidth =
random->nextBool() ? 0.0f : 1.0f;
On 2016/07/06 17:08:30, robertphillips wrote:
> Do we care about testing kRound_Join here ?

It would fail unless we're in hairline mode, in which case it is ignored.

[bsalomon, 2016-07-06 17:13:45]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-07-06 17:13:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bsalomon, 2016-07-06 17:18:30]

The CQ bit was unchecked by bsalomon@google.com

[bsalomon, 2016-07-06 17:23:15]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-07-06 17:23:16]

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/2125663003/#ps80001
(title: "Make test create factory only use miter join")

[commit-bot: I haz the power, 2016-07-06 17:23:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-06 17:34:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-06 17:34:58]

Try jobs failed on following builders:
  Build-Win-MSVC-x86_64-Debug-Trybot on master.client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[bsalomon, 2016-07-06 17:40:50]

https://codereview.chromium.org/2125663003/diff/60001/src/gpu/batches/GrNonAA...
File src/gpu/batches/GrNonAAStrokeRectBatch.cpp (right):

https://codereview.chromium.org/2125663003/diff/60001/src/gpu/batches/GrNonAA...
src/gpu/batches/GrNonAAStrokeRectBatch.cpp:246: SkScalar strokeWidth =
random->nextBool() ? 0.0f : 1.0f;
On 2016/07/06 17:13:40, bsalomon wrote:
> On 2016/07/06 17:08:30, robertphillips wrote:
> > Do we care about testing kRound_Join here ?
> 
> It would fail unless we're in hairline mode, in which case it is ignored.

I modified it to always use miter since bevel is also not allowed.

[bsalomon, 2016-07-06 17:43:29]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-07-06 17:43:29]

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/2125663003/#ps30011
(title: "fix windows double->scalar warning")

[commit-bot: I haz the power, 2016-07-06 17:43:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-06 18:55:02]

Description was changed from

==========
Add gm that tests shaded stroked rectangles.

Fix GPU handling of previously untested cases.

Move rect->path fallback from SkGpuDevice to GrDrawContext.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2125663003
==========

to

==========
Add gm that tests shaded stroked rectangles.

Fix GPU handling of previously untested cases.

Move rect->path fallback from SkGpuDevice to GrDrawContext.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2125663003

Committed:
https://skia.googlesource.com/skia/+/a7d85ba138ee09739defbd277b04c479fdef82a6
==========

[commit-bot: I haz the power, 2016-07-06 18:55:03]

Committed patchset #6 (id:30011) as
https://skia.googlesource.com/skia/+/a7d85ba138ee09739defbd277b04c479fdef82a6

[commit-bot: I haz the power, 2016-07-06 18:55:06]

CQ bit was unchecked.