From 3c02cdcc5700a1a7b576f95767b2c48892967189 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 5 Feb 2019 15:55:15 -0500 Subject: [PATCH 1/3] service/nvflinger: Rename GetVsyncEvent() to FindVsyncEvent() This was missed within #2075. Renames the member function to make it consistent with the rest of the Find* functions. --- src/core/hle/service/nvflinger/nvflinger.cpp | 2 +- src/core/hle/service/nvflinger/nvflinger.h | 2 +- src/core/hle/service/vi/vi.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/hle/service/nvflinger/nvflinger.cpp b/src/core/hle/service/nvflinger/nvflinger.cpp index 8dfc0df03c..e15080b3b6 100644 --- a/src/core/hle/service/nvflinger/nvflinger.cpp +++ b/src/core/hle/service/nvflinger/nvflinger.cpp @@ -78,7 +78,7 @@ u32 NVFlinger::FindBufferQueueId(u64 display_id, u64 layer_id) const { return layer.buffer_queue->GetId(); } -Kernel::SharedPtr NVFlinger::GetVsyncEvent(u64 display_id) { +Kernel::SharedPtr NVFlinger::FindVsyncEvent(u64 display_id) { return FindDisplay(display_id).vsync_event.readable; } diff --git a/src/core/hle/service/nvflinger/nvflinger.h b/src/core/hle/service/nvflinger/nvflinger.h index 83e974ed36..984be68a70 100644 --- a/src/core/hle/service/nvflinger/nvflinger.h +++ b/src/core/hle/service/nvflinger/nvflinger.h @@ -67,7 +67,7 @@ public: u32 FindBufferQueueId(u64 display_id, u64 layer_id) const; /// Gets the vsync event for the specified display. - Kernel::SharedPtr GetVsyncEvent(u64 display_id); + Kernel::SharedPtr FindVsyncEvent(u64 display_id); /// Obtains a buffer queue identified by the ID. std::shared_ptr FindBufferQueue(u32 id) const; diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index fe08c38f2e..724d48df89 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -1088,7 +1088,7 @@ private: LOG_WARNING(Service_VI, "(STUBBED) called. display_id=0x{:016X}", display_id); - const auto vsync_event = nv_flinger->GetVsyncEvent(display_id); + const auto vsync_event = nv_flinger->FindVsyncEvent(display_id); IPC::ResponseBuilder rb{ctx, 2, 1}; rb.Push(RESULT_SUCCESS); From 7320c667dfa8993f4ebec8d6adfbf954d0591784 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 5 Feb 2019 15:57:26 -0500 Subject: [PATCH 2/3] service/nvflinger: Mark FindVsyncEvent() as a const member function This member function doesn't actually modify instance state, so it can be marked as a const member function. --- src/core/hle/service/nvflinger/nvflinger.cpp | 2 +- src/core/hle/service/nvflinger/nvflinger.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/hle/service/nvflinger/nvflinger.cpp b/src/core/hle/service/nvflinger/nvflinger.cpp index e15080b3b6..ce3f2d8c1d 100644 --- a/src/core/hle/service/nvflinger/nvflinger.cpp +++ b/src/core/hle/service/nvflinger/nvflinger.cpp @@ -78,7 +78,7 @@ u32 NVFlinger::FindBufferQueueId(u64 display_id, u64 layer_id) const { return layer.buffer_queue->GetId(); } -Kernel::SharedPtr NVFlinger::FindVsyncEvent(u64 display_id) { +Kernel::SharedPtr NVFlinger::FindVsyncEvent(u64 display_id) const { return FindDisplay(display_id).vsync_event.readable; } diff --git a/src/core/hle/service/nvflinger/nvflinger.h b/src/core/hle/service/nvflinger/nvflinger.h index 984be68a70..61db2c23be 100644 --- a/src/core/hle/service/nvflinger/nvflinger.h +++ b/src/core/hle/service/nvflinger/nvflinger.h @@ -67,7 +67,7 @@ public: u32 FindBufferQueueId(u64 display_id, u64 layer_id) const; /// Gets the vsync event for the specified display. - Kernel::SharedPtr FindVsyncEvent(u64 display_id); + Kernel::SharedPtr FindVsyncEvent(u64 display_id) const; /// Obtains a buffer queue identified by the ID. std::shared_ptr FindBufferQueue(u32 id) const; From ef073ff117752274f374443756652fdda9c44773 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 5 Feb 2019 16:20:04 -0500 Subject: [PATCH 3/3] service/nvflinger,service/vi: Handle failure cases with exposed API Converts many of the Find* functions to return a std::optional as opposed to returning the raw return values directly. This allows removing a few assertions and handles error cases like the service itself does. --- src/core/hle/service/am/am.cpp | 7 +- src/core/hle/service/nvflinger/nvflinger.cpp | 90 ++++++++++++++------ src/core/hle/service/nvflinger/nvflinger.h | 23 +++-- src/core/hle/service/vi/vi.cpp | 60 ++++++++++--- 4 files changed, 133 insertions(+), 47 deletions(-) diff --git a/src/core/hle/service/am/am.cpp b/src/core/hle/service/am/am.cpp index d1cbe0e44d..3f009d2b7c 100644 --- a/src/core/hle/service/am/am.cpp +++ b/src/core/hle/service/am/am.cpp @@ -322,14 +322,15 @@ void ISelfController::SetScreenShotImageOrientation(Kernel::HLERequestContext& c void ISelfController::CreateManagedDisplayLayer(Kernel::HLERequestContext& ctx) { LOG_WARNING(Service_AM, "(STUBBED) called"); + // TODO(Subv): Find out how AM determines the display to use, for now just // create the layer in the Default display. - u64 display_id = nvflinger->OpenDisplay("Default"); - u64 layer_id = nvflinger->CreateLayer(display_id); + const auto display_id = nvflinger->OpenDisplay("Default"); + const auto layer_id = nvflinger->CreateLayer(*display_id); IPC::ResponseBuilder rb{ctx, 4}; rb.Push(RESULT_SUCCESS); - rb.Push(layer_id); + rb.Push(*layer_id); } void ISelfController::SetHandlesRequestToDisplay(Kernel::HLERequestContext& ctx) { diff --git a/src/core/hle/service/nvflinger/nvflinger.cpp b/src/core/hle/service/nvflinger/nvflinger.cpp index ce3f2d8c1d..cde06916d5 100644 --- a/src/core/hle/service/nvflinger/nvflinger.cpp +++ b/src/core/hle/service/nvflinger/nvflinger.cpp @@ -46,7 +46,7 @@ void NVFlinger::SetNVDrvInstance(std::shared_ptr instance) { nvdrv = std::move(instance); } -u64 NVFlinger::OpenDisplay(std::string_view name) { +std::optional NVFlinger::OpenDisplay(std::string_view name) { LOG_DEBUG(Service, "Opening \"{}\" display", name); // TODO(Subv): Currently we only support the Default display. @@ -54,32 +54,48 @@ u64 NVFlinger::OpenDisplay(std::string_view name) { const auto itr = std::find_if(displays.begin(), displays.end(), [&](const Display& display) { return display.name == name; }); - - ASSERT(itr != displays.end()); + if (itr == displays.end()) { + return {}; + } return itr->id; } -u64 NVFlinger::CreateLayer(u64 display_id) { - auto& display = FindDisplay(display_id); +std::optional NVFlinger::CreateLayer(u64 display_id) { + auto* const display = FindDisplay(display_id); - ASSERT_MSG(display.layers.empty(), "Only one layer is supported per display at the moment"); + if (display == nullptr) { + return {}; + } + + ASSERT_MSG(display->layers.empty(), "Only one layer is supported per display at the moment"); const u64 layer_id = next_layer_id++; const u32 buffer_queue_id = next_buffer_queue_id++; auto buffer_queue = std::make_shared(buffer_queue_id, layer_id); - display.layers.emplace_back(layer_id, buffer_queue); + display->layers.emplace_back(layer_id, buffer_queue); buffer_queues.emplace_back(std::move(buffer_queue)); return layer_id; } -u32 NVFlinger::FindBufferQueueId(u64 display_id, u64 layer_id) const { - const auto& layer = FindLayer(display_id, layer_id); - return layer.buffer_queue->GetId(); +std::optional NVFlinger::FindBufferQueueId(u64 display_id, u64 layer_id) const { + const auto* const layer = FindLayer(display_id, layer_id); + + if (layer == nullptr) { + return {}; + } + + return layer->buffer_queue->GetId(); } Kernel::SharedPtr NVFlinger::FindVsyncEvent(u64 display_id) const { - return FindDisplay(display_id).vsync_event.readable; + auto* const display = FindDisplay(display_id); + + if (display == nullptr) { + return nullptr; + } + + return display->vsync_event.readable; } std::shared_ptr NVFlinger::FindBufferQueue(u32 id) const { @@ -90,40 +106,60 @@ std::shared_ptr NVFlinger::FindBufferQueue(u32 id) const { return *itr; } -Display& NVFlinger::FindDisplay(u64 display_id) { +Display* NVFlinger::FindDisplay(u64 display_id) { const auto itr = std::find_if(displays.begin(), displays.end(), [&](const Display& display) { return display.id == display_id; }); - ASSERT(itr != displays.end()); - return *itr; + if (itr == displays.end()) { + return nullptr; + } + + return &*itr; } -const Display& NVFlinger::FindDisplay(u64 display_id) const { +const Display* NVFlinger::FindDisplay(u64 display_id) const { const auto itr = std::find_if(displays.begin(), displays.end(), [&](const Display& display) { return display.id == display_id; }); - ASSERT(itr != displays.end()); - return *itr; + if (itr == displays.end()) { + return nullptr; + } + + return &*itr; } -Layer& NVFlinger::FindLayer(u64 display_id, u64 layer_id) { - auto& display = FindDisplay(display_id); +Layer* NVFlinger::FindLayer(u64 display_id, u64 layer_id) { + auto* const display = FindDisplay(display_id); - const auto itr = std::find_if(display.layers.begin(), display.layers.end(), + if (display == nullptr) { + return nullptr; + } + + const auto itr = std::find_if(display->layers.begin(), display->layers.end(), [&](const Layer& layer) { return layer.id == layer_id; }); - ASSERT(itr != display.layers.end()); - return *itr; + if (itr == display->layers.end()) { + return nullptr; + } + + return &*itr; } -const Layer& NVFlinger::FindLayer(u64 display_id, u64 layer_id) const { - const auto& display = FindDisplay(display_id); +const Layer* NVFlinger::FindLayer(u64 display_id, u64 layer_id) const { + const auto* const display = FindDisplay(display_id); - const auto itr = std::find_if(display.layers.begin(), display.layers.end(), + if (display == nullptr) { + return nullptr; + } + + const auto itr = std::find_if(display->layers.begin(), display->layers.end(), [&](const Layer& layer) { return layer.id == layer_id; }); - ASSERT(itr != display.layers.end()); - return *itr; + if (itr == display->layers.end()) { + return nullptr; + } + + return &*itr; } void NVFlinger::Compose() { diff --git a/src/core/hle/service/nvflinger/nvflinger.h b/src/core/hle/service/nvflinger/nvflinger.h index 61db2c23be..4c55e99f46 100644 --- a/src/core/hle/service/nvflinger/nvflinger.h +++ b/src/core/hle/service/nvflinger/nvflinger.h @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -58,15 +59,23 @@ public: void SetNVDrvInstance(std::shared_ptr instance); /// Opens the specified display and returns the ID. - u64 OpenDisplay(std::string_view name); + /// + /// If an invalid display name is provided, then an empty optional is returned. + std::optional OpenDisplay(std::string_view name); /// Creates a layer on the specified display and returns the layer ID. - u64 CreateLayer(u64 display_id); + /// + /// If an invalid display ID is specified, then an empty optional is returned. + std::optional CreateLayer(u64 display_id); /// Finds the buffer queue ID of the specified layer in the specified display. - u32 FindBufferQueueId(u64 display_id, u64 layer_id) const; + /// + /// If an invalid display ID or layer ID is provided, then an empty optional is returned. + std::optional FindBufferQueueId(u64 display_id, u64 layer_id) const; /// Gets the vsync event for the specified display. + /// + /// If an invalid display ID is provided, then nullptr is returned. Kernel::SharedPtr FindVsyncEvent(u64 display_id) const; /// Obtains a buffer queue identified by the ID. @@ -78,16 +87,16 @@ public: private: /// Finds the display identified by the specified ID. - Display& FindDisplay(u64 display_id); + Display* FindDisplay(u64 display_id); /// Finds the display identified by the specified ID. - const Display& FindDisplay(u64 display_id) const; + const Display* FindDisplay(u64 display_id) const; /// Finds the layer identified by the specified ID in the desired display. - Layer& FindLayer(u64 display_id, u64 layer_id); + Layer* FindLayer(u64 display_id, u64 layer_id); /// Finds the layer identified by the specified ID in the desired display. - const Layer& FindLayer(u64 display_id, u64 layer_id) const; + const Layer* FindLayer(u64 display_id, u64 layer_id) const; std::shared_ptr nvdrv; diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index 724d48df89..a317a2885e 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -34,6 +34,7 @@ namespace Service::VI { constexpr ResultCode ERR_OPERATION_FAILED{ErrorModule::VI, 1}; constexpr ResultCode ERR_UNSUPPORTED{ErrorModule::VI, 6}; +constexpr ResultCode ERR_NOT_FOUND{ErrorModule::VI, 7}; struct DisplayInfo { /// The name of this particular display. @@ -838,11 +839,16 @@ private: "(STUBBED) called. unknown=0x{:08X}, display=0x{:016X}, aruid=0x{:016X}", unknown, display, aruid); - const u64 layer_id = nv_flinger->CreateLayer(display); + const auto layer_id = nv_flinger->CreateLayer(display); + if (!layer_id) { + IPC::ResponseBuilder rb{ctx, 2}; + rb.Push(ERR_NOT_FOUND); + return; + } IPC::ResponseBuilder rb{ctx, 4}; rb.Push(RESULT_SUCCESS); - rb.Push(layer_id); + rb.Push(*layer_id); } void AddToLayerStack(Kernel::HLERequestContext& ctx) { @@ -950,9 +956,16 @@ private: ASSERT_MSG(name == "Default", "Non-default displays aren't supported yet"); + const auto display_id = nv_flinger->OpenDisplay(name); + if (!display_id) { + IPC::ResponseBuilder rb{ctx, 2}; + rb.Push(ERR_NOT_FOUND); + return; + } + IPC::ResponseBuilder rb{ctx, 4}; rb.Push(RESULT_SUCCESS); - rb.Push(nv_flinger->OpenDisplay(name)); + rb.Push(*display_id); } void CloseDisplay(Kernel::HLERequestContext& ctx) { @@ -1043,10 +1056,21 @@ private: LOG_DEBUG(Service_VI, "called. layer_id=0x{:016X}, aruid=0x{:016X}", layer_id, aruid); - const u64 display_id = nv_flinger->OpenDisplay(display_name); - const u32 buffer_queue_id = nv_flinger->FindBufferQueueId(display_id, layer_id); + const auto display_id = nv_flinger->OpenDisplay(display_name); + if (!display_id) { + IPC::ResponseBuilder rb{ctx, 2}; + rb.Push(ERR_NOT_FOUND); + return; + } - NativeWindow native_window{buffer_queue_id}; + const auto buffer_queue_id = nv_flinger->FindBufferQueueId(*display_id, layer_id); + if (!buffer_queue_id) { + IPC::ResponseBuilder rb{ctx, 2}; + rb.Push(ERR_NOT_FOUND); + return; + } + + NativeWindow native_window{*buffer_queue_id}; IPC::ResponseBuilder rb{ctx, 4}; rb.Push(RESULT_SUCCESS); rb.Push(ctx.WriteBuffer(native_window.Serialize())); @@ -1062,13 +1086,24 @@ private: // TODO(Subv): What's the difference between a Stray and a Managed layer? - const u64 layer_id = nv_flinger->CreateLayer(display_id); - const u32 buffer_queue_id = nv_flinger->FindBufferQueueId(display_id, layer_id); + const auto layer_id = nv_flinger->CreateLayer(display_id); + if (!layer_id) { + IPC::ResponseBuilder rb{ctx, 2}; + rb.Push(ERR_NOT_FOUND); + return; + } - NativeWindow native_window{buffer_queue_id}; + const auto buffer_queue_id = nv_flinger->FindBufferQueueId(display_id, *layer_id); + if (!buffer_queue_id) { + IPC::ResponseBuilder rb{ctx, 2}; + rb.Push(ERR_NOT_FOUND); + return; + } + + NativeWindow native_window{*buffer_queue_id}; IPC::ResponseBuilder rb{ctx, 6}; rb.Push(RESULT_SUCCESS); - rb.Push(layer_id); + rb.Push(*layer_id); rb.Push(ctx.WriteBuffer(native_window.Serialize())); } @@ -1089,6 +1124,11 @@ private: LOG_WARNING(Service_VI, "(STUBBED) called. display_id=0x{:016X}", display_id); const auto vsync_event = nv_flinger->FindVsyncEvent(display_id); + if (!vsync_event) { + IPC::ResponseBuilder rb{ctx, 2}; + rb.Push(ERR_NOT_FOUND); + return; + } IPC::ResponseBuilder rb{ctx, 2, 1}; rb.Push(RESULT_SUCCESS);