Merge pull request #12701 from liamwhite/flinger-layer-issues

 vi: check layer state before opening or closing

6 files changed, 42 insertions, 15 deletions

diff --git a/src/core/hle/service/nvnflinger/nvnflinger.cpp b/src/core/hle/service/nvnflinger/nvnflinger.cpp
index 0469110e8..af6591370 100644
--- a/src/core/hle/service/nvnflinger/nvnflinger.cpp
+++ b/src/core/hle/service/nvnflinger/nvnflinger.cpp
@@ -112,9 +112,7 @@ void Nvnflinger::ShutdownLayers() {
     {
         const auto lock_guard = Lock();
         for (auto& display : displays) {
-            for (size_t layer = 0; layer < display.GetNumLayers(); ++layer) {
-                display.GetLayer(layer).GetConsumer().Abandon();
-            }
+            display.Abandon();
         }
 
         is_abandoned = true;
@@ -176,24 +174,28 @@ void Nvnflinger::CreateLayerAtId(VI::Display& display, u64 layer_id) {
     display.CreateLayer(layer_id, buffer_id, nvdrv->container);
 }
 
-void Nvnflinger::OpenLayer(u64 layer_id) {
+bool Nvnflinger::OpenLayer(u64 layer_id) {
     const auto lock_guard = Lock();
 
     for (auto& display : displays) {
         if (auto* layer = display.FindLayer(layer_id); layer) {
-            layer->Open();
+            return layer->Open();
         }
     }
+
+    return false;
 }
 
-void Nvnflinger::CloseLayer(u64 layer_id) {
+bool Nvnflinger::CloseLayer(u64 layer_id) {
     const auto lock_guard = Lock();
 
     for (auto& display : displays) {
         if (auto* layer = display.FindLayer(layer_id); layer) {
-            layer->Close();
+            return layer->Close();
         }
     }
+
+    return false;
 }
 
 void Nvnflinger::DestroyLayer(u64 layer_id) {
diff --git a/src/core/hle/service/nvnflinger/nvnflinger.h b/src/core/hle/service/nvnflinger/nvnflinger.h
index 871285764..a60e0ae6b 100644
--- a/src/core/hle/service/nvnflinger/nvnflinger.h
+++ b/src/core/hle/service/nvnflinger/nvnflinger.h
@@ -74,10 +74,10 @@ public:
     [[nodiscard]] std::optional<u64> CreateLayer(u64 display_id);
 
     /// Opens a layer on all displays for the given layer ID.
-    void OpenLayer(u64 layer_id);
+    bool OpenLayer(u64 layer_id);
 
     /// Closes a layer on all displays for the given layer ID.
-    void CloseLayer(u64 layer_id);
+    bool CloseLayer(u64 layer_id);
 
     /// Destroys the given layer ID.
     void DestroyLayer(u64 layer_id);
diff --git a/src/core/hle/service/vi/display/vi_display.cpp b/src/core/hle/service/vi/display/vi_display.cpp
index e2d9cd98a..725311c53 100644
--- a/src/core/hle/service/vi/display/vi_display.cpp
+++ b/src/core/hle/service/vi/display/vi_display.cpp
@@ -91,6 +91,10 @@ void Display::CreateLayer(u64 layer_id, u32 binder_id,
     layers.emplace_back(std::make_unique<Layer>(layer_id, binder_id, *core, *producer,
                                                 std::move(buffer_item_consumer)));
 
+    if (is_abandoned) {
+        this->FindLayer(layer_id)->GetConsumer().Abandon();
+    }
+
     hos_binder_driver_server.RegisterProducer(std::move(producer));
 }
 
@@ -103,6 +107,13 @@ void Display::DestroyLayer(u64 layer_id) {
                   [layer_id](const auto& layer) { return layer->GetLayerId() == layer_id; });
 }
 
+void Display::Abandon() {
+    for (auto& layer : layers) {
+        layer->GetConsumer().Abandon();
+    }
+    is_abandoned = true;
+}
+
 Layer* Display::FindLayer(u64 layer_id) {
     const auto itr =
         std::find_if(layers.begin(), layers.end(), [layer_id](const std::unique_ptr<Layer>& layer) {
diff --git a/src/core/hle/service/vi/display/vi_display.h b/src/core/hle/service/vi/display/vi_display.h
index 7e68ee79b..8eb8a5155 100644
--- a/src/core/hle/service/vi/display/vi_display.h
+++ b/src/core/hle/service/vi/display/vi_display.h
@@ -98,6 +98,8 @@ public:
         layers.clear();
     }
 
+    void Abandon();
+
     /// Attempts to find a layer with the given ID.
     ///
     /// @param layer_id The layer ID.
@@ -124,6 +126,7 @@ private:
 
     std::vector<std::unique_ptr<Layer>> layers;
     Kernel::KEvent* vsync_event{};
+    bool is_abandoned{};
 };
 
 } // namespace Service::VI
diff --git a/src/core/hle/service/vi/layer/vi_layer.h b/src/core/hle/service/vi/layer/vi_layer.h
index 295005e23..f95e2dc71 100644
--- a/src/core/hle/service/vi/layer/vi_layer.h
+++ b/src/core/hle/service/vi/layer/vi_layer.h
@@ -4,6 +4,7 @@
 #pragma once
 
 #include <memory>
+#include <utility>
 
 #include "common/common_types.h"
 
@@ -75,12 +76,12 @@ public:
         return open;
     }
 
-    void Close() {
-        open = false;
+    bool Close() {
+        return std::exchange(open, false);
     }
 
-    void Open() {
-        open = true;
+    bool Open() {
+        return !std::exchange(open, true);
     }
 
 private:
diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp
index 39d5be90d..bfcc27ddc 100644
--- a/src/core/hle/service/vi/vi.cpp
+++ b/src/core/hle/service/vi/vi.cpp
@@ -719,7 +719,12 @@ private:
             return;
         }
 
-        nvnflinger.OpenLayer(layer_id);
+        if (!nvnflinger.OpenLayer(layer_id)) {
+            LOG_WARNING(Service_VI, "Tried to open layer which was already open");
+            IPC::ResponseBuilder rb{ctx, 2};
+            rb.Push(ResultOperationFailed);
+            return;
+        }
 
         android::OutputParcel parcel;
         parcel.WriteInterface(NativeWindow{*buffer_queue_id});
@@ -737,7 +742,12 @@ private:
 
         LOG_DEBUG(Service_VI, "called. layer_id=0x{:016X}", layer_id);
 
-        nvnflinger.CloseLayer(layer_id);
+        if (!nvnflinger.CloseLayer(layer_id)) {
+            LOG_WARNING(Service_VI, "Tried to close layer which was not open");
+            IPC::ResponseBuilder rb{ctx, 2};
+            rb.Push(ResultOperationFailed);
+            return;
+        }
 
         IPC::ResponseBuilder rb{ctx, 2};
         rb.Push(ResultSuccess);