Remove DetachedBufferHandle from libui
DetachedBufferHandle was a temporary measure introduced in Android P
to bridge BufferHub and GraphicBuffer. In Android Q however,
GraphicBuffer will be directly backed by BufferHub, thus
DetachedBufferHandle becomes redundant.
Also note that removing DetachedBufferHandle from libui should bare no
impact on vendors for two reasons:
1. BufferHub in P is only enabled for Daydream ready
devices (i.e. Pixel lines and VR AIO devices). No other vendors should
have BufferHub enabled.
2. DetachedBufferHandle.h was hidden from vndk and thus vendors
shouldn't get access to it anyway.
Bug: 117522732
Test: Build system
Change-Id: I3828eaa9499051e5ad5e4e270b5c26bae5f2c707
diff --git a/libs/gui/BufferHubProducer.cpp b/libs/gui/BufferHubProducer.cpp
index 1a7c2d3..ed773e0 100644
--- a/libs/gui/BufferHubProducer.cpp
+++ b/libs/gui/BufferHubProducer.cpp
@@ -20,7 +20,6 @@
#include <log/log.h>
#include <system/window.h>
#include <ui/BufferHubBuffer.h>
-#include <ui/DetachedBufferHandle.h>
namespace android {
@@ -276,14 +275,10 @@
status_or_handle.error());
return BAD_VALUE;
}
- std::unique_ptr<DetachedBufferHandle> handle =
- DetachedBufferHandle::Create(status_or_handle.take());
- if (!handle->isValid()) {
- ALOGE("detachBuffer: Failed to create a DetachedBufferHandle at slot %zu.", slot);
- return BAD_VALUE;
- }
- return graphic_buffer->setDetachedBufferHandle(std::move(handle));
+ // TODO(b/70912269): Reimplement BufferHubProducer::DetachBufferLocked() once GraphicBuffer can
+ // be directly backed by BufferHub.
+ return INVALID_OPERATION;
}
status_t BufferHubProducer::detachNextBuffer(sp<GraphicBuffer>* out_buffer, sp<Fence>* out_fence) {
@@ -373,7 +368,7 @@
ALOGE("attachBuffer: out_slot cannot be NULL.");
return BAD_VALUE;
}
- if (buffer == nullptr || !buffer->isDetachedBuffer()) {
+ if (buffer == nullptr) {
ALOGE("attachBuffer: invalid GraphicBuffer.");
return BAD_VALUE;
}
@@ -394,45 +389,9 @@
return BAD_VALUE;
}
- // Creates a BufferProducer from the GraphicBuffer.
- std::unique_ptr<DetachedBufferHandle> detached_handle = buffer->takeDetachedBufferHandle();
- if (detached_handle == nullptr) {
- ALOGE("attachBuffer: DetachedBufferHandle cannot be NULL.");
- return BAD_VALUE;
- }
- std::shared_ptr<BufferProducer> buffer_producer =
- BufferProducer::Import(std::move(detached_handle->handle()));
- if (buffer_producer == nullptr) {
- ALOGE("attachBuffer: Failed to import BufferProducer.");
- return BAD_VALUE;
- }
-
- // Adds the BufferProducer into the Queue.
- auto status_or_slot = queue_->InsertBuffer(buffer_producer);
- if (!status_or_slot.ok()) {
- ALOGE("attachBuffer: Failed to insert buffer, error=%d.", status_or_slot.error());
- return BAD_VALUE;
- }
-
- size_t slot = status_or_slot.get();
- ALOGV("attachBuffer: returning slot %zu.", slot);
- if (slot >= static_cast<size_t>(max_buffer_count_)) {
- ALOGE("attachBuffer: Invalid slot: %zu.", slot);
- return BAD_VALUE;
- }
-
- // The just attached buffer should be in dequeued state according to IGraphicBufferProducer
- // interface. In BufferHub's language the buffer should be in Gained state.
- buffers_[slot].mGraphicBuffer = buffer;
- buffers_[slot].mBufferState.attachProducer();
- buffers_[slot].mEglFence = EGL_NO_SYNC_KHR;
- buffers_[slot].mFence = Fence::NO_FENCE;
- buffers_[slot].mRequestBufferCalled = true;
- buffers_[slot].mAcquireCalled = false;
- buffers_[slot].mNeedsReallocation = false;
-
- *out_slot = static_cast<int>(slot);
- return NO_ERROR;
+ // TODO(b/70912269): Reimplement BufferHubProducer::DetachBufferLocked() once GraphicBuffer can
+ // be directly backed by BufferHub.
+ return INVALID_OPERATION;
}
status_t BufferHubProducer::queueBuffer(int slot, const QueueBufferInput& input,
diff --git a/libs/gui/tests/IGraphicBufferProducer_test.cpp b/libs/gui/tests/IGraphicBufferProducer_test.cpp
index 6d03374..aef7aed 100644
--- a/libs/gui/tests/IGraphicBufferProducer_test.cpp
+++ b/libs/gui/tests/IGraphicBufferProducer_test.cpp
@@ -777,14 +777,6 @@
ASSERT_OK(mProducer->detachBuffer(slot));
EXPECT_OK(buffer->initCheck());
- if (GetParam() == USE_BUFFER_HUB_PRODUCER) {
- // For a GraphicBuffer backed by BufferHub, once detached from an IGBP, it should have
- // isDetachedBuffer() set. Note that this only applies to BufferHub.
- EXPECT_TRUE(buffer->isDetachedBuffer());
- } else {
- EXPECT_FALSE(buffer->isDetachedBuffer());
- }
-
ASSERT_OK(mProducer->disconnect(TEST_API));
ASSERT_EQ(NO_INIT, mProducer->attachBuffer(&slot, buffer));
@@ -801,16 +793,7 @@
ASSERT_OK(mProducer->detachBuffer(slot));
EXPECT_OK(buffer->initCheck());
- if (GetParam() == USE_BUFFER_HUB_PRODUCER) {
- // For a GraphicBuffer backed by BufferHub, once detached from an IGBP, it should have
- // isDetachedBuffer() set. Note that this only applies to BufferHub.
- EXPECT_TRUE(buffer->isDetachedBuffer());
- } else {
- EXPECT_FALSE(buffer->isDetachedBuffer());
- }
-
EXPECT_OK(mProducer->attachBuffer(&slot, buffer));
- EXPECT_FALSE(buffer->isDetachedBuffer());
EXPECT_OK(buffer->initCheck());
}
diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp
index a6e6d73..8cc1a4e 100644
--- a/libs/ui/BufferHubBuffer.cpp
+++ b/libs/ui/BufferHubBuffer.cpp
@@ -37,7 +37,6 @@
#pragma clang diagnostic pop
#include <ui/BufferHubBuffer.h>
-#include <ui/DetachedBufferHandle.h>
#include <poll.h>
diff --git a/libs/ui/GraphicBuffer.cpp b/libs/ui/GraphicBuffer.cpp
index c50d1d0..5a1ddee 100644
--- a/libs/ui/GraphicBuffer.cpp
+++ b/libs/ui/GraphicBuffer.cpp
@@ -22,7 +22,6 @@
#include <grallocusage/GrallocUsageConversion.h>
-#include <ui/DetachedBufferHandle.h>
#include <ui/Gralloc2.h>
#include <ui/GraphicBufferAllocator.h>
#include <ui/GraphicBufferMapper.h>
@@ -490,24 +489,6 @@
return NO_ERROR;
}
-bool GraphicBuffer::isDetachedBuffer() const {
- return mDetachedBufferHandle && mDetachedBufferHandle->isValid();
-}
-
-status_t GraphicBuffer::setDetachedBufferHandle(std::unique_ptr<DetachedBufferHandle> channel) {
- if (isDetachedBuffer()) {
- ALOGW("setDetachedBuffer: there is already a BufferHub channel associated with this "
- "GraphicBuffer. Replacing the old one.");
- }
-
- mDetachedBufferHandle = std::move(channel);
- return NO_ERROR;
-}
-
-std::unique_ptr<DetachedBufferHandle> GraphicBuffer::takeDetachedBufferHandle() {
- return std::move(mDetachedBufferHandle);
-}
-
// ---------------------------------------------------------------------------
}; // namespace android
diff --git a/libs/ui/include/ui/DetachedBufferHandle.h b/libs/ui/include/ui/DetachedBufferHandle.h
deleted file mode 100644
index f3c328d..0000000
--- a/libs/ui/include/ui/DetachedBufferHandle.h
+++ /dev/null
@@ -1,52 +0,0 @@
-/*
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef ANDROID_DETACHED_BUFFER_HUB_HANDLE_H
-#define ANDROID_DETACHED_BUFFER_HUB_HANDLE_H
-
-#include <pdx/channel_handle.h>
-
-#include <memory>
-
-namespace android {
-
-// A wrapper that holds a pdx::LocalChannelHandle object. From the handle, a BufferHub buffer can be
-// created. Current implementation assumes that the underlying transport is using libpdx (thus
-// holding a pdx::LocalChannelHandle object), but future implementation can change it to a Binder
-// backend if ever needed.
-class DetachedBufferHandle {
-public:
- static std::unique_ptr<DetachedBufferHandle> Create(pdx::LocalChannelHandle handle) {
- return std::unique_ptr<DetachedBufferHandle>(new DetachedBufferHandle(std::move(handle)));
- }
-
- // Accessors to get or take the internal pdx::LocalChannelHandle.
- pdx::LocalChannelHandle& handle() { return mHandle; }
- const pdx::LocalChannelHandle& handle() const { return mHandle; }
-
- // Returns whether the DetachedBufferHandle holds a BufferHub channel.
- bool isValid() const { return mHandle.valid(); }
-
-private:
- // Constructs a DetachedBufferHandle from a pdx::LocalChannelHandle.
- explicit DetachedBufferHandle(pdx::LocalChannelHandle handle) : mHandle(std::move(handle)) {}
-
- pdx::LocalChannelHandle mHandle;
-};
-
-} // namespace android
-
-#endif // ANDROID_DETACHED_BUFFER_HUB_HANDLE_H
diff --git a/libs/ui/include/ui/GraphicBuffer.h b/libs/ui/include/ui/GraphicBuffer.h
index cc38982..e794462 100644
--- a/libs/ui/include/ui/GraphicBuffer.h
+++ b/libs/ui/include/ui/GraphicBuffer.h
@@ -34,7 +34,6 @@
namespace android {
-class DetachedBufferHandle;
class GraphicBufferMapper;
// ===========================================================================
@@ -191,11 +190,6 @@
status_t flatten(void*& buffer, size_t& size, int*& fds, size_t& count) const;
status_t unflatten(void const*& buffer, size_t& size, int const*& fds, size_t& count);
- // Sets and takes DetachedBuffer. Should only be called from BufferHub.
- bool isDetachedBuffer() const;
- status_t setDetachedBufferHandle(std::unique_ptr<DetachedBufferHandle> detachedBuffer);
- std::unique_ptr<DetachedBufferHandle> takeDetachedBufferHandle();
-
private:
~GraphicBuffer();
@@ -246,17 +240,6 @@
// match the BufferQueue's internal generation number (set through
// IGBP::setGenerationNumber), attempts to attach the buffer will fail.
uint32_t mGenerationNumber;
-
- // Stores a BufferHub handle that can be used to re-attach this GraphicBuffer back into a
- // BufferHub producer/consumer set. In terms of GraphicBuffer's relationship with BufferHub,
- // there are three different modes:
- // 1. Legacy mode: GraphicBuffer is not backed by BufferHub and mDetachedBufferHandle must be
- // invalid.
- // 2. Detached mode: GraphicBuffer is backed by BufferHub, but not part of a producer/consumer
- // set. In this mode, mDetachedBufferHandle must be valid.
- // 3. Attached mode: GraphicBuffer is backed by BufferHub and it's part of a producer/consumer
- // set. In this mode, mDetachedBufferHandle must be invalid.
- std::unique_ptr<DetachedBufferHandle> mDetachedBufferHandle;
};
}; // namespace android
diff --git a/libs/ui/tests/Android.bp b/libs/ui/tests/Android.bp
index e8bda67..1521e1d 100644
--- a/libs/ui/tests/Android.bp
+++ b/libs/ui/tests/Android.bp
@@ -29,13 +29,6 @@
}
cc_test {
- name: "GraphicBuffer_test",
- shared_libs: ["libpdx_default_transport", "libui", "libutils"],
- srcs: ["GraphicBuffer_test.cpp"],
- cflags: ["-Wall", "-Werror"],
-}
-
-cc_test {
name: "BufferHubBuffer_test",
header_libs: [
"libbufferhub_headers",
diff --git a/libs/ui/tests/GraphicBuffer_test.cpp b/libs/ui/tests/GraphicBuffer_test.cpp
deleted file mode 100644
index eb679ac..0000000
--- a/libs/ui/tests/GraphicBuffer_test.cpp
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * Copyright 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#define LOG_TAG "GraphicBufferTest"
-
-#include <ui/DetachedBufferHandle.h>
-#include <ui/GraphicBuffer.h>
-
-#include <gtest/gtest.h>
-
-namespace android {
-
-namespace {
-
-constexpr uint32_t kTestWidth = 1024;
-constexpr uint32_t kTestHeight = 1;
-constexpr uint32_t kTestFormat = HAL_PIXEL_FORMAT_BLOB;
-constexpr uint32_t kTestLayerCount = 1;
-constexpr uint64_t kTestUsage = GraphicBuffer::USAGE_SW_WRITE_OFTEN;
-
-} // namespace
-
-class GraphicBufferTest : public testing::Test {};
-
-TEST_F(GraphicBufferTest, DetachedBuffer) {
- sp<GraphicBuffer> buffer(
- new GraphicBuffer(kTestWidth, kTestHeight, kTestFormat, kTestLayerCount, kTestUsage));
-
- // Currently a newly allocated GraphicBuffer is in legacy mode, i.e. not associated with
- // BufferHub. But this may change in the future.
- EXPECT_FALSE(buffer->isDetachedBuffer());
-
- pdx::LocalChannelHandle channel{nullptr, 1234};
- EXPECT_TRUE(channel.valid());
-
- std::unique_ptr<DetachedBufferHandle> handle = DetachedBufferHandle::Create(std::move(channel));
- EXPECT_FALSE(channel.valid());
- EXPECT_TRUE(handle->isValid());
- EXPECT_TRUE(handle->handle().valid());
-
- buffer->setDetachedBufferHandle(std::move(handle));
- EXPECT_TRUE(handle == nullptr);
- EXPECT_TRUE(buffer->isDetachedBuffer());
-
- handle = buffer->takeDetachedBufferHandle();
- EXPECT_TRUE(handle != nullptr);
- EXPECT_TRUE(handle->isValid());
- EXPECT_FALSE(buffer->isDetachedBuffer());
-}
-
-} // namespace android
diff --git a/libs/vr/libbufferhub/buffer_hub-test.cpp b/libs/vr/libbufferhub/buffer_hub-test.cpp
index db624de..b248ece 100644
--- a/libs/vr/libbufferhub/buffer_hub-test.cpp
+++ b/libs/vr/libbufferhub/buffer_hub-test.cpp
@@ -5,7 +5,6 @@
#include <sys/epoll.h>
#include <sys/eventfd.h>
#include <ui/BufferHubBuffer.h>
-#include <ui/DetachedBufferHandle.h>
#include <mutex>
#include <thread>