mesa3d-dev

[Mesa3d-dev] [PATCH] glGet: Optimize glGet calls by avoiding redundant state updates

[Robert B. <bo...@o-...>]

Hi,

Every now and then while working on the Clutter toolkit we end up with
glGet{Integer,Float}v in our profiles. Our typical response is to simply
cache the offending state so as to avoid querying OpenGL, but it feels
like this is papering over the problem. 

I've attached a patch that instead tries to address the performance
problem in Mesa and hoping something like this could be accepted
upstream?

kind regards,
- Robert
-- 
Robert Bragg, Intel Open Source Technology Center

Re: [Mesa3d-dev] [PATCH] glGet: Optimize glGet calls by avoiding redundant state updates

[Brian P. <br...@vm...>]

Robert Bragg wrote:
> Hi,
> 
> Every now and then while working on the Clutter toolkit we end up with
> glGet{Integer,Float}v in our profiles. Our typical response is to simply
> cache the offending state so as to avoid querying OpenGL, but it feels
> like this is papering over the problem. 
> 
> I've attached a patch that instead tries to address the performance
> problem in Mesa and hoping something like this could be accepted
> upstream?

Coincidentally, I was looking at this just recently.

It would be a good change to make.  Though I think I'd add a new flag 
to the StateVars table entries to indicate whether the state requires 
validation (and what state).  Then auto-generate the needed code.

So GL_RED_BITS case would be predicated like this:

    case GL_RED_BITS:
       if (ctx->NewState & _NEW_BUFFERS)
          _mesa_update_state(ctx);
       params[0] = ctx->DrawBuffer->Visual.redBits;
       break;

_NEW_BUFFERS would be a new field in the StateVars list.


Most state queries don't need validation.  The ones that do are the 
ones that you patched plus the "current" attribs 
(_NEW_CURRENT_ATTRIB).  Maybe a few others?

Are you interested in coding this up?

-Brian

Re: [Mesa3d-dev] [PATCH] glGet: Optimize glGet calls by avoiding redundant state updates

[Brian P. <br...@vm...>]

Following up on an message from a while ago:

Brian Paul wrote:
> Robert Bragg wrote:
>> Hi,
>>
>> Every now and then while working on the Clutter toolkit we end up with
>> glGet{Integer,Float}v in our profiles. Our typical response is to simply
>> cache the offending state so as to avoid querying OpenGL, but it feels
>> like this is papering over the problem.
>> I've attached a patch that instead tries to address the performance
>> problem in Mesa and hoping something like this could be accepted
>> upstream?
> 
> Coincidentally, I was looking at this just recently.
> 
> It would be a good change to make.  Though I think I'd add a new flag to 
> the StateVars table entries to indicate whether the state requires 
> validation (and what state).  Then auto-generate the needed code.
> 
> So GL_RED_BITS case would be predicated like this:
> 
>    case GL_RED_BITS:
>       if (ctx->NewState & _NEW_BUFFERS)
>          _mesa_update_state(ctx);
>       params[0] = ctx->DrawBuffer->Visual.redBits;
>       break;
> 
> _NEW_BUFFERS would be a new field in the StateVars list.
> 
> 
> Most state queries don't need validation.  The ones that do are the ones 
> that you patched plus the "current" attribs (_NEW_CURRENT_ATTRIB).  
> Maybe a few others?
> 
> Are you interested in coding this up?

I've implemented change this and will be committing it soon.

-Brian

Re: [Mesa3d-dev] [PATCH] glGet: Optimize glGet calls by avoiding redundant state updates

[Robert B. <bo...@o-...>]

Excerpts from Brian Paul's message of Fri Mar 26 16:13:35 +0000 2010:
> Following up on an message from a while ago:
> 
> Brian Paul wrote:
> > Robert Bragg wrote:
> >> Hi,
> >>
> >> Every now and then while working on the Clutter toolkit we end up with
> >> glGet{Integer,Float}v in our profiles. Our typical response is to simply
> >> cache the offending state so as to avoid querying OpenGL, but it feels
> >> like this is papering over the problem.
> >> I've attached a patch that instead tries to address the performance
> >> problem in Mesa and hoping something like this could be accepted
> >> upstream?
> > 
> > Coincidentally, I was looking at this just recently.
> > 
> > It would be a good change to make.  Though I think I'd add a new flag to 
> > the StateVars table entries to indicate whether the state requires 
> > validation (and what state).  Then auto-generate the needed code.
> > 
> > So GL_RED_BITS case would be predicated like this:
> > 
> >    case GL_RED_BITS:
> >       if (ctx->NewState & _NEW_BUFFERS)
> >          _mesa_update_state(ctx);
> >       params[0] = ctx->DrawBuffer->Visual.redBits;
> >       break;
> > 
> > _NEW_BUFFERS would be a new field in the StateVars list.
> > 
> > 
> > Most state queries don't need validation.  The ones that do are the ones 
> > that you patched plus the "current" attribs (_NEW_CURRENT_ATTRIB).  
> > Maybe a few others?
> > 
> > Are you interested in coding this up?
> 
> I've implemented change this and will be committing it soon.
> 
> -Brian

thanks, sorry I didn't get around to looking at this again myself.
regards,
- Robert

-- 
Robert Bragg, Intel Open Source Technology Center