composer: cleanup CommandWriterBase and CommandReaderBase part 2

Bug: 208856704
Test: VTS
Change-Id: I30f4cfef66a1c833142b4645f69e8ad9fbf0de8e
diff --git a/graphics/composer/aidl/aidl_api/android.hardware.graphics.composer3/current/android/hardware/graphics/composer3/HandleIndex.aidl b/graphics/composer/aidl/aidl_api/android.hardware.graphics.composer3/current/android/hardware/graphics/composer3/HandleIndex.aidl
deleted file mode 100644
index b87870d..0000000
--- a/graphics/composer/aidl/aidl_api/android.hardware.graphics.composer3/current/android/hardware/graphics/composer3/HandleIndex.aidl
+++ /dev/null
@@ -1,39 +0,0 @@
-/**
- * Copyright (c) 2021, 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.
- */
-///////////////////////////////////////////////////////////////////////////////
-// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE.                          //
-///////////////////////////////////////////////////////////////////////////////
-
-// This file is a snapshot of an AIDL file. Do not edit it manually. There are
-// two cases:
-// 1). this is a frozen version file - do not edit this in any case.
-// 2). this is a 'current' file. If you make a backwards compatible change to
-//     the interface (from the latest frozen version), the build system will
-//     prompt you to update this file with `m <name>-update-api`.
-//
-// You must not make a backward incompatible change to any AIDL file built
-// with the aidl_interface module type with versions property set. The module
-// type is used to build AIDL files in a way that they can be used across
-// independently updatable components of the system. If a device is shipped
-// with such a backward incompatible change, it has a high risk of breaking
-// later when a module using the interface is updated, e.g., Mainline modules.
-
-package android.hardware.graphics.composer3;
-@Backing(type="int") @VintfStability
-enum HandleIndex {
-  EMPTY = -1,
-  CACHED = -2,
-}
diff --git a/graphics/composer/aidl/android/hardware/graphics/composer3/HandleIndex.aidl b/graphics/composer/aidl/android/hardware/graphics/composer3/HandleIndex.aidl
deleted file mode 100644
index 0a93c9e..0000000
--- a/graphics/composer/aidl/android/hardware/graphics/composer3/HandleIndex.aidl
+++ /dev/null
@@ -1,33 +0,0 @@
-/**
- * Copyright (c) 2021, 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.
- */
-
-package android.hardware.graphics.composer3;
-
-/**
- * Special index values (always negative) for command queue commands.
- */
-@VintfStability
-@Backing(type="int")
-enum HandleIndex {
-    /**
-     * No handle
-     */
-    EMPTY = -1,
-    /**
-     * Use cached handle
-     */
-    CACHED = -2,
-}
diff --git a/graphics/composer/aidl/android/hardware/graphics/composer3/translate-ndk.cpp b/graphics/composer/aidl/android/hardware/graphics/composer3/translate-ndk.cpp
index c1e988a..c2a4fdd 100644
--- a/graphics/composer/aidl/android/hardware/graphics/composer3/translate-ndk.cpp
+++ b/graphics/composer/aidl/android/hardware/graphics/composer3/translate-ndk.cpp
@@ -129,15 +129,6 @@
               static_cast<int>(::android::hardware::graphics::composer::V2_1::IComposerClient::
                                        DisplayRequest::WRITE_CLIENT_TARGET_TO_OUTPUT));
 
