Hi Alejandro On 26 April 2015 at 14:23, Alejandro Piñeiro <apinhe...@igalia.com> wrote: > On 26/04/15 00:08, Timothy Arceri wrote: >> On Sat, 2015-04-25 at 18:46 +0200, Alejandro Piñeiro wrote: >>> There was a typo on commit c0cd5b, doing it when explicit_binding >>> was false. This prevented to use any binding point different to 0.
Considering that the offending commit landed in 10.4-devel, please add the following line in the commit message. This way we won't forget picking this up for the stable branches. Cc: "10.4 10.5" <mesa-sta...@lists.freedesktop.org> >>> --- >>> >>> Taking into account the explanation on the header about the >>> variable binding (ast.h:553) >>> >>> /** >>> * Binding specified via GL_ARB_shading_language_420pack's "binding" >>> keyword. >>> * >>> * \note >>> * This field is only valid if \c explicit_binding is set. >>> */ >>> int binding; >>> >>> The binding is correct (and should be updated) if explicit_binding is true. >>> But the current behaviour was updating it if it was false. >>> >>> This was not detected by piglit because all the calls to >>> glBindBufferBase(GL_ATOMIC_COUNTER_BUFFER are using 0. >>> >>> I tested this patch by running all piglit on my system, and I didn't >>> detect regression. I also runned make check without issues. >>> >>> https://bugs.freedesktop.org/show_bug.cgi?id=90175 Please move the bug entry to the commit message and prefix with Bugzilla: >> You should probably convert your test program to a piglit test also so >> this bug can be detected if it happens again in the future. > > There are several piglit tests at spec/arb_shader_atomic_counters > testing that you get what you expect while using atomic counters. > Probably it would be enough to just modify some of the already existing > tests, using a non-zero binding point (for example at semantics.c). > > But I don't have too much experience tweaking/creating piglit tests. > What option would be preferred for this case? A new test or just modify > a little one of the already available? > Don't think that changing the behaviour of existing tests is a good idea. As Timothy mentioned, you can add a subtest which will cut down the copy/pasta a bit. Cheers, Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
