[Piglit] [PATCH 14/14] khr_debug/debug-push-pop-group: fix test

[Timothy Arceri]

On Sat, 2015-04-25 at 12:44 +0200, Marek Olšák wrote:
> If the spec and the example in the spec don't agree, it's a spec bug, isn't it?
> 
> Marek

I should have been a bit clearer in my initial reply.

They do not disagree, it just doesn't explicitly say the message is
added to the log, but neither does DebugMessageInsert().
It seems implied that because you are passing a message to these
functions that it will end up in the log.

I'm assuming the reason PopDebugGroup() explicitly states that a message
is added to the log is because this function does not accept a message
as a parameter, it reuses the one from PushDebugGroup().

> 
> On Sat, Apr 25, 2015 at 6:59 AM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> > On Mon, 2015-04-13 at 20:28 +0200, Marek Olšák wrote:
> >> From: Daniel Kurtz <djkurtz at chromium.org>
> >>
> >> AFAICT from [0], only PopDebugGroup() adds a message to the log, not
> >> PushDebugGroup().
> >>
> >> [0] https://www.opengl.org/registry/specs/KHR/debug.txt
> >>
> >> Thus, there should only be three messages in test_push_pop_debug_group:
> >>   (1) DebugMessageInsert() -> TestMessage1
> >>   (2) PopDebugGroup()      -> TestMessage2
> >>   (3) DebugMessageInsert() -> TestMessage4
> >
> > Although Daniel is correct that the text describing PushDebugGroup does
> > not explicitly say that PushDebugGroup adds a message to the log. I
> > recall basing the piglit test on the example given in the spec which
> > shows the PushDebugGroup does add a message to the log.
> >
> > So this change is incorrect.
> >
> > Example from the spec below:
> >
> >     Scenario 1: skip a section of the code
> >     // Setup of the default active debug group: Filter everything in
> >     glDebugMessageControl(GL_DONT_CARE, GL_DONT_CARE, GL_DONT_CARE, 0,
> > NULL, GL_TRUE);
> >
> >     // Generate a debug marker debug output message
> >     glDebugMessageInsert(
> >         GL_DEBUG_SOURCE_APPLICATION,
> >         GL_DEBUG_TYPE_MARKER, 100,
> >         GL_DEBUG_SEVERITY_NOTIFICATION,
> >         -1, "Message 1");
> >
> >     // Push debug group 1
> >     glPushDebugGroup(
> >         GL_DEBUG_SOURCE_APPLICATION,
> >         1,
> >         -1, "Message 2");
> >
> >     // Setup of the debug group 1: Filter everything out
> >     glDebugMessageControl(GL_DONT_CARE, GL_DONT_CARE, GL_DONT_CARE, 0,
> > NULL, GL_FALSE);
> >
> >     // This message won't appear in the debug output log of
> >     glDebugMessageInsert(
> >         GL_DEBUG_SOURCE_APPLICATION,
> >         GL_DEBUG_TYPE_MARKER, 100,
> >         GL_DEBUG_SEVERITY_NOTIFICATION,
> >         -1, "Message 3");
> >
> >     // Pop debug group 1, restore the volume control of the default
> > debug group.
> >     glPopDebugGroup();
> >
> >     // Generate a debug marker debug output message
> >     glDebugMessageInsert(
> >         GL_DEBUG_SOURCE_APPLICATION,
> >         GL_DEBUG_TYPE_MARKER, 100,
> >         GL_DEBUG_SEVERITY_NOTIFICATION,
> >         -1, "Message 5");
> >
> >     // Expected debug output from the GL implementation
> >     // Message 1
> >     // Message 2
> >     // Message 2
> >     // Message 5
> >
> >
> >
> >
> >>
> >> Signed-off-by: Daniel Kurtz <djkurtz at chromium.org>
> >> ---
> >>  tests/spec/khr_debug/debug-push-pop-group.c | 14 +++++++-------
> >>  1 file changed, 7 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/tests/spec/khr_debug/debug-push-pop-group.c b/tests/spec/khr_debug/debug-push-pop-group.c
> >> index 8fa4474..105bdc4 100644
> >> --- a/tests/spec/khr_debug/debug-push-pop-group.c
> >> +++ b/tests/spec/khr_debug/debug-push-pop-group.c
> >> @@ -268,8 +268,8 @@ static bool test_push_pop_debug_group()
> >>                                    lengths,
> >>                                    messageLog);
> >>
> >> -     if (count != 4) {
> >> -             fprintf(stderr, "The message log should contain 4 messages not %i messages\n", count);
> >> +     if (count != 3) {
> >> +             fprintf(stderr, "The message log should contain 3 messages not %i messages\n", count);
> >>               nextMessage = 0;
> >>               for (i = 0; i < count; i++) {
> >>                       fprintf(stderr, "%s\n", messageLog+nextMessage);
> >> @@ -279,14 +279,14 @@ static bool test_push_pop_debug_group()
> >>       }
> >>
> >>       if (pass) {
> >> -             /* the third message should contain TestMessage2 from PopDebugGroup() */
> >> -             nextMessage = lengths[0] + lengths[1];
> >> +             /* the second message should contain TestMessage2 from PopDebugGroup() */
> >> +             nextMessage = lengths[0];
> >>               if (strstr(messageLog+nextMessage, TestMessage2) == NULL) {
> >>                       fprintf(stderr, "Expected: %s Message: %s\n", TestMessage2, messageLog+nextMessage);
> >>                       pass = false;
> >>               }
> >>
> >> -             /* double check that TestMessage3 didnt sneak into the log */
> >> +             /* double check that TestMessage3 didn't sneak into the log */
> >>               nextMessage = 0;
> >>               for (i = 0; i < count; i++) {
> >>                       if (strstr(messageLog+nextMessage, TestMessage3) != NULL) {
> >> @@ -297,8 +297,8 @@ static bool test_push_pop_debug_group()
> >>                       nextMessage += lengths[i];
> >>               }
> >>
> >> -             /* the forth message should contain TestMessage4 */
> >> -             nextMessage = lengths[0] + lengths[1] + lengths[2];
> >> +             /* the third message should contain TestMessage4 */
> >> +             nextMessage = lengths[0] + lengths[1];
> >>               if (strstr(messageLog+nextMessage, TestMessage4) == NULL) {
> >>                       fprintf(stderr, "Expected: %s Message: %s\n", TestMessage4, messageLog+nextMessage);
> >>                       pass = false;
> >
> >