-static_assert(aidl::android::hardware::graphics::composer3::HandleIndex::EMPTY ==
-              static_cast<aidl::android::hardware::graphics::composer3::HandleIndex>(
-                      ::android::hardware::graphics::composer::V2_1::IComposerClient::HandleIndex::
-                              EMPTY));
-static_assert(aidl::android::hardware::graphics::composer3::HandleIndex::CACHED ==
-              static_cast<aidl::android::hardware::graphics::composer3::HandleIndex>(
-                      ::android::hardware::graphics::composer::V2_1::IComposerClient::HandleIndex::
-                              CACHED));
-
 static_assert(
         aidl::android::hardware::graphics::composer3::PowerMode::OFF ==
         static_cast<aidl::android::hardware::graphics::composer3::PowerMode>(
diff --git a/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_ReadbackTest.cpp b/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_ReadbackTest.cpp
index eddc2d3..96d240a 100644
--- a/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_ReadbackTest.cpp
+++ b/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_ReadbackTest.cpp
@@ -19,6 +19,7 @@
 #include <aidl/Gtest.h>
 #include <aidl/Vintf.h>
 #include <aidl/android/hardware/graphics/common/BufferUsage.h>
+#include <aidl/android/hardware/graphics/composer3/IComposer.h>
 #include <android/binder_manager.h>
 #include <composer-vts/include/ReadbackVts.h>
 #include <composer-vts/include/RenderEngineVts.h>
@@ -103,13 +104,8 @@
         ASSERT_NO_FATAL_FAILURE(mComposerClient->setPowerMode(mPrimaryDisplay, PowerMode::OFF));
         const auto errors = mReader.takeErrors();
         ASSERT_TRUE(mReader.takeErrors().empty());
+        ASSERT_TRUE(mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty());
 
-        std::vector<int64_t> layers;
-        std::vector<Composition> types;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &layers, &types);
-
-        ASSERT_TRUE(layers.empty());
-        ASSERT_TRUE(types.empty());
         if (mComposerCallback != nullptr) {
             EXPECT_EQ(0, mComposerCallback->getInvalidHotplugCount());
             EXPECT_EQ(0, mComposerCallback->getInvalidRefreshCount());
@@ -118,12 +114,14 @@
     }
 
     ::android::sp<::android::GraphicBuffer> allocate() {
+        const auto width = static_cast<uint32_t>(mDisplayWidth);
+        const auto height = static_cast<uint32_t>(mDisplayHeight);
+        const auto usage = static_cast<uint32_t>(common::BufferUsage::CPU_WRITE_OFTEN) |
+                           static_cast<uint32_t>(common::BufferUsage::CPU_READ_OFTEN);
+
         return ::android::sp<::android::GraphicBuffer>::make(
-                mDisplayWidth, mDisplayHeight, ::android::PIXEL_FORMAT_RGBA_8888,
-                /*layerCount*/ 1,
-                static_cast<uint64_t>(static_cast<int>(common::BufferUsage::CPU_WRITE_OFTEN) |
-                                      static_cast<int>(common::BufferUsage::CPU_READ_OFTEN)),
-                "VtsHalGraphicsComposer3_ReadbackTest");
+                width, height, ::android::PIXEL_FORMAT_RGBA_8888,
+                /*layerCount*/ 1u, usage, "VtsHalGraphicsComposer3_ReadbackTest");
     }
 
     void writeLayers(const std::vector<std::shared_ptr<TestLayer>>& layers) {
@@ -141,10 +139,10 @@
         }
 
         std::vector<CommandResultPayload> results;
-        const auto status = mComposerClient->executeCommands(commands, &results);
+        auto status = mComposerClient->executeCommands(commands, &results);
         ASSERT_TRUE(status.isOk()) << "executeCommands failed " << status.getDescription();
 
-        mReader.parse(results);
+        mReader.parse(std::move(results));
         mWriter.reset();
     }
 
