-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 04/19/2011 12:23 PM, nobled wrote: > spec file: > http://www.opengl.org/registry/specs/ARB/robustness.txt > > The first four parts of this series add infrastructure to support > bounds-checking client memory buffers by re-using the PBO > bounds-checking code; the fifth patch adds the actual functions of the > extension. However, the series is incomplete because I'm not sure how > to turn the spec file into an xml file for > src/mapi/glapi/gen/ARB_robustness.xml, and then generate the dispatch > stuff from it with the python script(*which* python script?) -- the > docs on this point seem to be a classic combination of completely > outdated and "hey, can you vague that up for me?": >> http://cgit.freedesktop.org/mesa/mesa/tree/docs/devinfo.html >> In the src/mesa/glapi/ directory, add the new extension functions and >> enums to the gl_API.xml file. >> Then, a bunch of source files must be regenerated by executing the >> corresponding Python scripts. > > I think I covered all the other steps, though. At least enough to be > faithful to the spec, as long as these two aren't exposed yet on the > GLX side of things: > http://www.opengl.org/registry/specs/ARB/glx_create_context.txt > http://www.opengl.org/registry/specs/ARB/glx_create_context_robustness.txt > > Still todo: adding piglit tests.
It's *REALLY* hard to review patches sent as attachments. It is much better and easier for reviewers to put comments alongside the code they are commenting about. In the future, could you please send patches using git-send-email? Patch 1/5 looks like it's doing to separate things. It renames / alters _mesa_validate_pbo_access and it starts using _mesa_is_bufferobj in some places. That should probably be split into two patches. First change over to using _mesa_is_bufferobj, then make the other changes. I'm also not sure _mesa_validate_buffer_access is a particularly good name. This loses the information that the validation is for pixel reads / writes. This also applies to _mesa_map_validate_buffer_dest and friends in later patches. For the internal Mesa functions, I think I might also change the parameter bufSize to clientMemorySize or similar. Using bufSize in the API functions (added in patch 5/5) matches the API definition and is fine. The changes to readpix.c in patch 4/5 looks odd. If the access is out-of-bounds and there is PBO, nothing happens but no error is generated. Is that what the spec says to do? It also looks like a lot of the checking for out-of-bounds followed by checking for being mapped could be refactored. Patch 5/5 also does two separate things. It adds data structure infrastructure for the extension and it adds a bunch of entry points. Usually gl_context::ResetStatus and gl_constants::ResetStratgy would be added in a separate patch from adding all the new functions. In patch 5/5 why +_mesa_GetMinmax(GLenum target, GLboolean reset, GLenum format, GLenum type, + GLvoid *values) +{ + const GLsizei bufSize = INT_MAX; + _mesa_GetnMinmaxARB(target, reset, format, type, bufSize, values); +} instead of +_mesa_GetMinmax(GLenum target, GLboolean reset, GLenum format, GLenum type, + GLvoid *values) +{ + _mesa_GetnMinmaxARB(target, reset, format, type, INT_MAX, values); +} On a different note, I explicitly *approve* of the use of 'goto overflow'. :) -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk2t8+oACgkQX1gOwKyEAw9wGQCgnMkLFee64zCdN/c1UutZ613x 4qIAn2ZKxnEuqClIzemoTuc98bewOY8X =Qc3X -----END PGP SIGNATURE----- _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev