Skip to content

Commit

Permalink
Enable asynchronous glReadPixels on Windows.
Browse files Browse the repository at this point in the history
Chromium is using the GL ES 2.0 spec with ANGLE, which doesn't allow
for async readback on its own.  However, ANGLE also provides two
extensions which, together, provide all the needed extra implementation:
GL_NV_pixel_buffer_object and GL_EXT_map_buffer_range.

This change enables the appropriate feature flags when the two
extensions are present.  It also changes the auto-generated bindings for
the glMapBufferRange function so that glMapBufferRangeEXT is used
instead of glMapBufferRange when ANGLE is initialized below the version
3 spec.

In addition, this change adds propagation of service-side glMapBuffer()
errors back to the glMapBufferCHROMIUM call client-side.  In the process
of writing tests for this, a minor shared memory allocation bug was
revealed and also fixed.

BUG=431420

Review URL: https://codereview.chromium.org/706173005

Cr-Commit-Position: refs/heads/master@{#304340}
  • Loading branch information
miu-chromium authored and Commit bot committed Nov 15, 2014
1 parent 9c7159e commit b70d785
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 12 deletions.
9 changes: 4 additions & 5 deletions gpu/command_buffer/client/fenced_allocator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,22 @@ namespace gpu {

namespace {

// Allocation alignment, must be a power of two.
const unsigned int kAllocAlignment = 16;

// Round down to the largest multiple of kAllocAlignment no greater than |size|.
unsigned int RoundDown(unsigned int size) {
return size & ~(kAllocAlignment - 1);
return size & ~(FencedAllocator::kAllocAlignment - 1);
}

// Round up to the smallest multiple of kAllocAlignment no smaller than |size|.
unsigned int RoundUp(unsigned int size) {
return (size + (kAllocAlignment - 1)) & ~(kAllocAlignment - 1);
return (size + (FencedAllocator::kAllocAlignment - 1)) &
~(FencedAllocator::kAllocAlignment - 1);
}

} // namespace

#ifndef _MSC_VER
const FencedAllocator::Offset FencedAllocator::kInvalidOffset;
const unsigned int FencedAllocator::kAllocAlignment;
#endif

FencedAllocator::FencedAllocator(unsigned int size,
Expand Down
3 changes: 3 additions & 0 deletions gpu/command_buffer/client/fenced_allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ class GPU_EXPORT FencedAllocator {
// Invalid offset, returned by Alloc in case of failure.
static const Offset kInvalidOffset = 0xffffffffU;

// Allocation alignment, must be a power of two.
static const unsigned int kAllocAlignment = 16;

// Creates a FencedAllocator. Note that the size of the buffer is passed, but
// not its base address: everything is handled as offsets into the buffer.
FencedAllocator(unsigned int size,
Expand Down
2 changes: 1 addition & 1 deletion gpu/command_buffer/client/mapped_memory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ MemoryChunk::~MemoryChunk() {}
MappedMemoryManager::MappedMemoryManager(CommandBufferHelper* helper,
const base::Closure& poll_callback,
size_t unused_memory_reclaim_limit)
: chunk_size_multiple_(1),
: chunk_size_multiple_(FencedAllocator::kAllocAlignment),
helper_(helper),
poll_callback_(poll_callback),
allocated_memory_(0),
Expand Down
2 changes: 1 addition & 1 deletion gpu/command_buffer/client/mapped_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ class GPU_EXPORT MappedMemoryManager {
}

void set_chunk_size_multiple(unsigned int multiple) {
DCHECK(multiple % FencedAllocator::kAllocAlignment == 0);
chunk_size_multiple_ = multiple;
}

Expand Down Expand Up @@ -201,4 +202,3 @@ class GPU_EXPORT MappedMemoryManager {
} // namespace gpu

#endif // GPU_COMMAND_BUFFER_CLIENT_MAPPED_MEMORY_H_

6 changes: 4 additions & 2 deletions gpu/command_buffer/service/feature_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -858,12 +858,14 @@ void FeatureInfo::InitializeFeatures() {
UMA_HISTOGRAM_BOOLEAN("GPU.FenceSupport", ui_gl_fence_works);

feature_flags_.map_buffer_range =
is_es3 || extensions.Contains("GL_ARB_map_buffer_range");
is_es3 || extensions.Contains("GL_ARB_map_buffer_range") ||
extensions.Contains("GL_EXT_map_buffer_range");

// Really it's part of core OpenGL 2.1 and up, but let's assume the
// extension is still advertised.
bool has_pixel_buffers =
is_es3 || extensions.Contains("GL_ARB_pixel_buffer_object");
is_es3 || extensions.Contains("GL_ARB_pixel_buffer_object") ||
extensions.Contains("GL_NV_pixel_buffer_object");

// We will use either glMapBuffer() or glMapBufferRange() for async readbacks.
if (has_pixel_buffers && ui_gl_fence_works &&
Expand Down
10 changes: 9 additions & 1 deletion gpu/command_buffer/service/gles2_cmd_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7578,6 +7578,11 @@ void GLES2DecoderImpl::FinishReadPixels(
} else {
data = glMapBuffer(GL_PIXEL_PACK_BUFFER_ARB, GL_READ_ONLY);
}
if (!data) {
LOCAL_SET_GL_ERROR(GL_OUT_OF_MEMORY, "glMapBuffer",
"Unable to map memory for readback.");
return;
}
memcpy(pixels, data, pixels_size);
// GL_PIXEL_PACK_BUFFER_ARB is currently unused, so we don't
// have to restore the state.
Expand Down Expand Up @@ -7779,7 +7784,10 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32 immediate_data_size,
GLuint buffer = 0;
glGenBuffersARB(1, &buffer);
glBindBuffer(GL_PIXEL_PACK_BUFFER_ARB, buffer);
glBufferData(GL_PIXEL_PACK_BUFFER_ARB, pixels_size, NULL, GL_STREAM_READ);
// For ANGLE client version 2, GL_STREAM_READ is not available.
const GLenum usage_hint =
features().is_angle ? GL_STATIC_DRAW : GL_STREAM_READ;
glBufferData(GL_PIXEL_PACK_BUFFER_ARB, pixels_size, NULL, usage_hint);
GLenum error = glGetError();
if (error == GL_NO_ERROR) {
glReadPixels(x, y, width, height, format, type, 0);
Expand Down
10 changes: 8 additions & 2 deletions ui/gl/generate_bindings.py
Original file line number Diff line number Diff line change
Expand Up @@ -595,9 +595,15 @@
'names': ['glMapBufferOES', 'glMapBuffer'],
'arguments': 'GLenum target, GLenum access', },
{ 'return_type': 'void*',
'names': ['glMapBufferRange'],
'known_as': 'glMapBufferRange',
'versions': [{ 'name': 'glMapBufferRange',
'gl_versions': ['gl3', 'gl4', 'es3'] },
{ 'name': 'glMapBufferRange',
'extensions': ['GL_ARB_map_buffer_range'] },
{ 'name': 'glMapBufferRangeEXT',
'extensions': ['GL_EXT_map_buffer_range'] }],
'arguments':
'GLenum target, GLintptr offset, GLsizeiptr length, GLenum access', },
'GLenum target, GLintptr offset, GLsizeiptr length, GLbitfield access', },
{ 'return_type': 'void',
'known_as': 'glMatrixLoadfEXT',
'versions': [{ 'name': 'glMatrixLoadfEXT',
Expand Down

0 comments on commit b70d785

Please sign in to comment.