Ganesh: caching texture domain rect to optimize uniform uploads

[junov1, 2011-05-20 15:41:12]

PTAL

[junov1, 2011-05-20 15:44:27]

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

http://codereview.appspot.com/4536073/diff/1/gpu/src/GrGLProgram.h#newcode196
gpu/src/GrGLProgram.h:196: memset(fTextureDomain, 0, sizeof(GrGLfloat) *
GrDrawTarget::kNumStages * 4);
because GrGLfloat has no inializing constructor

http://codereview.appspot.com/4536073/diff/1/gpu/src/GrGLProgram.h#newcode228
gpu/src/GrGLProgram.h:228: GrGLfloat                  
fTextureDomain[GrDrawTarget::kNumStages][4];
GrGLFloat rather that GrRect because we need the transform result to be cached,
not the original rect stored in the draw state

[bsalomon, 2011-05-20 17:50:28]

Hey Justin,

Is there a reason we transform before caching? Would the combo of ((dirtyflag)
|| (changed unxformed rect)) work?  I just ask because we store the other values
in their skia/gr rep. 

Also, the other values are reset getUniformLocationsAndInitCache. We should
probably do that for the bounds as well just to be consistent.

[junov, 2011-05-20 19:00:31]

Just caching the untransformed rect does not work because the transform
parameters can change.
So either we compare the transformed rect, or we compare {untrasformed rect,
texture orientation, texture scale x, and texture scale y}

On Fri, May 20, 2011 at 1:50 PM, <bsalomon@google.com> wrote:

> Hey Justin,
>
> Is there a reason we transform before caching? Would the combo of
> ((dirtyflag) || (changed unxformed rect)) work?  I just ask because we
> store the other values in their skia/gr rep.
>
> Also, the other values are reset getUniformLocationsAndInitCache. We
> should probably do that for the bounds as well just to be consistent.
>
>
> http://codereview.appspot.com/4536073/
>

[bsalomon, 2011-05-20 20:52:48]

On 2011/05/20 19:00:31, junov wrote:
> Just caching the untransformed rect does not work because the transform
> parameters can change.
> So either we compare the transformed rect, or we compare {untrasformed rect,
> texture orientation, texture scale x, and texture scale y}

Wouldn't the texture dirty flag catch this? Its supposed to be set when we bind
a new texture. flushTextureMatrix has the same issue.

> 
> On Fri, May 20, 2011 at 1:50 PM, <mailto:bsalomon@google.com> wrote:
> 
> > Hey Justin,
> >
> > Is there a reason we transform before caching? Would the combo of
> > ((dirtyflag) || (changed unxformed rect)) work?  I just ask because we
> > store the other values in their skia/gr rep.
> >
> > Also, the other values are reset getUniformLocationsAndInitCache. We
> > should probably do that for the bounds as well just to be consistent.
> >
> >
> > http://codereview.appspot.com/4536073/
> >

[junov, 2011-05-20 22:06:25]

Ah, then I guess that would be fine.  I will update the change.

On Fri, May 20, 2011 at 4:52 PM, <bsalomon@google.com> wrote:

> On 2011/05/20 19:00:31, junov wrote:
>
>> Just caching the untransformed rect does not work because the
>>
> transform
>
>> parameters can change.
>> So either we compare the transformed rect, or we compare {untrasformed
>>
> rect,
>
>> texture orientation, texture scale x, and texture scale y}
>>
>
> Wouldn't the texture dirty flag catch this? Its supposed to be set when
> we bind a new texture. flushTextureMatrix has the same issue.
>
>
>
>  On Fri, May 20, 2011 at 1:50 PM, <mailto:bsalomon@google.com> wrote:
>>
>
>  > Hey Justin,
>> >
>> > Is there a reason we transform before caching? Would the combo of
>> > ((dirtyflag) || (changed unxformed rect)) work?  I just ask because
>>
> we
>
>> > store the other values in their skia/gr rep.
>> >
>> > Also, the other values are reset getUniformLocationsAndInitCache. We
>> > should probably do that for the bounds as well just to be
>>
> consistent.
>
>> >
>> >
>> > http://codereview.appspot.com/4536073/
>> >
>>
>
>
>
> http://codereview.appspot.com/4536073/
>

[bsalomon, 2011-05-20 22:13:00]

On Fri, May 20, 2011 at 6:06 PM, Justin Novosad <junov@google.com> wrote:

> Ah, then I guess that would be fine.  I will update the change.
>
>
Cool, we have a bug that this flag is set more often than it should be. I'll
fix that soon.


>
> On Fri, May 20, 2011 at 4:52 PM, <bsalomon@google.com> wrote:
>
>> On 2011/05/20 19:00:31, junov wrote:
>>
>>> Just caching the untransformed rect does not work because the
>>>
>> transform
>>
>>> parameters can change.
>>> So either we compare the transformed rect, or we compare {untrasformed
>>>
>> rect,
>>
>>> texture orientation, texture scale x, and texture scale y}
>>>
>>
>> Wouldn't the texture dirty flag catch this? Its supposed to be set when
>> we bind a new texture. flushTextureMatrix has the same issue.
>>
>>
>>
>>  On Fri, May 20, 2011 at 1:50 PM, <mailto:bsalomon@google.com> wrote:
>>>
>>
>>  > Hey Justin,
>>> >
>>> > Is there a reason we transform before caching? Would the combo of
>>> > ((dirtyflag) || (changed unxformed rect)) work?  I just ask because
>>>
>> we
>>
>>> > store the other values in their skia/gr rep.
>>> >
>>> > Also, the other values are reset getUniformLocationsAndInitCache. We
>>> > should probably do that for the bounds as well just to be
>>>
>> consistent.
>>
>>> >
>>> >
>>> > http://codereview.appspot.com/4536073/
>>> >
>>>
>>
>>
>>
>> http://codereview.appspot.com/4536073/
>>
>
>