From adc307961357243a67ed4400d18ec9979a0c55db Mon Sep 17 00:00:00 2001 From: Liam Date: Sun, 10 Dec 2023 12:32:44 -0500 Subject: [PATCH] vi: fix confusion between closing and destroying layers --- .../nvnflinger/fb_share_buffer_manager.cpp | 3 +- .../hle/service/nvnflinger/nvnflinger.cpp | 42 ++++++++++--------- src/core/hle/service/nvnflinger/nvnflinger.h | 11 ++--- .../hle/service/vi/display/vi_display.cpp | 25 +++++++++-- src/core/hle/service/vi/display/vi_display.h | 15 +++---- src/core/hle/service/vi/layer/vi_layer.cpp | 4 +- src/core/hle/service/vi/layer/vi_layer.h | 13 ++++++ src/core/hle/service/vi/vi.cpp | 3 ++ 8 files changed, 74 insertions(+), 42 deletions(-) diff --git a/src/core/hle/service/nvnflinger/fb_share_buffer_manager.cpp b/src/core/hle/service/nvnflinger/fb_share_buffer_manager.cpp index 75bf31e32..2fef6cc1a 100644 --- a/src/core/hle/service/nvnflinger/fb_share_buffer_manager.cpp +++ b/src/core/hle/service/nvnflinger/fb_share_buffer_manager.cpp @@ -204,8 +204,9 @@ Result FbShareBufferManager::Initialize(u64* out_buffer_id, u64* out_layer_id, u // Record the display id. m_display_id = display_id; - // Create a layer for the display. + // Create and open a layer for the display. m_layer_id = m_flinger.CreateLayer(m_display_id).value(); + m_flinger.OpenLayer(m_layer_id); // Set up the buffer. m_buffer_id = m_next_buffer_id++; diff --git a/src/core/hle/service/nvnflinger/nvnflinger.cpp b/src/core/hle/service/nvnflinger/nvnflinger.cpp index 0745434c5..6352b09a9 100644 --- a/src/core/hle/service/nvnflinger/nvnflinger.cpp +++ b/src/core/hle/service/nvnflinger/nvnflinger.cpp @@ -176,17 +176,37 @@ void Nvnflinger::CreateLayerAtId(VI::Display& display, u64 layer_id) { display.CreateLayer(layer_id, buffer_id, nvdrv->container); } +void Nvnflinger::OpenLayer(u64 layer_id) { + const auto lock_guard = Lock(); + + for (auto& display : displays) { + if (auto* layer = display.FindLayer(layer_id); layer) { + layer->Open(); + } + } +} + void Nvnflinger::CloseLayer(u64 layer_id) { const auto lock_guard = Lock(); for (auto& display : displays) { - display.CloseLayer(layer_id); + if (auto* layer = display.FindLayer(layer_id); layer) { + layer->Close(); + } + } +} + +void Nvnflinger::DestroyLayer(u64 layer_id) { + const auto lock_guard = Lock(); + + for (auto& display : displays) { + display.DestroyLayer(layer_id); } } std::optional Nvnflinger::FindBufferQueueId(u64 display_id, u64 layer_id) { const auto lock_guard = Lock(); - const auto* const layer = FindOrCreateLayer(display_id, layer_id); + const auto* const layer = FindLayer(display_id, layer_id); if (layer == nullptr) { return std::nullopt; @@ -240,24 +260,6 @@ VI::Layer* Nvnflinger::FindLayer(u64 display_id, u64 layer_id) { return display->FindLayer(layer_id); } -VI::Layer* Nvnflinger::FindOrCreateLayer(u64 display_id, u64 layer_id) { - auto* const display = FindDisplay(display_id); - - if (display == nullptr) { - return nullptr; - } - - auto* layer = display->FindLayer(layer_id); - - if (layer == nullptr) { - LOG_DEBUG(Service_Nvnflinger, "Layer at id {} not found. Trying to create it.", layer_id); - CreateLayerAtId(*display, layer_id); - return display->FindLayer(layer_id); - } - - return layer; -} - void Nvnflinger::Compose() { for (auto& display : displays) { // Trigger vsync for this display at the end of drawing diff --git a/src/core/hle/service/nvnflinger/nvnflinger.h b/src/core/hle/service/nvnflinger/nvnflinger.h index f5d73acdb..871285764 100644 --- a/src/core/hle/service/nvnflinger/nvnflinger.h +++ b/src/core/hle/service/nvnflinger/nvnflinger.h @@ -73,9 +73,15 @@ public: /// If an invalid display ID is specified, then an empty optional is returned. [[nodiscard]] std::optional CreateLayer(u64 display_id); + /// Opens a layer on all displays for the given layer ID. + void OpenLayer(u64 layer_id); + /// Closes a layer on all displays for the given layer ID. void CloseLayer(u64 layer_id); + /// Destroys the given layer ID. + void DestroyLayer(u64 layer_id); + /// Finds the buffer queue ID of the specified layer in the specified display. /// /// If an invalid display ID or layer ID is provided, then an empty optional is returned. @@ -117,11 +123,6 @@ private: /// Finds the layer identified by the specified ID in the desired display. [[nodiscard]] VI::Layer* FindLayer(u64 display_id, u64 layer_id); - /// Finds the layer identified by the specified ID in the desired display, - /// or creates the layer if it is not found. - /// To be used when the system expects the specified ID to already exist. - [[nodiscard]] VI::Layer* FindOrCreateLayer(u64 display_id, u64 layer_id); - /// Creates a layer with the specified layer ID in the desired display. void CreateLayerAtId(VI::Display& display, 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 d30f49877..71ce9be50 100644 --- a/src/core/hle/service/vi/display/vi_display.cpp +++ b/src/core/hle/service/vi/display/vi_display.cpp @@ -51,11 +51,24 @@ Display::~Display() { } Layer& Display::GetLayer(std::size_t index) { - return *layers.at(index); + size_t i = 0; + for (auto& layer : layers) { + if (!layer->IsOpen()) { + continue; + } + + if (i == index) { + return *layer; + } + + i++; + } + + UNREACHABLE(); } -const Layer& Display::GetLayer(std::size_t index) const { - return *layers.at(index); +size_t Display::GetNumLayers() const { + return std::ranges::count_if(layers, [](auto& l) { return l->IsOpen(); }); } Result Display::GetVSyncEvent(Kernel::KReadableEvent** out_vsync_event) { @@ -92,7 +105,11 @@ void Display::CreateLayer(u64 layer_id, u32 binder_id, hos_binder_driver_server.RegisterProducer(std::move(producer)); } -void Display::CloseLayer(u64 layer_id) { +void Display::DestroyLayer(u64 layer_id) { + if (auto* layer = this->FindLayer(layer_id); layer != nullptr) { + layer->GetConsumer().Abandon(); + } + std::erase_if(layers, [layer_id](const auto& layer) { return layer->GetLayerId() == layer_id; }); } diff --git a/src/core/hle/service/vi/display/vi_display.h b/src/core/hle/service/vi/display/vi_display.h index 101cbce20..1d9360b96 100644 --- a/src/core/hle/service/vi/display/vi_display.h +++ b/src/core/hle/service/vi/display/vi_display.h @@ -66,18 +66,13 @@ public: /// Whether or not this display has any layers added to it. bool HasLayers() const { - return !layers.empty(); + return GetNumLayers() > 0; } /// Gets a layer for this display based off an index. Layer& GetLayer(std::size_t index); - /// Gets a layer for this display based off an index. - const Layer& GetLayer(std::size_t index) const; - - std::size_t GetNumLayers() const { - return layers.size(); - } + std::size_t GetNumLayers() const; /** * Gets the internal vsync event. @@ -100,11 +95,11 @@ public: /// void CreateLayer(u64 layer_id, u32 binder_id, Service::Nvidia::NvCore::Container& core); - /// Closes and removes a layer from this display with the given ID. + /// Removes a layer from this display with the given ID. /// - /// @param layer_id The ID assigned to the layer to close. + /// @param layer_id The ID assigned to the layer to destroy. /// - void CloseLayer(u64 layer_id); + void DestroyLayer(u64 layer_id); /// Resets the display for a new connection. void Reset() { diff --git a/src/core/hle/service/vi/layer/vi_layer.cpp b/src/core/hle/service/vi/layer/vi_layer.cpp index 9ae2e0e44..04e52a23b 100644 --- a/src/core/hle/service/vi/layer/vi_layer.cpp +++ b/src/core/hle/service/vi/layer/vi_layer.cpp @@ -8,8 +8,8 @@ namespace Service::VI { Layer::Layer(u64 layer_id_, u32 binder_id_, android::BufferQueueCore& core_, android::BufferQueueProducer& binder_, std::shared_ptr&& consumer_) - : layer_id{layer_id_}, binder_id{binder_id_}, core{core_}, binder{binder_}, consumer{std::move( - consumer_)} {} + : layer_id{layer_id_}, binder_id{binder_id_}, core{core_}, binder{binder_}, + consumer{std::move(consumer_)}, open{false} {} Layer::~Layer() = default; diff --git a/src/core/hle/service/vi/layer/vi_layer.h b/src/core/hle/service/vi/layer/vi_layer.h index 8cf1b5275..295005e23 100644 --- a/src/core/hle/service/vi/layer/vi_layer.h +++ b/src/core/hle/service/vi/layer/vi_layer.h @@ -71,12 +71,25 @@ public: return core; } + bool IsOpen() const { + return open; + } + + void Close() { + open = false; + } + + void Open() { + open = true; + } + private: const u64 layer_id; const u32 binder_id; android::BufferQueueCore& core; android::BufferQueueProducer& binder; std::shared_ptr consumer; + bool open; }; } // namespace Service::VI diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index b1bfb9898..9ab8788e3 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -719,6 +719,8 @@ private: return; } + nv_flinger.OpenLayer(layer_id); + android::OutputParcel parcel; parcel.WriteInterface(NativeWindow{*buffer_queue_id}); @@ -783,6 +785,7 @@ private: const u64 layer_id = rp.Pop(); LOG_WARNING(Service_VI, "(STUBBED) called. layer_id=0x{:016X}", layer_id); + nv_flinger.DestroyLayer(layer_id); IPC::ResponseBuilder rb{ctx, 2}; rb.Push(ResultSuccess);