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'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. 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. Keith ------------------------------------------------------------------------------ _______________________________________________ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
