|Summary:||Track GLBufferObject's Data-Store and it's NIO mapped buffers accurately, synchronized and secure.|
|Product:||[JogAmp] Jogl||Reporter:||Sven Gothel <sgothel>|
|Component:||core||Assignee:||Sven Gothel <sgothel>|
|Severity:||major||CC:||fzanelli, gouessej, sgothel|
7c5483d5b20aed9c87c5ce3f6bc840b6546edcd1 f8a74c9831c65725a699320c27e62161a0378241 30bd30d6563041b71f40e4c336e636768ae26f9d 09fc7aa5539731bb0fba835caee61f6eb837ecff 343deff48d81b0abf58c275d9e0aced12a911802 97f4ef2763596993bcb8a6b84150c9ec906dde08 e7002849ff0b55d86e351a9ccfc279bddd1a6d51
|Bug Depends on:|
Description Sven Gothel 2014-01-14 05:36:51 CET
Currently we are 'lucky' that the JVM implementation's NewDirectByteBuffer JNI method allows us to return the local-reference and use it beyond it's scope! By specification, leaving the stack frame, i.e. the native method, the local reference is no more guaranteed to exist and free to be GC'ed. We need to evaluate adding a global reference via NewGlobalRef, track this object until it's native deletion and issue DeleteGlobalRef. glMapBuffer's mapped memory region is released by the OpenGL driver if: - glUnmapBuffer - unmap buffer - glBufferData - buffer recreation - glDeleteBuffers - buffer deletion Implementation may track the ByteBuffer while mapping it to the target buffer type in GLBufferStateTracker. +++ Due to the above, the GLBufferStateTracker instance must be shared across shared GLContextImpl instances similar to GLSizeStateTracker! This allows us to merge GLSizeStateTracker code into GLBufferStateTracker to simplify the implementation. +++
Comment 1 Sven Gothel 2014-01-14 07:27:36 CET
The following assumption as derived from local/global references from the jni-spec doesn't seem to be correct for an NIO buffer created w/ NewDirectByteBuffer and _returned_ to the java side: +++ Currently we are 'lucky' that the JVM implementation's NewDirectByteBuffer JNI method allows us to return the local-reference and use it beyond it's scope! By specification, leaving the stack frame, i.e. the native method, the local reference is no more guaranteed to exist and free to be GC'ed. +++ Currently we only use the above (global-ref) to store a reference on the JNI side. +++ However, tracking the lifecycle of such a memory mapped object would be useful. Further more, contemplate whether glMapBuffer (etc) methods shall simply return a pre-mapped ByteBuffer (as it is now), or better a 2nd view of such via 'slice()' having it's own position and limit state.
Comment 2 Sven Gothel 2014-01-14 17:05:45 CET
GLBufferStateTracker needs to be thread safe due to shared GLContext.
Comment 3 Sven Gothel 2014-01-14 19:30:16 CET
(In reply to comment #2) > GLBufferStateTracker needs to be thread safe due > to shared GLContext. Note: Only the buffer object is shared - not the binding state!
Comment 4 Sven Gothel 2014-01-14 20:26:22 CET
7c5483d5b20aed9c87c5ce3f6bc840b6546edcd1 f8a74c9831c65725a699320c27e62161a0378241 30bd30d6563041b71f40e4c336e636768ae26f9d Commit f8a74c9831c65725a699320c27e62161a0378241 reverted commit 7c5483d5b20aed9c87c5ce3f6bc840b6546edcd1 due to the fact that the buffer binding itself is _not_ shared across shared GLContext! Apply uncritical changes of 7c5483d5b20aed9c87c5ce3f6bc840b6546edcd1: +++ Simplify GLBufferSizeTracker creation @ GLContextImpl ctor, make it final. +++ Clear the GLBufferSizeTracker (@destruction) only if no more created shares are left! +++ Refine API doc. +++
Comment 5 Sven Gothel 2014-01-21 17:49:39 CET
GLBufferSizeTracker shall become GLBufferObjectTracker and track the buffer's data store, GLBufferStorage, accurately, synchronized and secure. Synchronization is required, since the GLBufferStorage can be shared across many GLContext on multiple threads. This requires all GLBufferStorage lifecycle affecting GL functions to utilize synchronized GLBufferObjectTracker methods while passing a native GL-func callback. These GL functions are: - glBufferData, glBufferStorage (GL 4.4), glNamedBufferDataEXT Creating the GLBufferStorage object - glMapBuffer, glMapBufferRange, and their *Named*EXT variants - glUnmapBuffer, glUnmapNamedBufferEXT 'glDeleteBuffers' can simply notify the GLBufferObjectTracker No more HashMap is required to associate the mapped buffer address to the mapped ByteBuffer. GLBufferObjectTracker simply utilizes a buffer-name (int) -> GLBufferStorage map.
Comment 6 Sven Gothel 2014-01-21 18:07:55 CET
(In reply to comment #5) > GLBufferSizeTracker shall become GLBufferObjectTracker > and track the buffer's data store, GLBufferStorage, accurately, synchronized > and secure. The security aspect shall be implemented by validating all arguments whether they match the required GL constraints, as well as validating tracked states like 'size'. The following functions will throw an GLException accordingly: - glBufferData, glNamedBufferDataEXT * @throws GLException if size is less-than zero * @throws GLException if a native GL-Error occurs - glBufferStorage (GL 4.4) * @throws GLException if size is less-or-eqaul zero * @throws GLException if a native GL-Error occurs - glMapBuffer, and it's *Named*EXT variant * @throws GLException if buffer is not bound to target * @throws GLException if buffer is not tracked * @throws GLException if buffer is already mapped * @throws GLException if buffer has invalid store size, i.e. less-than zero - glMapBufferRange, and it's *Named*EXT variant * @throws GLException if buffer is not bound to target * @throws GLException if buffer is not tracked * @throws GLException if buffer is already mapped * @throws GLException if buffer has invalid store size, i.e. less-than zero * @throws GLException if buffer mapping range does not fit, incl. offset - glMapBufferRange, and it's *Named*EXT variant Only clear mapped buffer reference of GLBufferStorage if native unmap was successful. Further more special error handling shall be applied to: - glMapBuffer, and it's *Named*EXT variant, glMapBuffer, and it's *Named*EXT variant - A zero GLBufferStorage size will avoid a native call and returns null - A null native mapping result indicating an error will not cause a GLException but returns null This allows the user to handle this case.
Comment 7 Sven Gothel 2014-01-21 18:59:45 CET
09fc7aa5539731bb0fba835caee61f6eb837ecff As described in Comment 5 and Comment 6 343deff48d81b0abf58c275d9e0aced12a911802 Add mapped buffer capabilities to GLArrayDataServer and add unit tests GLArrayDataServer: - Add create*Mapped(..) variants for GPU mapped buffer usage w/o client buffers. - Fix API documentation (arguments) - Fix 'addGLSLSubArray(..)' - properly compute and pass 'subStrideB' in bytes to GLArrayDataWrapper ctor. - Add 'mapStorage(..)' and 'unmapStorage(..)' allowing to map the GPU buffer. GLArrayDataWrapper: - Fix getElementCount(): Consider stride in bytes and consider 'mappedElementCount' - getSizeInBytes(): Consider 'mappedElementCount' Tests: - Use new GLBase methods, e.g. getBoundBuffer(..) instead of glGetBoundBuffer(..) - TestMapBufferRead01NEWT: Validate GLBufferStorage (i.e. GLBufferObjectTracker) - Add RedSquareMappedES2 using GPU mapped buffer - Test w/ TestRedSquareES2NEWT, cmd-line 'mappedBuffers' - GearsES1 and GearsES2: Add GPU buffer mapping mode for all test cases - Add buffer validation mode, i.e. test whether GLBufferObjectTracker works properly. - Test w/ TestGearsES2NEWT, cmd-line 'mappedBuffers' - TestSharedContextVBOES2NEWT0, TestSharedContextVBOES2NEWT3: - Add GPU mapped buffers tests to validate GLBufferObjectTracker code path with shared GLContext across multiple threads.
Comment 8 Sven Gothel 2014-01-21 19:00:34 CET
97f4ef2763596993bcb8a6b84150c9ec906dde08 Better shared GLAutoDrawable synchronization. Block slave instances to also block until all master's GLEventListener.init(..) methods have been called - GLSharedContextSetter: Add areAllGLEventListenerInitialized() - GLCanvas (SWT, AWT) - GLJPanel - GLAutoDrawableBase (GLWindow, ..) - GLDrawableHelper's isSharedGLContextPending(..) takes 'areAllGLEventListenerInitialized()' into consideration allowing to block the slave creation until master is completed. This solves teh use case, where the master creates resources in it's GLEventListener initialization (buffers), which are shared with it's slaves.
Comment 9 Sven Gothel 2014-01-22 13:38:47 CET
e7002849ff0b55d86e351a9ccfc279bddd1a6d51 Adding missing 'gl-if-CustomJavaCode-gl2.java' of commit 09fc7aa5539731bb0fba835caee61f6eb837ecff