Skip to content

Various bug-fixes for playback and recording of GL streams #222

Closed
wants to merge 3 commits into from

2 participants

@idinev
idinev commented Jan 30, 2014

Bugfixes for GL trace/retrace; comments are inlined in the diffs of the original commit.

@idinev

Noticed with Quake3-class games, needed to not leak memory forever, to take longer traces instead of get OOM-killed.

@idinev

Was necessary maybe with glVertexPointer(... stride=256)

apitrace member

I think the problem is that several OpenGL implementations end up reading

numElements * stride

instead of reading

elementSize + (numElements - 1)*stride
@idinev

Defer creation of m_file, was very problematic on OSX otherwise.

apitrace member

Could you elaborate? I don't understand why this would make any difference.

Detected it over a year ago, with ApiTrace1.0, not sure if the issue still stands; can't retest.
So, reject this change.

@idinev

Do not abort if a function is not available. Absolutely vital for cross-platform and cross-vendor debugging. Almost all such cases are calls that can be safely ignored (are minor optimisations/hints).

apitrace member

I'm OK with this when replaying, but not when tracing. See why on issue #184.

You change affects both. Could you refactor this so it doesn't affect tracing?

Improved with
840d649

@idinev

Major fix.
ApiTrace: Detect and fix garbled auto-generated glVertexAttribPointer() calls, where a pointer is used while a VBO is bound

When tracing, just before every drawcall, ApiTrace inserts gl**Pointer() calls into the stream on attributes it detected as resident in user-mem (instead of in a VBO).
ApiTrace doesn't insert a glBindBuffer(GL_ARRRAY_BUFFER,0 ) and glBindBuffer(GL_ARRAY_BUFFER,prevBuf) around those auto-generated calls. Thus, it misrepresents pointers as huge offsets into the currently bound VBO.

This fix detects this issue and inserts a glBindBuffer(,0) /glBindBuffer(,prevVBO) around the problematic auto-generated gl**Pointer() calls.

Noticed with SeriousSam2

apitrace member

Good catch!

This should be fixed fixed when tracing, instead of being worked around when replaying. I did that on change8d1408b3a3c50cebcd8cafeca3ea363aa5f1b3fb .

I verified this on a simple test case.

But if you could confirm this fixes SeriousSam2 it would be great.

I agree that tracing should be fixed, instead of making the retracer guess. I just had a lot of traces I couldn't re-record, so implemented it locally like this.

@idinev

MAJOR
Fix glUnmapBuffer() bug in tracing;
When the app maps 10 buffers and then unmaps each, apitrace was holding info only about the last mapping. On each mapping, it inserts into the trace a call "memcpy(mapped_buf_ptr, blob(map_size), map_size) ". It would add 10 identical memcpy() calls, and not update the 9 other buffers with proper data.

A lot of traces need to be redumped, as this is a serious issue. In Trine2, the characters were missing; in LeagueOfLegends retracing crashes, in Unigine HeavenBenchmark it might be causing pagefaults in the gpu.

apitrace member

I don't understand why this is necessary.

get_buffer_mapping() should only be called for OpenGL implementations with a broken glGetBufferParameteriv(GL_BUFFER_MAP_LENGTH) implementation. This issue has been fixed in Mesa ages ago.

That is the message

apitrace: warning: glGetBufferParameteriv(GL_BUFFER_MAP_LENGTH) failed

should not appear.

Are you actually hitting this in practice? I really think you should upgrade to a more recent version of Mesa drivers if you want to get reliable traces.

apitrace member

I'm not using Mesa for this. This was detected when recording from OSX and Win32. The tracefiles ended-up having 10 memcpy() with identical args, that were inappropriate for 9 of 10 buffers. So, I guess AMD's OSX and nVidia's Win32 drivers had the same GL_BUFFER_MAP_LENGTH driver-bug.

@idinev
idinev commented on 23eb274 Jan 30, 2014

This commit fixes multiple major and minor issues with GL tracing/retracing. Each bug is limited to a single file, generally.
Look at line-comments for descriptions of the bugs and where they were met.

@jrfonseca
apitrace member

Thanks for this.

I prefer addressing one issue per commit, so I'll break down your change in several commits.

Some of the changes I have questions on. I'll comment that separately.

@jrfonseca jrfonseca added a commit that referenced this pull request Feb 3, 2014
@idinev idinev glretrace: Set GLX_SAMPLE_BUFFERS.
Issue #222.
d8f68f4
@jrfonseca
apitrace member

Something in this change causes the https://github.com/apitrace/apitrace-tests/blob/master/apps/gl/map_buffer.c test to fail

