Add flags to GrStencilSettings

[bsalomon, 2011-12-15 21:25:48]

On 2011/12/15 21:25:27, bsalomon wrote:

10% on linux, 5% on Mac, too noisy to tell on windows

[TomH, 2011-12-19 15:36:45]

5-10% on which benchmarks? What's making stencil state checking sooooo
expensive?

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrGpuGL.cpp
File src/gpu/GrGpuGL.cpp (right):

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrGpuGL.cpp#newcode1807
src/gpu/GrGpuGL.cpp:1807: fHWStencilClip != stencilClip ||
Just reordering to put the most-likely or cheapest success first?

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrStencil.h
File src/gpu/GrStencil.h (right):

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrStencil.h#newcode213
src/gpu/GrStencil.h:213: // just write an illegal value to the first member
Comment slightly obsolete?

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrStencil.h#newcode221
src/gpu/GrStencil.h:221: sizeof(fFlags);
Why not rewrite the comparison instead so that we're still guaranteeing tight
packing including the flags? I presume the reason for wanting tight packing is
that this is a memcpy'd struct?

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrStencil.h#newcode247
src/gpu/GrStencil.h:247: kNotDisabled_Flag     = 0x2,
Eww, double negative, but it's probably clearer that way.

[bsalomon, 2012-01-03 16:04:27]

It was the chrome/gmail benchmark that showed the improvement.

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrGpuGL.cpp
File src/gpu/GrGpuGL.cpp (right):

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrGpuGL.cpp#newcode1807
src/gpu/GrGpuGL.cpp:1807: fHWStencilClip != stencilClip ||
On 2011/12/19 15:36:46, TomH wrote:
> Just reordering to put the most-likely or cheapest success first?

I guess it is most-likely to fail. I don't fully understand but there was a
noticeable difference with this change on the mac

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrStencil.h
File src/gpu/GrStencil.h (right):

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrStencil.h#newcode213
src/gpu/GrStencil.h:213: // just write an illegal value to the first member
On 2011/12/19 15:36:46, TomH wrote:
> Comment slightly obsolete?

Done.

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrStencil.h#newcode221
src/gpu/GrStencil.h:221: sizeof(fFlags);
On 2011/12/19 15:36:46, TomH wrote:
> Why not rewrite the comparison instead so that we're still guaranteeing tight
> packing including the flags? I presume the reason for wanting tight packing is
> that this is a memcpy'd struct?

It seems we should care about packing for both = and ==. I moved the static
asserts to below the struct. I also explicitly declare the pad bytes and init
them to 0 in the cons (so that we don't fail comparisons due to garbage).

[TomH, 2012-01-03 19:22:31]

lgtm otherwise

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrStencil.h
File src/gpu/GrStencil.h (right):

http://codereview.appspot.com/5489057/diff/1/src/gpu/GrStencil.h#newcode221
src/gpu/GrStencil.h:221: sizeof(fFlags);
On 2012/01/03 16:04:27, bsalomon wrote:
> It seems we should care about packing for both = and ==. I moved the static
> asserts to below the struct. I also explicitly declare the pad bytes and init
> them to 0 in the cons (so that we don't fail comparisons due to garbage).

Init fix is good. Somehow I don't see the moved GR_STATIC_ASSERT anywhere?

[bsalomon, 2012-01-03 19:37:33]

http://codereview.appspot.com/5489057/diff/6004/src/gpu/GrStencil.h
File src/gpu/GrStencil.h (right):

http://codereview.appspot.com/5489057/diff/6004/src/gpu/GrStencil.h#newcode115
src/gpu/GrStencil.h:115: GR_STATIC_ASSERT(sizeof(GrStencilSettingsStruct) ==
Here is the static assertion.

[TomH, 2012-02-07 18:13:22]

Has this landed?

[bsalomon, 2012-02-07 18:14:16]

On 2012/02/07 18:13:22, TomH wrote:
> Has this landed?

Yes, closed.