From 22dc42d6cf91ef90ae3fb6342d8e716666a87df8 Mon Sep 17 00:00:00 2001 From: Lingfeng Yang Date: Tue, 29 May 2018 10:11:38 -0700 Subject: Fix glReadPixels when row length != 0 Fixes: 79208762 The pipe encoder assumes the user in the guest side has allocated all the client memory for glReadPixels including the padding between image rows (the total pitch determined by GL_PACK_ROW_LENGTH), but that is not necessarily the case; the guest can have allocated fewer bytes than the row length would suggest. This can cause memory corruption. This CL detects the case when GL_PACK_ROW_LENGTH != 0 and there is a client buffer for glReadPixels, in which case it takes the pipe buffer and only writes the pixels, row by row, to the client buffer, discarding the padding. this cl does not impact real devices Change-Id: I6fde6677897f2717c7ac05bc349225ea1e02243e Merged-In: I6fde6677897f2717c7ac05bc349225ea1e02243e (cherry picked from commit 372c425bd2d1d67eced350383e6369bca1530b4e) --- host/include/libOpenglRender/IOStream.h | 2 + shared/OpenglCodecCommon/GLClientState.cpp | 23 +++++++++++ shared/OpenglCodecCommon/GLClientState.h | 1 + shared/OpenglCodecCommon/GLESTextureUtils.cpp | 25 ++++++++++++ shared/OpenglCodecCommon/GLESTextureUtils.h | 15 +++++++ system/GLESv2_enc/Android.mk | 1 + system/GLESv2_enc/gl2_enc.cpp | 5 ++- system/enc_common/IOStream_common.cpp | 59 +++++++++++++++++++++++++++ 8 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 system/enc_common/IOStream_common.cpp diff --git a/host/include/libOpenglRender/IOStream.h b/host/include/libOpenglRender/IOStream.h index 445ec17d..1d32ea15 100644 --- a/host/include/libOpenglRender/IOStream.h +++ b/host/include/libOpenglRender/IOStream.h @@ -83,6 +83,8 @@ public: return readFully(buf, len); } + void readbackPixels(void* context, int width, int height, unsigned int format, unsigned int type, void* pixels); + private: unsigned char *m_buf; diff --git a/shared/OpenglCodecCommon/GLClientState.cpp b/shared/OpenglCodecCommon/GLClientState.cpp index 6e9c8f67..7d403894 100644 --- a/shared/OpenglCodecCommon/GLClientState.cpp +++ b/shared/OpenglCodecCommon/GLClientState.cpp @@ -664,6 +664,29 @@ size_t GLClientState::clearBufferNumElts(GLenum buffer) const return 1; } +void GLClientState::getPackingOffsets2D(GLsizei width, GLsizei height, GLenum format, GLenum type, int* startOffset, int* pixelRowSize, int* totalRowSize, int* skipRows) const +{ + if (width <= 0 || height <= 0) { + *startOffset = 0; + *pixelRowSize = 0; + *totalRowSize = 0; + return; + } + + GLESTextureUtils::computePackingOffsets2D( + width, height, + format, type, + m_pixelStore.pack_alignment, + m_pixelStore.pack_row_length, + m_pixelStore.pack_skip_pixels, + m_pixelStore.pack_skip_rows, + startOffset, + pixelRowSize, + totalRowSize); + + *skipRows = m_pixelStore.pack_skip_rows; +} + void GLClientState::setNumActiveUniformsInUniformBlock(GLuint program, GLuint uniformBlockIndex, GLint numActiveUniforms) { UniformBlockInfoKey key; key.program = program; diff --git a/shared/OpenglCodecCommon/GLClientState.h b/shared/OpenglCodecCommon/GLClientState.h index e41cf440..675cea40 100644 --- a/shared/OpenglCodecCommon/GLClientState.h +++ b/shared/OpenglCodecCommon/GLClientState.h @@ -239,6 +239,7 @@ public: size_t pixelDataSize(GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type, int pack) const; size_t pboNeededDataSize(GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type, int pack) const; size_t clearBufferNumElts(GLenum buffer) const; + void getPackingOffsets2D(GLsizei width, GLsizei height, GLenum format, GLenum type, int* startOffset, int* pixelRowSize, int* totalRowSize, int* skipRows) const; void setCurrentProgram(GLint program) { m_currentProgram = program; } void setCurrentShaderProgram(GLint program) { m_currentShaderProgram = program; } diff --git a/shared/OpenglCodecCommon/GLESTextureUtils.cpp b/shared/OpenglCodecCommon/GLESTextureUtils.cpp index 1aef8cb4..45729053 100644 --- a/shared/OpenglCodecCommon/GLESTextureUtils.cpp +++ b/shared/OpenglCodecCommon/GLESTextureUtils.cpp @@ -284,4 +284,29 @@ int computeNeededBufferSize( return end - start; } +void computePackingOffsets2D( + GLsizei width, GLsizei height, + GLenum format, GLenum type, + int packAlignment, + int packRowLength, + int packSkipPixels, + int packSkipRows, + int* startOffset, + int* packingPixelRowSize, + int* packingTotalRowSize) { + + int widthTotal = (packRowLength == 0) ? width : packRowLength; + int totalRowSize = computePitch(widthTotal, format, type, packAlignment); + int pixelsOnlyRowSize = computePitch(width, format, type, packAlignment); + + int packingOffsetStart = + computePackingOffset( + format, type, widthTotal, height, packAlignment, packSkipPixels, packSkipRows, 0 /* skip images = 0 */); + + if (startOffset) *startOffset = packingOffsetStart; + if (packingPixelRowSize) *packingPixelRowSize = pixelsOnlyRowSize; + if (packingTotalRowSize) *packingTotalRowSize = totalRowSize; +} + + } // namespace GLESTextureUtils diff --git a/shared/OpenglCodecCommon/GLESTextureUtils.h b/shared/OpenglCodecCommon/GLESTextureUtils.h index 906e5904..f623d23b 100644 --- a/shared/OpenglCodecCommon/GLESTextureUtils.h +++ b/shared/OpenglCodecCommon/GLESTextureUtils.h @@ -37,6 +37,21 @@ int computeNeededBufferSize( int unpackSkipRows, int unpackSkipImages); +// Writes out |height| offsets for glReadPixels to read back +// data in separate rows of pixels. Returns: +// 1. |startOffset|: offset in bytes to apply at the beginning +// 2. |packingPixelRowSize|: the buffer size in bytes that has the actual pixels per row. +// 2. |packingTotalRowSize|: the length in bytes of each row including the padding from row length. +void computePackingOffsets2D( + GLsizei width, GLsizei height, + GLenum format, GLenum type, + int packAlignment, + int packRowLength, + int packSkipPixels, + int packSkipRows, + int* startOffset, + int* packingPixelRowSize, + int* packingTotalRowSize); } // namespace GLESTextureUtils #endif diff --git a/system/GLESv2_enc/Android.mk b/system/GLESv2_enc/Android.mk index c5081d33..f61c9f70 100644 --- a/system/GLESv2_enc/Android.mk +++ b/system/GLESv2_enc/Android.mk @@ -10,6 +10,7 @@ LOCAL_SRC_FILES := \ gl2_client_context.cpp \ gl2_enc.cpp \ gl2_entry.cpp \ + ../enc_common/IOStream_common.cpp \ LOCAL_CFLAGS += -DLOG_TAG=\"emuglGLESv2_enc\" diff --git a/system/GLESv2_enc/gl2_enc.cpp b/system/GLESv2_enc/gl2_enc.cpp index 8357c686..f42e7ba3 100644 --- a/system/GLESv2_enc/gl2_enc.cpp +++ b/system/GLESv2_enc/gl2_enc.cpp @@ -3028,7 +3028,10 @@ void glReadPixels_enc(void *self , GLint x, GLint y, GLsizei width, GLsizei heig if (useChecksum) checksumCalculator->addBuffer(buf, ptr-buf); if (useChecksum) checksumCalculator->writeChecksum(ptr, checksumSize); ptr += checksumSize; - stream->readback(pixels, __size_pixels); + // TODO: make this part of autogenerated pipe code. + // b/79208762 + stream->readbackPixels(self, width, height, format, type, pixels); + if (useChecksum) checksumCalculator->addBuffer(pixels, __size_pixels); if (useChecksum) { unsigned char *checksumBufPtr = NULL; diff --git a/system/enc_common/IOStream_common.cpp b/system/enc_common/IOStream_common.cpp new file mode 100644 index 00000000..43f03af2 --- /dev/null +++ b/system/enc_common/IOStream_common.cpp @@ -0,0 +1,59 @@ +#include "IOStream.h" + +#include "GL2Encoder.h" + +#include + +#include + +void IOStream::readbackPixels(void* context, int width, int height, unsigned int format, unsigned int type, void* pixels) { + GL2Encoder *ctx = (GL2Encoder *)context; + assert (ctx->state() != NULL); + + int startOffset = 0; + int pixelRowSize = 0; + int totalRowSize = 0; + int skipRows = 0; + + ctx->state()->getPackingOffsets2D(width, height, format, type, + &startOffset, + &pixelRowSize, + &totalRowSize, + &skipRows); + + size_t pixelDataSize = + ctx->state()->pixelDataSize( + width, height, 1, format, type, 1 /* is pack */); + + if (startOffset == 0 && + pixelRowSize == totalRowSize) { + // fast path + readback(pixels, pixelDataSize); + } else if (pixelRowSize == totalRowSize) { + // fast path but with skip in the beginning + std::vector paddingToDiscard(startOffset, 0); + readback(&paddingToDiscard[0], startOffset); + readback((char*)pixels + startOffset, pixelDataSize - startOffset); + } else { + int totalReadback = 0; + + if (startOffset > 0) { + std::vector paddingToDiscard(startOffset, 0); + readback(&paddingToDiscard[0], startOffset); + totalReadback += startOffset; + } + // need to read back row by row + size_t paddingSize = totalRowSize - pixelRowSize; + std::vector paddingToDiscard(paddingSize, 0); + + char* start = (char*)pixels + startOffset; + + for (int i = 0; i < height; i++) { + readback(start, pixelRowSize); + totalReadback += pixelRowSize; + readback(&paddingToDiscard[0], paddingSize); + totalReadback += paddingSize; + start += totalRowSize; + } + } +} -- cgit v1.2.3