@@ -262,11 +260,7 @@
         execute();
         // if hwc cannot handle and asks for composition change,
         // just succeed the test
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -320,11 +314,7 @@
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
 
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -380,11 +370,7 @@
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
 
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -495,14 +481,10 @@
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
 
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
-            ASSERT_EQ(1, changedCompositionLayers.size());
+        auto changedCompositionTypes = mReader.takeChangedCompositionTypes(mPrimaryDisplay);
+        if (!changedCompositionTypes.empty()) {
             ASSERT_EQ(1, changedCompositionTypes.size());
-            ASSERT_EQ(Composition::CLIENT, changedCompositionTypes[0]);
+            ASSERT_EQ(Composition::CLIENT, changedCompositionTypes[0].composition);
 
             PixelFormat clientFormat = PixelFormat::RGBA_8888;
             auto clientUsage = static_cast<uint32_t>(
@@ -537,9 +519,8 @@
             mWriter.setClientTarget(mPrimaryDisplay, 0, mGraphicBuffer->handle, fenceHandle.get(),
                                     clientDataspace, std::vector<common::Rect>(1, damage));
             execute();
-            mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                                &changedCompositionTypes);
-            ASSERT_TRUE(changedCompositionLayers.empty());
+            changedCompositionTypes = mReader.takeChangedCompositionTypes(mPrimaryDisplay);
+            ASSERT_TRUE(changedCompositionTypes.empty());
         }
         ASSERT_TRUE(mReader.takeErrors().empty());
 
@@ -609,15 +590,12 @@
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
 
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
+        auto changedCompositionTypes = mReader.takeChangedCompositionTypes(mPrimaryDisplay);
         if (changedCompositionTypes.size() != 1) {
             continue;
         }
         // create client target buffer
-        ASSERT_EQ(Composition::CLIENT, changedCompositionTypes[0]);
+        ASSERT_EQ(Composition::CLIENT, changedCompositionTypes[0].composition);
         mGraphicBuffer->reallocate(static_cast<uint32_t>(mDisplayWidth),
                                    static_cast<uint32_t>(mDisplayHeight),
                                    static_cast<int32_t>(common::PixelFormat::RGBA_8888),
@@ -642,9 +620,8 @@
         mWriter.setClientTarget(mPrimaryDisplay, 0, mGraphicBuffer->handle, fenceHandle.get(),
                                 clientDataspace, std::vector<common::Rect>(1, clientFrame));
         execute();
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        ASSERT_EQ(0, changedCompositionLayers.size());
+        changedCompositionTypes = mReader.takeChangedCompositionTypes(mPrimaryDisplay);
+        ASSERT_TRUE(changedCompositionTypes.empty());
         ASSERT_TRUE(mReader.takeErrors().empty());
 
         mWriter.presentDisplay(mPrimaryDisplay);
@@ -687,11 +664,7 @@
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -718,10 +691,7 @@
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
         ASSERT_TRUE(mReader.takeErrors().empty());
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        ASSERT_TRUE(changedCompositionLayers.empty());
-        ASSERT_TRUE(changedCompositionTypes.empty());
+        ASSERT_TRUE(mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty());
         mWriter.presentDisplay(mPrimaryDisplay);
         execute();
         ASSERT_TRUE(mReader.takeErrors().empty());
@@ -758,11 +728,7 @@
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -821,11 +787,7 @@
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -879,11 +841,7 @@
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -904,10 +862,7 @@
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        ASSERT_TRUE(changedCompositionLayers.empty());
-        ASSERT_TRUE(changedCompositionTypes.empty());
+        ASSERT_TRUE(mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty());
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.presentDisplay(mPrimaryDisplay);
         execute();
@@ -1029,11 +984,7 @@
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -1074,11 +1025,7 @@
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -1114,11 +1061,7 @@
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -1197,11 +1140,7 @@
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -1243,11 +1182,7 @@
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        std::vector<int64_t> changedCompositionLayers;
-        std::vector<Composition> changedCompositionTypes;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &changedCompositionLayers,
-                                            &changedCompositionTypes);
-        if (!changedCompositionLayers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
@@ -1289,10 +1224,7 @@
         ASSERT_TRUE(mReader.takeErrors().empty());
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        std::vector<int64_t> layers;
-        std::vector<Composition> types;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &layers, &types);
-        if (!layers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED();
             return;
         }
