If scissor would be empty in GrClipMaskManager::SetupClipping indicate draw can be skipped.

If scissor would be empty in GrClipMaskManager::SetupClipping indicate draw can be skipped.

BUG=chromium:632185
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2196803002

Committed: https://skia.googlesource.com/skia/+/6d9a213694cd30974f2d13261eade51cd6f1ee1f

[bsalomon, 2016-07-29 17:51:24]

Description was changed from

==========
If scissor would be empty in GrClipMaskManager::SetupClipping indicate draw can
be skipped.

BUG=chromium:632185
==========

to

==========
If scissor would be empty in GrClipMaskManager::SetupClipping indicate draw can
be skipped.

BUG=chromium:632185
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2196803002
==========

[bsalomon, 2016-07-29 17:54:41]

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

[bsalomon, 2016-07-29 17:54:42]

This fixes an issue where we compute an empty scissor rect (top==bottom) and
then assert here:

https://cs.chromium.org/chromium/src/third_party/skia/src/gpu/GrDrawTarget.cp...


I want to add a test. However, between the time I verified that this fixes the
bug and now I've started having trouble reproducing this. I'll keep trying to
get a repro to generate a test case but since I'm going away for a week I'd like
to get something landed.

[robertphillips, 2016-07-29 18:01:10]

lgtm

[csmartdalton, 2016-07-29 18:07:30]

On 2016/07/29 18:01:10, robertphillips wrote:
> lgtm

lgtm too.

Curious, where does this happen? I would have thought GrReducedClip covered
every obvious case where this would happen by returning AllOut?

[bsalomon, 2016-07-29 18:10:01]

On 2016/07/29 18:07:30, csmartdalton wrote:
> On 2016/07/29 18:01:10, robertphillips wrote:
> > lgtm
> 
> lgtm too.
> 
> Curious, where does this happen? I would have thought GrReducedClip covered
> every obvious case where this would happen by returning AllOut?

That's what I was trying to figure out when I stopped being able to reproduce
it. I agree that GrReducedClip should return AllOut. If I can get this
reproducing again I'll figure out how this exits ReduceClipStack.

[bsalomon, 2016-07-29 18:10:07]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-07-29 18:10:22]

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-29 19:24:31]

Description was changed from

==========
If scissor would be empty in GrClipMaskManager::SetupClipping indicate draw can
be skipped.

BUG=chromium:632185
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2196803002
==========

to

==========
If scissor would be empty in GrClipMaskManager::SetupClipping indicate draw can
be skipped.

BUG=chromium:632185
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2196803002

Committed:
https://skia.googlesource.com/skia/+/6d9a213694cd30974f2d13261eade51cd6f1ee1f
==========

[commit-bot: I haz the power, 2016-07-29 19:24:32]

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

[csmartdalton, 2016-07-29 19:24:33]

On 2016/07/29 18:10:01, bsalomon wrote:
> On 2016/07/29 18:07:30, csmartdalton wrote:
> > On 2016/07/29 18:01:10, robertphillips wrote:
> > > lgtm
> > 
> > lgtm too.
> > 
> > Curious, where does this happen? I would have thought GrReducedClip covered
> > every obvious case where this would happen by returning AllOut?
> 
> That's what I was trying to figure out when I stopped being able to reproduce
> it. I agree that GrReducedClip should return AllOut. If I can get this
> reproducing again I'll figure out how this exits ReduceClipStack.

Oh I see it now. Looking at the code..

GrReducedClip seems to catch all the cases of totally empty clip bounds.

However, I see that very narrow (<.5) non-aa clip rects can round to empty.
Also, any extremely narrow clip bounds (<GrClip::kBoundsTolerance) can round
"out" to empty if they are close enough to a pixel boundary.