On Thu, Jul 02, 2009 at 06:03:01PM +0100, Keith Whitwell wrote: > On Thu, 2009-07-02 at 09:20 -0700, Ian Romanick wrote: > > -----BEGIN PGP SIGNED MESSAGE----- > > Hash: SHA1 > > > > Keith Whitwell wrote: > > > Module: Mesa > > > Branch: mesa_7_5_branch > > > Commit: 47173cf67f0ed55012b0820397f6d620d8ad4473 > > > URL: > > > http://cgit.freedesktop.org/mesa/mesa/commit/?id=47173cf67f0ed55012b0820397f6d620d8ad4473 > > > > > > Author: Keith Whitwell <kei...@vmware.com> > > > Date: Tue Jun 30 09:55:33 2009 +0100 > > > > > > mesa/dlist: shortcircuit some redundant statechanges at compile time > > > > > > Currently, state-changes in mesa display lists are more or less > > > a verbatim recording of the GL calls made during compilation. > > > > > > This change introduces a minor optimization to recognize and eliminate > > > cases where the application emits redundant state changes, eg: > > > > > > glShadeModel( GL_FLAT ); > > > glBegin( prim ) > > > ... > > > glEnd() > > > glShadeModel( GL_FLAT ); > > > glBegin( prim ) > > > ... > > > glEnd() > > > > > > The big win is when we can eliminate all the statechanges between two > > > primitive blocks and combine them into a single VBO node. > > > > > > This commit implements state-change elimination for Material and > > > ShadeModel > > > only. This is enough to make a start on debugging, etc. > > > > Uh... did you intend to merge these into the 7.5 branch? RC4 seems a > > little late to merge significant changes that aren't bug fixes. I've > > looked at the changes via the commit messages to your working branch. > > They look good, but it seems like they could use more soak time before > > hitting a stable branch. > > Thanks for reviewing the patches, Ian. I probably could have added some > background to let people know what the intentions are. > > The merged code has two natures: > > 1) a performance bug-fix for a specific application which is heavy on > display list geometry but emits a lot of redundant material & shademodel > calls. The before/after difference is between unusable and fine. > > 2) an exploration of how we can introduce far-reaching state-change > optimization for all state in the future. > > At this point the change affects *only* glMaterial and glShadeModel > calls. This helps minimize the potential negative impact of the change > -- at worst (barring asserts, etc) we might end up incorrectly recording > material or flat/smooth shading commands in display lists. > > In the testing for this change I found & fixed two bugs with Mesa that > had gone unnoticed: > - glMaterial calls between begin/ends stopped working some time ago > - display lists with eg. CallList between begin/end weren't working > > The fact that I'm finding bugs not directly related to the change at > hand leads me to believe that the testing for this change has been at > least as good as what Mesa normally gets even with extended exposure.
I guess that's not too surprising. Display lists are pure evil, and they don't get enough exercise in any of our available test suites. Voting to have them removed from OpenGL 3.1 was one of the happier moments in my time on the ARB. :) > I've tested with glean, conform and all the mesa demos which exercise > display lists. > > I've written new tests which exercise tricky conditions which might trap > this code up, and fixed bugs they reveal. Have those been added to glean or piglit or something? > I'm not arguing that display lists aren't a difficult area to work in, > but at this point I feel I've done the work to be as confident about > this change as any of the other fixes going into the 7.5 branch. > > I'm also hopeful that with the number of QA teams following Mesa these > days, any regressions will come to light sooner rather than later. > > I share your concerns about merging this to 7.5 at this late stage, and > talked with Brian about it yesterday. He was OK with it providing the > testing was extensive, which hopefully it has been. Okay. I just wanted to make sure that it wasn't accidental. It was just surprising. It sounds like the changes have been tested. If you guys are comfortable with it, I'm convinced.
signature.asc
Description: Digital signature
------------------------------------------------------------------------------
_______________________________________________ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
