On Thu, 2009-07-02 at 11:01 -0700, Ian Romanick wrote: > 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. :)
If they'd been specified slightly differently, it would have been a different story -- making CallList illegal between Begin/End would have made them a lot more amenable to simple analysis. > > 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? Right now they're just in trivial. I think there's a wealth of test cases in there -- it's a worthwhile project to figure out how to pull those into an automated test-suite without losing the aspects that make them so useful as developer's unit tests. > > 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. No worries Ian. Hopefully it'll work out OK -- we're pretty focussed on getting 7.5 right, so if there are issues we'll be on call to put them right... Keith ------------------------------------------------------------------------------ _______________________________________________ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