diff --git a/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_TargetTest.cpp b/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_TargetTest.cpp
index 6638744..4dbe191 100644
--- a/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_TargetTest.cpp
+++ b/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_TargetTest.cpp
@@ -1142,13 +1142,7 @@
     void TearDown() override {
         const auto errors = mReader.takeErrors();
         ASSERT_TRUE(mReader.takeErrors().empty());
-
-        std::vector<int64_t> layers;
-        std::vector<Composition> types;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &layers, &types);
-
-        ASSERT_TRUE(layers.empty());
-        ASSERT_TRUE(types.empty());
+        ASSERT_TRUE(mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty());
 
         ASSERT_NO_FATAL_FAILURE(GraphicsComposerAidlTest::TearDown());
     }
@@ -1164,7 +1158,7 @@
         const auto status = mComposerClient->executeCommands(commands, &results);
         ASSERT_TRUE(status.isOk()) << "executeCommands failed " << status.getDescription();
 
-        mReader.parse(results);
+        mReader.parse(std::move(results));
         mWriter.reset();
     }
 
@@ -1299,10 +1293,12 @@
         execute();
         EXPECT_TRUE(mReader.takeErrors().empty());
 
-        int presentFence;
-        mReader.takePresentFence(mPrimaryDisplay, &presentFence);
-        EXPECT_NE(-1, presentFence);
-        return sp<::android::Fence>::make(presentFence);
+        auto presentFence = mReader.takePresentFence(mPrimaryDisplay);
+        // take ownership
+        const int fenceOwner = presentFence.get();
+        *presentFence.getR() = -1;
+        EXPECT_NE(-1, fenceOwner);
+        return sp<::android::Fence>::make(fenceOwner);
     }
 
     int32_t getVsyncPeriod() {
@@ -1592,10 +1588,7 @@
 
         mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
         execute();
-        std::vector<int64_t> layers;
-        std::vector<Composition> types;
-        mReader.takeChangedCompositionTypes(mPrimaryDisplay, &layers, &types);
-        if (!layers.empty()) {
+        if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
             GTEST_SUCCEED() << "Composition change requested, skipping test";
             return;
         }
@@ -1640,10 +1633,8 @@
     mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
 
     execute();
-    std::vector<int64_t> layers;
-    std::vector<Composition> types;
-    mReader.takeChangedCompositionTypes(mPrimaryDisplay, &layers, &types);
-    if (!layers.empty()) {
+
+    if (!mReader.takeChangedCompositionTypes(mPrimaryDisplay).empty()) {
         GTEST_SUCCEED() << "Composition change requested, skipping test";
         return;
     }
diff --git a/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientReader.h b/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientReader.h
index 1dc9145..f9e35e9 100644
--- a/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientReader.h
+++ b/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientReader.h
@@ -26,40 +26,16 @@
 #include <inttypes.h>
 #include <string.h>
 
-#include <aidl/android/hardware/graphics/common/BlendMode.h>
 #include <aidl/android/hardware/graphics/composer3/ClientTargetProperty.h>
-#include <aidl/android/hardware/graphics/composer3/Color.h>
 #include <aidl/android/hardware/graphics/composer3/Composition.h>
-#include <aidl/android/hardware/graphics/composer3/FloatColor.h>
-#include <aidl/android/hardware/graphics/composer3/HandleIndex.h>
-#include <aidl/android/hardware/graphics/composer3/IComposer.h>
-#include <aidl/android/hardware/graphics/composer3/IComposerClient.h>
-#include <aidl/android/hardware/graphics/composer3/PerFrameMetadata.h>
-#include <aidl/android/hardware/graphics/composer3/PerFrameMetadataBlob.h>
-
 #include <aidl/android/hardware/graphics/composer3/CommandResultPayload.h>
-#include <aidl/android/hardware/graphics/composer3/DisplayCommand.h>
 
-#include <aidl/android/hardware/graphics/common/ColorTransform.h>
-#include <aidl/android/hardware/graphics/common/FRect.h>
-#include <aidl/android/hardware/graphics/common/Rect.h>
-#include <aidl/android/hardware/graphics/common/Transform.h>
 
 #include <log/log.h>
 #include <sync/sync.h>
 
-#include <aidlcommonsupport/NativeHandle.h>
 
-using aidl::android::hardware::graphics::common::BlendMode;
-using aidl::android::hardware::graphics::common::ColorTransform;
 using aidl::android::hardware::graphics::common::Dataspace;
-using aidl::android::hardware::graphics::common::FRect;
-using aidl::android::hardware::graphics::common::Rect;
-using aidl::android::hardware::graphics::common::Transform;
-
-using namespace aidl::android::hardware::graphics::composer3;
-
-using aidl::android::hardware::common::NativeHandle;
 
 namespace aidl::android::hardware::graphics::composer3 {
 
@@ -69,35 +45,37 @@
 
     // Parse and execute commands from the command queue.  The commands are
     // actually return values from the server and will be saved in ReturnData.
-    void parse(const std::vector<CommandResultPayload>& results) {
+    void parse(std::vector<CommandResultPayload>&& results) {
         resetData();
 
-        for (const auto& result : results) {
+        for (auto& result : results) {
             switch (result.getTag()) {
                 case CommandResultPayload::Tag::error:
-                    parseSetError(result.get<CommandResultPayload::Tag::error>());
+                    parseSetError(std::move(result.get<CommandResultPayload::Tag::error>()));
                     break;
                 case CommandResultPayload::Tag::changedCompositionTypes:
-                    parseSetChangedCompositionTypes(
-                            result.get<CommandResultPayload::Tag::changedCompositionTypes>());
+                    parseSetChangedCompositionTypes(std::move(
+                            result.get<CommandResultPayload::Tag::changedCompositionTypes>()));
                     break;
                 case CommandResultPayload::Tag::displayRequest:
                     parseSetDisplayRequests(
-                            result.get<CommandResultPayload::Tag::displayRequest>());
+                            std::move(result.get<CommandResultPayload::Tag::displayRequest>()));
                     break;
                 case CommandResultPayload::Tag::presentFence:
-                    parseSetPresentFence(result.get<CommandResultPayload::Tag::presentFence>());
+                    parseSetPresentFence(
+                            std::move(result.get<CommandResultPayload::Tag::presentFence>()));
                     break;
                 case CommandResultPayload::Tag::releaseFences:
-                    parseSetReleaseFences(result.get<CommandResultPayload::Tag::releaseFences>());
+                    parseSetReleaseFences(
+                            std::move(result.get<CommandResultPayload::Tag::releaseFences>()));
                     break;
                 case CommandResultPayload::Tag::presentOrValidateResult:
-                    parseSetPresentOrValidateDisplayResult(
-                            result.get<CommandResultPayload::Tag::presentOrValidateResult>());
+                    parseSetPresentOrValidateDisplayResult(std::move(
+                            result.get<CommandResultPayload::Tag::presentOrValidateResult>()));
                     break;
                 case CommandResultPayload::Tag::clientTargetProperty:
-                    parseSetClientTargetProperty(
-                            result.get<CommandResultPayload::Tag::clientTargetProperty>());
+                    parseSetClientTargetProperty(std::move(
+                            result.get<CommandResultPayload::Tag::clientTargetProperty>()));
                     break;
             }
         }
@@ -105,211 +83,140 @@
 
     std::vector<CommandError> takeErrors() { return std::move(mErrors); }
 
-    bool hasChanges(int64_t display, uint32_t* outNumChangedCompositionTypes,
+    void hasChanges(int64_t display, uint32_t* outNumChangedCompositionTypes,
                     uint32_t* outNumLayerRequestMasks) const {
         auto found = mReturnData.find(display);
         if (found == mReturnData.end()) {
             *outNumChangedCompositionTypes = 0;
             *outNumLayerRequestMasks = 0;
-            return false;
+            return;
         }
 
         const ReturnData& data = found->second;
 
-        *outNumChangedCompositionTypes = static_cast<uint32_t>(data.compositionTypes.size());
-        *outNumLayerRequestMasks = static_cast<uint32_t>(data.requestMasks.size());
-
-        return !(data.compositionTypes.empty() && data.requestMasks.empty());
+        *outNumChangedCompositionTypes = static_cast<uint32_t>(data.changedLayers.size());
+        *outNumLayerRequestMasks = static_cast<uint32_t>(data.displayRequests.layerRequests.size());
     }
 
     // Get and clear saved changed composition types.
-    void takeChangedCompositionTypes(int64_t display, std::vector<int64_t>* outLayers,
-                                     std::vector<Composition>* outTypes) {
+    std::vector<ChangedCompositionLayer> takeChangedCompositionTypes(int64_t display) {
         auto found = mReturnData.find(display);
         if (found == mReturnData.end()) {
-            outLayers->clear();
-            outTypes->clear();
-            return;
+            return {};
         }
 
         ReturnData& data = found->second;
-
-        *outLayers = std::move(data.changedLayers);
-        *outTypes = std::move(data.compositionTypes);
+        return std::move(data.changedLayers);
     }
 
     // Get and clear saved display requests.
-    void takeDisplayRequests(int64_t display, uint32_t* outDisplayRequestMask,
-                             std::vector<int64_t>* outLayers,
-                             std::vector<uint32_t>* outLayerRequestMasks) {
+    DisplayRequest takeDisplayRequests(int64_t display) {
         auto found = mReturnData.find(display);
         if (found == mReturnData.end()) {
-            *outDisplayRequestMask = 0;
-            outLayers->clear();
-            outLayerRequestMasks->clear();
-            return;
+            return {};
         }
 
         ReturnData& data = found->second;
-
-        *outDisplayRequestMask = data.displayRequests;
-        *outLayers = std::move(data.requestedLayers);
-        *outLayerRequestMasks = std::move(data.requestMasks);
+        return std::move(data.displayRequests);
     }
 
     // Get and clear saved release fences.
-    void takeReleaseFences(int64_t display, std::vector<int64_t>* outLayers,
-                           std::vector<int>* outReleaseFences) {
+    std::vector<ReleaseFences::Layer> takeReleaseFences(int64_t display) {
         auto found = mReturnData.find(display);
         if (found == mReturnData.end()) {
-            outLayers->clear();
-            outReleaseFences->clear();
-            return;
+            return {};
         }
 
         ReturnData& data = found->second;
-
-        *outLayers = std::move(data.releasedLayers);
-        *outReleaseFences = std::move(data.releaseFences);
+        return std::move(data.releasedLayers);
     }
 
     // Get and clear saved present fence.
-    void takePresentFence(int64_t display, int* outPresentFence) {
+    ndk::ScopedFileDescriptor takePresentFence(int64_t display) {
         auto found = mReturnData.find(display);
         if (found == mReturnData.end()) {
-            *outPresentFence = -1;
-            return;
+            return {};
         }
 
         ReturnData& data = found->second;
-
-        *outPresentFence = data.presentFence;
-        data.presentFence = -1;
+        return std::move(data.presentFence);
     }
 
     // Get what stage succeeded during PresentOrValidate: Present or Validate
-    void takePresentOrValidateStage(int64_t display, uint32_t* state) {
+    std::optional<PresentOrValidate::Result> takePresentOrValidateStage(int64_t display) {
         auto found = mReturnData.find(display);
         if (found == mReturnData.end()) {
-            *state = static_cast<uint32_t>(-1);
-            return;
+            return std::nullopt;
         }
         ReturnData& data = found->second;
-        *state = data.presentOrValidateState;
+        return data.presentOrValidateState;
     }
 
     // Get the client target properties requested by hardware composer.
-    void takeClientTargetProperty(int64_t display, ClientTargetProperty* outClientTargetProperty,
-                                  float* outWhitePointNits) {
+    ClientTargetPropertyWithNits takeClientTargetProperty(int64_t display) {
         auto found = mReturnData.find(display);
 
         // If not found, return the default values.
         if (found == mReturnData.end()) {
-            outClientTargetProperty->pixelFormat = common::PixelFormat::RGBA_8888;
-            outClientTargetProperty->dataspace = Dataspace::UNKNOWN;
-            *outWhitePointNits = -1.f;
-            return;
+            return ClientTargetPropertyWithNits{
+                    .clientTargetProperty = {common::PixelFormat::RGBA_8888, Dataspace::UNKNOWN},
+                    .whitePointNits = -1.f,
+            };
         }
 
         ReturnData& data = found->second;
-        *outClientTargetProperty = data.clientTargetProperty;
-        *outWhitePointNits = data.clientTargetWhitePointNits;
+        return std::move(data.clientTargetProperty);
     }
 
   private:
     void resetData() {
         mErrors.clear();
-
-        for (auto& data : mReturnData) {
-            if (data.second.presentFence >= 0) {
-                close(data.second.presentFence);
-            }
-            for (auto fence : data.second.releaseFences) {
-                if (fence >= 0) {
-                    close(fence);
-                }
-            }
-        }
-
         mReturnData.clear();
     }
 
-    void parseSetError(const CommandError& error) { mErrors.emplace_back(error); }
+    void parseSetError(CommandError&& error) { mErrors.emplace_back(error); }
 
-    void parseSetChangedCompositionTypes(const ChangedCompositionTypes& changedCompositionTypes) {
+    void parseSetChangedCompositionTypes(ChangedCompositionTypes&& changedCompositionTypes) {
         auto& data = mReturnData[changedCompositionTypes.display];
-
-        data.changedLayers.reserve(changedCompositionTypes.layers.size());
-        data.compositionTypes.reserve(changedCompositionTypes.layers.size());
-        for (const auto& layer : changedCompositionTypes.layers) {
-            data.changedLayers.push_back(layer.layer);
-            data.compositionTypes.push_back(layer.composition);
-        }
+        data.changedLayers = std::move(changedCompositionTypes.layers);
     }
 
-    void parseSetDisplayRequests(const DisplayRequest& displayRequest) {
+    void parseSetDisplayRequests(DisplayRequest&& displayRequest) {
         auto& data = mReturnData[displayRequest.display];
-
-        data.displayRequests = displayRequest.mask;
-        data.requestedLayers.reserve(displayRequest.layerRequests.size());
-        data.requestMasks.reserve(displayRequest.layerRequests.size());
-        for (const auto& layerRequest : displayRequest.layerRequests) {
-            data.requestedLayers.push_back(layerRequest.layer);
-            data.requestMasks.push_back(layerRequest.mask);
-        }
+        data.displayRequests = std::move(displayRequest);
     }
 
-    void parseSetPresentFence(const PresentFence& presentFence) {
+    void parseSetPresentFence(PresentFence&& presentFence) {
         auto& data = mReturnData[presentFence.display];
-        if (data.presentFence >= 0) {
-            close(data.presentFence);
-        }
-        data.presentFence = dup(presentFence.fence.get());
+        data.presentFence = std::move(presentFence.fence);
     }
 
-    void parseSetReleaseFences(const ReleaseFences& releaseFences) {
+    void parseSetReleaseFences(ReleaseFences&& releaseFences) {
         auto& data = mReturnData[releaseFences.display];
-        data.releasedLayers.reserve(releaseFences.layers.size());
-        data.releaseFences.reserve(releaseFences.layers.size());
-        for (const auto& layer : releaseFences.layers) {
-            data.releasedLayers.push_back(layer.layer);
-            data.releaseFences.push_back(dup(layer.fence.get()));
-        }
+        data.releasedLayers = std::move(releaseFences.layers);
     }
 
-    void parseSetPresentOrValidateDisplayResult(const PresentOrValidate& presentOrValidate) {
+    void parseSetPresentOrValidateDisplayResult(const PresentOrValidate&& presentOrValidate) {
         auto& data = mReturnData[presentOrValidate.display];
-        data.presentOrValidateState =
-                presentOrValidate.result == PresentOrValidate::Result::Presented ? 1 : 0;
+        data.presentOrValidateState = std::move(presentOrValidate.result);
     }
 
-    void parseSetClientTargetProperty(const ClientTargetPropertyWithNits& clientTargetProperty) {
+    void parseSetClientTargetProperty(const ClientTargetPropertyWithNits&& clientTargetProperty) {
         auto& data = mReturnData[clientTargetProperty.display];
-        data.clientTargetProperty.pixelFormat =
-                clientTargetProperty.clientTargetProperty.pixelFormat;
-        data.clientTargetProperty.dataspace = clientTargetProperty.clientTargetProperty.dataspace;
-        data.clientTargetWhitePointNits = clientTargetProperty.whitePointNits;
+        data.clientTargetProperty = std::move(clientTargetProperty);
     }
 
     struct ReturnData {
-        int32_t displayRequests = 0;
+        DisplayRequest displayRequests;
+        std::vector<ChangedCompositionLayer> changedLayers;
+        ndk::ScopedFileDescriptor presentFence;
+        std::vector<ReleaseFences::Layer> releasedLayers;
+        PresentOrValidate::Result presentOrValidateState;
 
-        std::vector<int64_t> changedLayers;
-        std::vector<Composition> compositionTypes;
-
-        std::vector<int64_t> requestedLayers;
-        std::vector<uint32_t> requestMasks;
-
-        int presentFence = -1;
-
-        std::vector<int64_t> releasedLayers;
-        std::vector<int> releaseFences;
-
-        uint32_t presentOrValidateState;
-
-        ClientTargetProperty clientTargetProperty{common::PixelFormat::RGBA_8888,
-                                                  Dataspace::UNKNOWN};
-        float clientTargetWhitePointNits = -1.f;
+        ClientTargetPropertyWithNits clientTargetProperty = {
+                .clientTargetProperty = {common::PixelFormat::RGBA_8888, Dataspace::UNKNOWN},
+                .whitePointNits = -1.f,
+        };
     };
 
     std::vector<CommandError> mErrors;
diff --git a/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientWriter.h b/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientWriter.h
index 5bab266..16d63e5 100644
--- a/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientWriter.h
+++ b/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientWriter.h
@@ -30,9 +30,6 @@
 #include <aidl/android/hardware/graphics/composer3/Color.h>
 #include <aidl/android/hardware/graphics/composer3/Composition.h>
 #include <aidl/android/hardware/graphics/composer3/FloatColor.h>
-#include <aidl/android/hardware/graphics/composer3/HandleIndex.h>
-#include <aidl/android/hardware/graphics/composer3/IComposer.h>
-#include <aidl/android/hardware/graphics/composer3/IComposerClient.h>
 #include <aidl/android/hardware/graphics/composer3/PerFrameMetadata.h>
 #include <aidl/android/hardware/graphics/composer3/PerFrameMetadataBlob.h>
 
diff --git a/graphics/composer/aidl/include/android/hardware/graphics/composer3/translate-ndk.h b/graphics/composer/aidl/include/android/hardware/graphics/composer3/translate-ndk.h
index 263167e..3a92ae6 100644
--- a/graphics/composer/aidl/include/android/hardware/graphics/composer3/translate-ndk.h
+++ b/graphics/composer/aidl/include/android/hardware/graphics/composer3/translate-ndk.h
@@ -30,7 +30,6 @@
 #include "aidl/android/hardware/graphics/composer3/DisplayConnectionType.h"
 #include "aidl/android/hardware/graphics/composer3/FloatColor.h"
 #include "aidl/android/hardware/graphics/composer3/FormatColorComponent.h"
-#include "aidl/android/hardware/graphics/composer3/HandleIndex.h"
 #include "aidl/android/hardware/graphics/composer3/IComposer.h"
 #include "aidl/android/hardware/graphics/composer3/PerFrameMetadata.h"
 #include "aidl/android/hardware/graphics/composer3/PerFrameMetadataBlob.h"