From eea6d682b8b0f7081f0fe8fab8feadb16e22b30b Mon Sep 17 00:00:00 2001 From: Dan Stoza Date: Mon, 20 Apr 2015 12:07:13 -0700 Subject: [PATCH] libui/libgui: Fix errors in parceling BufferItem and GraphicBuffer were not parceling correctly, which had not been noticed because the libgui tests (specifically, one that tests placing a BufferQueue in a separate process from the IGBP/C) had not been run recently. This change fixes the errors found in those classes. Change-Id: Ie224361a534a79115a3481d83ff97f21d154d4f5 --- libs/gui/BufferItem.cpp | 133 +++++++++++++++++++++++--------------- libs/ui/GraphicBuffer.cpp | 4 +- 2 files changed, 83 insertions(+), 54 deletions(-) diff --git a/libs/gui/BufferItem.cpp b/libs/gui/BufferItem.cpp index 239da20a0..5793d405e 100644 --- a/libs/gui/BufferItem.cpp +++ b/libs/gui/BufferItem.cpp @@ -39,52 +39,66 @@ BufferItem::BufferItem() : BufferItem::~BufferItem() {} +template +static void addAligned(size_t& size, T /* value */) { + size = FlattenableUtils::align(size); + size += sizeof(T); +} + size_t BufferItem::getPodSize() const { - size_t c = sizeof(mCrop) + - sizeof(mTransform) + - sizeof(mScalingMode) + - sizeof(mTimestamp) + - sizeof(mIsAutoTimestamp) + - sizeof(mDataSpace) + - sizeof(mFrameNumber) + - sizeof(mSlot) + - sizeof(mIsDroppable) + - sizeof(mAcquireCalled) + - sizeof(mTransformToDisplayInverse); - return c; + // Must align<8> before writing these fields for this to be correct + size_t size = 0; + addAligned(size, mCrop); + addAligned(size, mTransform); + addAligned(size, mScalingMode); + addAligned(size, mTimestamp); + addAligned(size, mIsAutoTimestamp); + addAligned(size, mDataSpace); + addAligned(size, mFrameNumber); + addAligned(size, mSlot); + addAligned(size, mIsDroppable); + addAligned(size, mAcquireCalled); + addAligned(size, mTransformToDisplayInverse); + return size; } size_t BufferItem::getFlattenedSize() const { - size_t c = 0; + size_t size = sizeof(uint32_t); // Flags if (mGraphicBuffer != 0) { - c += mGraphicBuffer->getFlattenedSize(); - FlattenableUtils::align<4>(c); + size += mGraphicBuffer->getFlattenedSize(); + FlattenableUtils::align<4>(size); } if (mFence != 0) { - c += mFence->getFlattenedSize(); - FlattenableUtils::align<4>(c); + size += mFence->getFlattenedSize(); + FlattenableUtils::align<4>(size); } - c += mSurfaceDamage.getFlattenedSize(); - FlattenableUtils::align<4>(c); - return sizeof(int32_t) + c + getPodSize(); + size += mSurfaceDamage.getFlattenedSize(); + size = FlattenableUtils::align<8>(size); + return size + getPodSize(); } size_t BufferItem::getFdCount() const { - size_t c = 0; + size_t count = 0; if (mGraphicBuffer != 0) { - c += mGraphicBuffer->getFdCount(); + count += mGraphicBuffer->getFdCount(); } if (mFence != 0) { - c += mFence->getFdCount(); + count += mFence->getFdCount(); } - return c; + return count; +} + +template +static void writeAligned(void*& buffer, size_t& size, T value) { + size -= FlattenableUtils::align(buffer); + FlattenableUtils::write(buffer, size, value); } status_t BufferItem::flatten( void*& buffer, size_t& size, int*& fds, size_t& count) const { // make sure we have enough space - if (count < BufferItem::getFlattenedSize()) { + if (size < BufferItem::getFlattenedSize()) { return NO_MEMORY; } @@ -107,35 +121,46 @@ status_t BufferItem::flatten( size -= FlattenableUtils::align<4>(buffer); flags |= 2; } + status_t err = mSurfaceDamage.flatten(buffer, size); if (err) return err; - size -= FlattenableUtils::align<4>(buffer); + FlattenableUtils::advance(buffer, size, mSurfaceDamage.getFlattenedSize()); - // check we have enough space (in case flattening the fence/graphicbuffer lied to us) + // Must align<8> so that getPodSize returns the correct value + size -= FlattenableUtils::align<8>(buffer); + + // Check we still have enough space if (size < getPodSize()) { return NO_MEMORY; } - FlattenableUtils::write(buffer, size, mCrop); - FlattenableUtils::write(buffer, size, mTransform); - FlattenableUtils::write(buffer, size, mScalingMode); - FlattenableUtils::write(buffer, size, mTimestamp); - FlattenableUtils::write(buffer, size, mIsAutoTimestamp); - FlattenableUtils::write(buffer, size, mDataSpace); - FlattenableUtils::write(buffer, size, mFrameNumber); - FlattenableUtils::write(buffer, size, mSlot); - FlattenableUtils::write(buffer, size, mIsDroppable); - FlattenableUtils::write(buffer, size, mAcquireCalled); - FlattenableUtils::write(buffer, size, mTransformToDisplayInverse); + writeAligned(buffer, size, mCrop); + writeAligned(buffer, size, mTransform); + writeAligned(buffer, size, mScalingMode); + writeAligned(buffer, size, mTimestamp); + writeAligned(buffer, size, mIsAutoTimestamp); + writeAligned(buffer, size, mDataSpace); + writeAligned(buffer, size, mFrameNumber); + writeAligned(buffer, size, mSlot); + writeAligned(buffer, size, mIsDroppable); + writeAligned(buffer, size, mAcquireCalled); + writeAligned(buffer, size, mTransformToDisplayInverse); return NO_ERROR; } +template +static void readAligned(const void*& buffer, size_t& size, T& value) { + size -= FlattenableUtils::align(buffer); + FlattenableUtils::read(buffer, size, value); +} + status_t BufferItem::unflatten( void const*& buffer, size_t& size, int const*& fds, size_t& count) { - if (size < sizeof(uint32_t)) + if (size < sizeof(uint32_t)) { return NO_MEMORY; + } uint32_t flags = 0; FlattenableUtils::read(buffer, size, flags); @@ -153,26 +178,30 @@ status_t BufferItem::unflatten( if (err) return err; size -= FlattenableUtils::align<4>(buffer); } + status_t err = mSurfaceDamage.unflatten(buffer, size); if (err) return err; - size -= FlattenableUtils::align<4>(buffer); + FlattenableUtils::advance(buffer, size, mSurfaceDamage.getFlattenedSize()); - // check we have enough space + // Must align<8> so that getPodSize returns the correct value + size -= FlattenableUtils::align<8>(buffer); + + // Check we still have enough space if (size < getPodSize()) { return NO_MEMORY; } - FlattenableUtils::read(buffer, size, mCrop); - FlattenableUtils::read(buffer, size, mTransform); - FlattenableUtils::read(buffer, size, mScalingMode); - FlattenableUtils::read(buffer, size, mTimestamp); - FlattenableUtils::read(buffer, size, mIsAutoTimestamp); - FlattenableUtils::read(buffer, size, mDataSpace); - FlattenableUtils::read(buffer, size, mFrameNumber); - FlattenableUtils::read(buffer, size, mSlot); - FlattenableUtils::read(buffer, size, mIsDroppable); - FlattenableUtils::read(buffer, size, mAcquireCalled); - FlattenableUtils::read(buffer, size, mTransformToDisplayInverse); + readAligned(buffer, size, mCrop); + readAligned(buffer, size, mTransform); + readAligned(buffer, size, mScalingMode); + readAligned(buffer, size, mTimestamp); + readAligned(buffer, size, mIsAutoTimestamp); + readAligned(buffer, size, mDataSpace); + readAligned(buffer, size, mFrameNumber); + readAligned(buffer, size, mSlot); + readAligned(buffer, size, mIsDroppable); + readAligned(buffer, size, mAcquireCalled); + readAligned(buffer, size, mTransformToDisplayInverse); return NO_ERROR; } diff --git a/libs/ui/GraphicBuffer.cpp b/libs/ui/GraphicBuffer.cpp index 638ac6299..52fa0df0b 100644 --- a/libs/ui/GraphicBuffer.cpp +++ b/libs/ui/GraphicBuffer.cpp @@ -303,7 +303,7 @@ status_t GraphicBuffer::flatten(void*& buffer, size_t& size, int*& fds, size_t& static_cast(handle->numInts) * sizeof(int)); } - buffer = reinterpret_cast(static_cast(buffer) + sizeNeeded); + buffer = static_cast(static_cast(buffer) + sizeNeeded); size -= sizeNeeded; if (handle) { fds += handle->numFds; @@ -385,7 +385,7 @@ status_t GraphicBuffer::unflatten( } } - buffer = reinterpret_cast(static_cast(buffer) + sizeNeeded); + buffer = static_cast(static_cast(buffer) + sizeNeeded); size -= sizeNeeded; fds += numFds; count -= numFds;