From 40aa1ea9f983130c407fda20da14c5399d6bb12b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 4 Jan 2019 19:31:14 -0500 Subject: [PATCH 1/3] service/vi: Unstub IApplicationDisplayService' SetLayerScalingMode() This appears to only check if the scaling mode can actually be handled, rather than actually setting the scaling mode for the layer. This implements the same error handling performed on the passed in values. --- src/core/hle/service/vi/vi.cpp | 59 ++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index 8528925eca..266909ba40 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -32,6 +32,9 @@ namespace Service::VI { +constexpr ResultCode ERR_OPERATION_FAILED{ErrorModule::VI, 1}; +constexpr ResultCode ERR_UNSUPPORTED{ErrorModule::VI, 6}; + struct DisplayInfo { /// The name of this particular display. char display_name[0x40]{"Default"}; @@ -874,6 +877,24 @@ public: ~IApplicationDisplayService() = default; private: + enum class ConvertedScaleMode : u64 { + None = 0, // VI seems to name this as "Unknown" but lots of games pass it, assume it's no + // scaling/default + Freeze = 1, + ScaleToWindow = 2, + Crop = 3, + NoCrop = 4, + }; + + // This struct is different, currently it's 1:1 but this might change in the future. + enum class NintendoScaleMode : u32 { + None = 0, + Freeze = 1, + ScaleToWindow = 2, + Crop = 3, + NoCrop = 4, + }; + void GetRelayService(Kernel::HLERequestContext& ctx) { LOG_WARNING(Service_VI, "(STUBBED) called"); @@ -978,13 +999,27 @@ private: void SetLayerScalingMode(Kernel::HLERequestContext& ctx) { IPC::RequestParser rp{ctx}; - const u32 scaling_mode = rp.Pop(); + const auto scaling_mode = rp.PopEnum(); const u64 unknown = rp.Pop(); - LOG_WARNING(Service_VI, "(STUBBED) called. scaling_mode=0x{:08X}, unknown=0x{:016X}", - scaling_mode, unknown); + LOG_DEBUG(Service_VI, "called. scaling_mode=0x{:08X}, unknown=0x{:016X}", + static_cast(scaling_mode), unknown); IPC::ResponseBuilder rb{ctx, 2}; + + if (scaling_mode > NintendoScaleMode::NoCrop) { + LOG_ERROR(Service_VI, "Invalid scaling mode provided."); + rb.Push(ERR_OPERATION_FAILED); + return; + } + + if (scaling_mode != NintendoScaleMode::ScaleToWindow && + scaling_mode != NintendoScaleMode::NoCrop) { + LOG_ERROR(Service_VI, "Unsupported scaling mode supplied."); + rb.Push(ERR_UNSUPPORTED); + return; + } + rb.Push(RESULT_SUCCESS); } @@ -1065,24 +1100,6 @@ private: rb.PushCopyObjects(vsync_event); } - enum class ConvertedScaleMode : u64 { - None = 0, // VI seems to name this as "Unknown" but lots of games pass it, assume it's no - // scaling/default - Freeze = 1, - ScaleToWindow = 2, - Crop = 3, - NoCrop = 4, - }; - - // This struct is different, currently it's 1:1 but this might change in the future. - enum class NintendoScaleMode : u32 { - None = 0, - Freeze = 1, - ScaleToWindow = 2, - Crop = 3, - NoCrop = 4, - }; - void ConvertScalingMode(Kernel::HLERequestContext& ctx) { IPC::RequestParser rp{ctx}; auto mode = rp.PopEnum(); From 56e51da1d90010ee5cc9f3c278d278d42f438a88 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 4 Jan 2019 19:55:01 -0500 Subject: [PATCH 2/3] service/vi: Factor out scaling mode conversions from the IPC function itself Avoids entangling the IPC buffer appending with the actual operation of converting the scaling values over. This also inserts the proper error handling for invalid scaling values. --- src/core/hle/service/vi/vi.cpp | 38 +++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index 266909ba40..b0ae074c96 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -1003,7 +1003,7 @@ private: const u64 unknown = rp.Pop(); LOG_DEBUG(Service_VI, "called. scaling_mode=0x{:08X}, unknown=0x{:016X}", - static_cast(scaling_mode), unknown); + static_cast(scaling_mode), unknown); IPC::ResponseBuilder rb{ctx, 2}; @@ -1102,31 +1102,35 @@ private: void ConvertScalingMode(Kernel::HLERequestContext& ctx) { IPC::RequestParser rp{ctx}; - auto mode = rp.PopEnum(); + const auto mode = rp.PopEnum(); LOG_DEBUG(Service_VI, "called mode={}", static_cast(mode)); - IPC::ResponseBuilder rb{ctx, 4}; - rb.Push(RESULT_SUCCESS); + const auto converted_mode = ConvertScalingModeImpl(mode); + + if (converted_mode.Succeeded()) { + IPC::ResponseBuilder rb{ctx, 4}; + rb.Push(RESULT_SUCCESS); + rb.PushEnum(*converted_mode); + } else { + IPC::ResponseBuilder rb{ctx, 2}; + rb.Push(converted_mode.Code()); + } + } + + static ResultVal ConvertScalingModeImpl(NintendoScaleMode mode) { switch (mode) { case NintendoScaleMode::None: - rb.PushEnum(ConvertedScaleMode::None); - break; + return MakeResult(ConvertedScaleMode::None); case NintendoScaleMode::Freeze: - rb.PushEnum(ConvertedScaleMode::Freeze); - break; + return MakeResult(ConvertedScaleMode::Freeze); case NintendoScaleMode::ScaleToWindow: - rb.PushEnum(ConvertedScaleMode::ScaleToWindow); - break; + return MakeResult(ConvertedScaleMode::ScaleToWindow); case NintendoScaleMode::Crop: - rb.PushEnum(ConvertedScaleMode::Crop); - break; + return MakeResult(ConvertedScaleMode::Crop); case NintendoScaleMode::NoCrop: - rb.PushEnum(ConvertedScaleMode::NoCrop); - break; + return MakeResult(ConvertedScaleMode::NoCrop); default: - UNIMPLEMENTED_MSG("Unknown scaling mode {}", static_cast(mode)); - rb.PushEnum(ConvertedScaleMode::None); - break; + return ERR_OPERATION_FAILED; } } From 9e8737b5355c63f55885db078a385cd3e0eb2628 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 4 Jan 2019 20:32:29 -0500 Subject: [PATCH 3/3] service/vi: Correct scaling mode conversions These values are not equivalent, based off RE. The internal value is put into a lookup table with the following values: [3, 0, 1, 2, 4] So the values absolutely do not map 1:1 like the comment was indicating. --- src/core/hle/service/vi/vi.cpp | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index b0ae074c96..a295a288b5 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -878,21 +878,19 @@ public: private: enum class ConvertedScaleMode : u64 { - None = 0, // VI seems to name this as "Unknown" but lots of games pass it, assume it's no - // scaling/default - Freeze = 1, - ScaleToWindow = 2, - Crop = 3, - NoCrop = 4, + Freeze = 0, + ScaleToWindow = 1, + ScaleAndCrop = 2, + None = 3, + PreserveAspectRatio = 4, }; - // This struct is different, currently it's 1:1 but this might change in the future. enum class NintendoScaleMode : u32 { None = 0, Freeze = 1, ScaleToWindow = 2, - Crop = 3, - NoCrop = 4, + ScaleAndCrop = 3, + PreserveAspectRatio = 4, }; void GetRelayService(Kernel::HLERequestContext& ctx) { @@ -1007,14 +1005,14 @@ private: IPC::ResponseBuilder rb{ctx, 2}; - if (scaling_mode > NintendoScaleMode::NoCrop) { + if (scaling_mode > NintendoScaleMode::PreserveAspectRatio) { LOG_ERROR(Service_VI, "Invalid scaling mode provided."); rb.Push(ERR_OPERATION_FAILED); return; } if (scaling_mode != NintendoScaleMode::ScaleToWindow && - scaling_mode != NintendoScaleMode::NoCrop) { + scaling_mode != NintendoScaleMode::PreserveAspectRatio) { LOG_ERROR(Service_VI, "Unsupported scaling mode supplied."); rb.Push(ERR_UNSUPPORTED); return; @@ -1125,10 +1123,10 @@ private: return MakeResult(ConvertedScaleMode::Freeze); case NintendoScaleMode::ScaleToWindow: return MakeResult(ConvertedScaleMode::ScaleToWindow); - case NintendoScaleMode::Crop: - return MakeResult(ConvertedScaleMode::Crop); - case NintendoScaleMode::NoCrop: - return MakeResult(ConvertedScaleMode::NoCrop); + case NintendoScaleMode::ScaleAndCrop: + return MakeResult(ConvertedScaleMode::ScaleAndCrop); + case NintendoScaleMode::PreserveAspectRatio: + return MakeResult(ConvertedScaleMode::PreserveAspectRatio); default: return ERR_OPERATION_FAILED; }