On Sun, Oct 14, 2018 at 9:45 AM Henri Verbeet hverbeet@gmail.com wrote:
On Sun, 14 Oct 2018 at 01:09, Andrew Wesie awesie@gmail.com wrote:
- GL_EXTCALL(glMemoryBarrier(GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT));
The ARB_buffer_storage spec is written against GL 4.3, but doesn't seem to require it. I think that means you need to check for ARB_SHADER_IMAGE_LOAD_STORE before using glMemoryBarrier().
That would imply that it is impossible to use non-coherent, persistent mappings unless we have the ARB_SHADER_IMAGE_LOAD_STORE extension. And based on the piglit tests, you are probably right. I'll revert back to using GL_MAP_COHERENT_BIT unless I can make a good case for the non-coherent mapping.
- /* NVIDIA requires a sync object. */
I think I'd phrase that as "The ARB_buffer_storage spec requires …", or just quote the relevant lines from the spec literally, and then add a note that NVIDIA appears to enforce that, while Mesa doesn't.
After further investigation, I don't believe that this statement is accurate. If I modify the implementation to avoid ARB_buffer_storage entirely (e.g. use glBufferData, never map the buffer, etc.), the sync object is still required to avoid NVIDIA crashing. I'm more inclined to indicate that this is a driver-specific workaround.
You can demonstrate the bad behavior on NVIDIA with a simple patch to the piglit query_buffer_object tests (which also do not use ARB_buffer_storage):
diff --git a/tests/spec/arb_query_buffer_object/qbo.c b/tests/spec/arb_query_buffer_object/qbo.c index 3ba11ee26..bfedefce0 100644 --- a/tests/spec/arb_query_buffer_object/qbo.c +++ b/tests/spec/arb_query_buffer_object/qbo.c @@ -138,7 +138,10 @@ run_subtest(void) else if (result_type == GL_UNSIGNED_INT) glGetQueryObjectuivARB(query, GL_QUERY_RESULT, BUFFER_OFFSET(0)); else - glGetQueryObjectui64v(query, GL_QUERY_RESULT, BUFFER_OFFSET(0)); + { + for (unsigned int i = 0; i < 100000; i++) + glGetQueryObjectui64v(query, GL_QUERY_RESULT, BUFFER_OFFSET(0)); + } } else { if (result_type == GL_INT) { glGetQueryObjectivARB(query, GL_QUERY_RESULT_AVAILABLE, BUFFER_OFFSET(8));