Re: [Piglit] [PATCH] Make piglitutil library API-independent
Before submitting a pull request should I rebase my commits for patch on master HEAD to check that there aren't any new includes that involve the old headers? Yes, that's a good idea. Hi, I am sending a pull request rebased on current master. I included it as an attachment since I didn't know if I should paste it here or add as an attachment. There were a few changes to the commits though: * One file needed to have an include statement changed upon the rebase. * piglit-util.h had some new gl-specific function definitions which were moved to -gl-common like others. * I split the first commit into two as Eric suggested. First one moves files piglit-util.[ch] to piglit-util-gl-common.[ch] and fixes all the includes. The second one extracts API-independent code from piglit-util-gl-common.[ch] to new files piglit-util.[ch]. This way it's cleaner to see what was split, because all those includes were renamed in a previous commit. Chad, I went ahead and added your reviewed-by to all the commits although the first one was split into two and therefore it should probably need a quick look at it. In the case everything is OK you can then just pull all the commits. Blaž The following changes since commit 3c830b8713fe2afc7134aad8152427e8c50a41ab: msaa: Test that the centroid interpolation qualifier works correctly. (2012-06-26 07:38:36 -0700) are available in the git repository at: git://github.com/blazt/piglit.git piglitutil-split-v2 for you to fetch changes up to 06fb2c385ec6b026e1800f2f6f0c2b0cf5c3d760: cmake: Build API-independent piglitutil library (2012-06-28 03:55:04 +0200) Blaž Tomažič (6): util: Move piglit-util to piglit-util-gl-common util: Move API-independent code to piglit-util util: Rename piglit-util-enum.c util: Move file reading function to piglit-util cmake: Move piglitutil library to piglitutil_gl cmake: Build API-independent piglitutil library tests/asmparsertest/CMakeLists.gl.txt | 2 +- tests/asmparsertest/asmparsertest.c| 2 +- tests/bugs/CMakeLists.gl.txt | 2 +- tests/bugs/crash-cubemap-order.c | 2 +- tests/bugs/crash-texparameter-before-teximage.c| 2 +- tests/bugs/drawbuffer-modes.c | 2 +- tests/bugs/fdo10370.c | 2 +- tests/bugs/fdo14575.c | 2 +- tests/bugs/fdo20701.c | 2 +- tests/bugs/fdo22540.c | 2 +- tests/bugs/fdo23489.c | 2 +- tests/bugs/fdo23670-depth_test.c | 2 +- tests/bugs/fdo23670-drawpix_stencil.c | 2 +- tests/bugs/fdo24066.c | 2 +- tests/bugs/fdo25614-genmipmap.c| 2 +- tests/bugs/fdo28551.c | 2 +- tests/bugs/fdo31934.c | 2 +- tests/bugs/fdo9833.c | 2 +- tests/bugs/point-sprite.c | 2 +- tests/bugs/r300-readcache.c| 2 +- tests/bugs/tex1d-2dborder.c| 2 +- tests/bugs/tri-tex-crash.c | 2 +- tests/bugs/vbo-buffer-unmap.c | 2 +- tests/egl/CMakeLists.gl.txt| 2 +- tests/egl/egl-create-surface.c | 2 +- tests/egl/egl-nok-swap-region.c| 2 +- tests/egl/egl-nok-texture-from-pixmap.c| 2 +- tests/egl/egl-util.c | 2 +- tests/fbo/CMakeLists.gl.txt| 2 +- tests/fbo/fbo-1d.c | 2 +- tests/fbo/fbo-3d.c | 2 +- tests/fbo/fbo-alpha.c | 2 +- tests/fbo/fbo-alphatest-formats.c | 2 +- tests/fbo/fbo-alphatest-nocolor-ff.c | 2 +- tests/fbo/fbo-alphatest-nocolor.c | 2 +- tests/fbo/fbo-array.c | 2 +- tests/fbo/fbo-bind-renderbuffer.c | 2 +- tests/fbo/fbo-blending-formats.c | 2 +- tests/fbo/fbo-blit-d24s8.c | 2 +- tests/fbo/fbo-blit.c | 2 +- tests/fbo/fbo-clear-formats.c | 2 +- tests/fbo/fbo-clearmipmap.c| 2 +- tests/fbo/fbo-copypix.c| 2 +- tests/fbo/fbo-copyteximage-simple.c| 2 +- tests/fbo/fbo-copyteximage.c | 2 +- tests/fbo/fbo-cubemap.c| 2 +- tests/fbo/fbo-depth-array.c| 2 +- tests/fbo/fbo-depth-sample-compare.c
Re: [Piglit] [PATCH] Make piglitutil library API-independent
On Fri, Jun 22, 2012 at 02:25:57AM +0200, Blaž Tomažič wrote: Currently all utility libraries (piglitutil, piglitutil_gles1, piglitutil_gles2) have built-in the same API-independent code and the code from piglit-util.{c,h} contains OpenGL functionality. To share more code across different APIs (e.g. OpenCL, OpenGL) this code should be split. piglit-util.{c,h} and piglitutil library should be API-independent. * Commits 1 and 2: OpenGL-dependent code from piglit-util.{c,h} is moved to piglit-util-gl-common.{c,h} (All includes from other files are properly renamed). File piglit-util-enum.c is renamed to piglit-util-gl-enum.c for the same purpose, because it contains enums from OpenGL. * Commit 3: The function for reading files from shader-load.c is moved to piglit-util.c, because it doesn't depend on any API or shaders (as its name wrongly suggests). In the process shader-load.c is deleted because it contains no other code. * Commits 4 and 5: The piglitutil library is split to utility library for OpenGL (piglitutil_gl) and API-independent utility library (piglitutil). All other utility libraries are now linked to piglitutil and all OpenGL test executables to piglitutil_gl. The commits for this patch are on this repo [1] on branch piglitutil-split (master..piglitutil-split or 14366de..b38b62c). Hi Blaž, Thanks for splitting this up. It looks OK to me, but someone a little more familiar with the piglit code should probably look at it too. -Tom Blaž [1] git://github.com/blazt/piglit.git ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] Make piglitutil library API-independent
Blaz, Thanks for splitting the earlier patch into this series of 5. As a result, the changes were much easier to review. Everything looks good to me and is Reviewed-by: Chad Versace chad.vers...@linux.intel.com When you feel that the review process is complete and are ready to commit this, submit a pull request. Since Piglit will soon have OpenCL tests, it is probably a good idea for us to begin renaming the GL-specific utility symbols so that they are prefixed with `piglit_gl`. However, there is no rush there. By the way, I briefly looked at your patch cl: Add OpenCL support. I am not familiar with OpenCL, but CMake modifications and the general direction of piglit-util-cl.c look good. I look forward to when the OpenCL tests begin arriving. -Chad ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH] Make piglitutil library API-independent
Currently all utility libraries (piglitutil, piglitutil_gles1, piglitutil_gles2) have built-in the same API-independent code and the code from piglit-util.{c,h} contains OpenGL functionality. To share more code across different APIs (e.g. OpenCL, OpenGL) this code should be split. piglit-util.{c,h} and piglitutil library should be API-independent. * Commits 1 and 2: OpenGL-dependent code from piglit-util.{c,h} is moved to piglit-util-gl-common.{c,h} (All includes from other files are properly renamed). File piglit-util-enum.c is renamed to piglit-util-gl-enum.c for the same purpose, because it contains enums from OpenGL. * Commit 3: The function for reading files from shader-load.c is moved to piglit-util.c, because it doesn't depend on any API or shaders (as its name wrongly suggests). In the process shader-load.c is deleted because it contains no other code. * Commits 4 and 5: The piglitutil library is split to utility library for OpenGL (piglitutil_gl) and API-independent utility library (piglitutil). All other utility libraries are now linked to piglitutil and all OpenGL test executables to piglitutil_gl. The commits for this patch are on this repo [1] on branch piglitutil-split (master..piglitutil-split or 14366de..b38b62c). Blaž [1] git://github.com/blazt/piglit.git ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit