apitrace/apitrace — pull request #240 by lawlove (2014-03-20 21:34:30+00:00)
Title: qapitrace: startup load of trace fails with groups

Problem:
In TraceLoader::parseTrace, each time glPushDebugGroup is called
within a trace, it's added twice to the group stack which causes
a wrong display when the group branch is expanded (see attached).
In some cases will SIGSEGV

Fix
Use the same logic as in TraceLoader::fetchFrameContents, .i.e,
only add a single glPushDebugGroup Call to the groups stack when
called from the trace.

Note: This is another case for combining the logic used in both
parseTrace and fetchFrameContents into a centralized location
for the TraceLoader class (which I will be submitting). See
details below.

Details:
In going through the TraceLoader.cpp code, I noticed that the same
logic causing the failure in issue #218 in

TraceLoader::fetchFrameContents

is also being used in

TraceLoader::parseTrace

and assumed the same fix was needed there.

In order to test TraceLoader::parseTrace, it was required to force
the boolean return of File::supportsOffsets to false

(If the return value is true, TraceLoader::scanTrace is called to load
the frames, and TraceLoader::fetchFrameContents is called dynamically
on frame expansion to load that Frame's Calls.

If the return value is false (the case needed for testing), then
TraceLoader::parseTrace is called to load the entire trace of all
Frames and Frames' Calls.
)

Forcing File::supportsOffsets to be false did indeed cause qapitrace
to crash and adding the same fix as for TraceLoader::fetchFrameContents
(https://github.com/apitrace/apitrace/pull/228) resolved the issue.

HOWEVER - further testing showed that when a glPushDebugGroup branch
was expanded the contents were not correct which led to this patch.

Caveat:
This patch only addresses the groups stack issue, not the crash
associated with issue #218 (unpaired glPush/PopDeubGroup call).
The #218 issue can be fixed in a separate commit. However as I noted
above it would be better to have this logic in a single location so
fixes don't have to be duplicated. I plan on submitting a patch for
that.

Signed-off-by: Lawrence L Love