$ make -C tests/apps/gl
Running tests...
Test project /home/jfonseca/projects/apitrace/tests/apps/gl
      Start  1: app_gl_default
 1/13 Test  #1: app_gl_default ...................   Passed    0.50 sec
      Start  2: app_gl_default_sb
 2/13 Test  #2: app_gl_default_sb ................   Passed    0.44 sec
      Start  3: app_gl_default_db
 3/13 Test  #3: app_gl_default_db ................   Passed    0.46 sec
      Start  4: app_gl_tri
 4/13 Test  #4: app_gl_tri .......................   Passed    0.51 sec
      Start  5: app_gl_tri_glsl
 5/13 Test  #5: app_gl_tri_glsl ..................   Passed    0.55 sec
      Start  6: app_gl_gremedy
 6/13 Test  #6: app_gl_gremedy ...................   Passed    0.51 sec
      Start  7: app_gl_map_buffer
 7/13 Test  #7: app_gl_map_buffer ................***Failed    0.43 sec
Run application...
/home/jfonseca/projects/apitrace/tests/apps/gl/map_buffer

Capturing trace...
/home/jfonseca/projects/apitrace/apitrace trace -v -a gl -o /home/jfonseca/projects/apitrace/tests/apps/gl/map_buffer.trace -- /home/jfonseca/projects/apitrace/tests/apps/gl/map_buffer
info: found /home/jfonseca/projects/apitrace/wrappers/glxtrace.so
LD_PRELOAD=/home/jfonseca/projects/apitrace/wrappers/glxtrace.so
/home/jfonseca/projects/apitrace/tests/apps/gl/map_buffer
apitrace: loaded
apitrace: tracing to /home/jfonseca/projects/apitrace/tests/apps/gl/map_buffer.trace
apitrace: warning: unknown function "glVDPAUFiniNV"
apitrace: warning: unknown function "glVDPAUGetSurfaceivNV"
apitrace: warning: unknown function "glVDPAUInitNV"
apitrace: warning: unknown function "glVDPAUIsSurfaceNV"
apitrace: warning: unknown function "glVDPAUMapSurfacesNV"
apitrace: warning: unknown function "glVDPAURegisterOutputSurfaceNV"
apitrace: warning: unknown function "glVDPAURegisterVideoSurfaceNV"
apitrace: warning: unknown function "glVDPAUSurfaceAccessNV"
apitrace: warning: unknown function "glVDPAUUnmapSurfacesNV"
apitrace: warning: unknown function "glVDPAUUnregisterSurfaceNV"

Comparing trace /home/jfonseca/projects/apitrace/tests/apps/gl/map_buffer.trace against /home/jfonseca/projects/apitrace/tests/apps/gl/map_buffer.ref.txt...
/home/jfonseca/projects/apitrace/apitrace dump --verbose --color=never /home/jfonseca/projects/apitrace/tests/apps/gl/map_buffer.trace
error: (glXQueryVersion) unknown call detail 103
FAIL (missing call memcpy(dest = <map1>, src = 'blob(200)', n = 200))

If you could look into it would be great.

@jrfonseca jrfonseca added a commit that referenced this pull request Feb 3, 2014
@jrfonseca jrfonseca gltrace: Ensure GL_ARRAY_BUFFER is unbound before emiting fake gl*Poi…
…nter() calls.

When tracing, just before every drawcall, ApiTrace inserts gl*Pointer()
calls into the stream on attributes it detected as resident in user-mem
(instead of in a VBO).

ApiTrace wasn't inserting a glBindBuffer(GL_ARRAY_BUFFER, 0) and
glBindBuffer(GL_ARRAY_BUFFER, prevBuf) around those auto-generated
calls.  Thus, it misrepresented pointers as huge offsets into the
currently bound VBO.

Thanks to idinev for spotting and diagnosing this issue.  This achieves
the same result as idinev's fix in issue #222, but from the trace side.
8d1408b
@idinev
idinev commented Feb 4, 2014

app_gl_map_buffer passes after my checkin to reenable aborts during tracing if APIs are missing. Tested on linux with proprietary AMD drivers.
I've met "unknown call detail XYZ" errors before, only with corrupted files or multithreading issues.

@jrfonseca jrfonseca added a commit that referenced this pull request Oct 6, 2014
@jrfonseca jrfonseca retrace: Put a ceiling into the total size of bound blobs.
Some OpenGL applications that use vertex arrays in user memory were
exausting all memory with bound buffers.  This change enables retrace to
complete.

Merit for the idea goes to idinev, in issue #222.  This implemetation
limits total size of bound blobs, instead of total number.
9f9bc14
@jrfonseca
apitrace member

I believe I've finally finished merging all the changes I could merge.

If you spot anything I left out let me know.

Thank you for your contribution.

@jrfonseca jrfonseca closed this Oct 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.