From 4129459495072e54d3e7f4c0a2628b7080aaab6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 29 Nov 2017 21:31:42 +0100 Subject: [PATCH 1/7] Avoid an assert in ARM64 emitter, seen in Google Play crash logs --- Common/Arm64Emitter.cpp | 6 ++++-- GPU/Common/VertexDecoderArm64.cpp | 7 ++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Common/Arm64Emitter.cpp b/Common/Arm64Emitter.cpp index 17771d19cc..b3ef8540db 100644 --- a/Common/Arm64Emitter.cpp +++ b/Common/Arm64Emitter.cpp @@ -733,9 +733,11 @@ void ARM64XEmitter::EncodeLoadStoreIndexedInst(u32 op, ARM64Reg Rt, ARM64Reg Rn, else if (size == 16) shift = 1; - _assert_msg_(DYNA_REC, ((imm >> shift) << shift) == imm, "%s(INDEX_UNSIGNED): offset must be aligned %d", __FUNCTION__, imm); + if (shift) { + _assert_msg_(DYNA_REC, ((imm >> shift) << shift) == imm, "%s(INDEX_UNSIGNED): offset must be aligned %d", __FUNCTION__, imm); + imm >>= shift; + } - imm >>= shift; _assert_msg_(DYNA_REC, imm >= 0, "%s(INDEX_UNSIGNED): offset must be positive %d", __FUNCTION__, imm); _assert_msg_(DYNA_REC, !(imm & ~0xFFF), "%s(INDEX_UNSIGNED): offset too large %d", __FUNCTION__, imm); diff --git a/GPU/Common/VertexDecoderArm64.cpp b/GPU/Common/VertexDecoderArm64.cpp index 37e3cb9bdd..0e31dd80c2 100644 --- a/GPU/Common/VertexDecoderArm64.cpp +++ b/GPU/Common/VertexDecoderArm64.cpp @@ -698,7 +698,12 @@ void VertexDecoderJitCache::Jit_PosS16Through() { } void VertexDecoderJitCache::Jit_NormalS8() { - LDRH(INDEX_UNSIGNED, tempReg1, srcReg, dec_->nrmoff); + // nrmoff can be odd in case of byte-only vertices! odd unsigned offsets are not allowed for LDRH. + // Switching to LDRB. + // Only seen this in a crash log. + LDRB(INDEX_UNSIGNED, tempReg1, srcReg, dec_->nrmoff); + LDRB(INDEX_UNSIGNED, tempReg3, srcReg, dec_->nrmoff + 1); + ORR(tempReg1, tempReg1, tempReg3, ArithOption(tempReg3, ST_LSL, 8)); LDRB(INDEX_UNSIGNED, tempReg3, srcReg, dec_->nrmoff + 2); ORR(tempReg1, tempReg1, tempReg3, ArithOption(tempReg3, ST_LSL, 16)); STR(INDEX_UNSIGNED, tempReg1, dstReg, dec_->decFmt.nrmoff); From 168d89284c146156b694cb73f8678387fda115b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 29 Nov 2017 22:14:01 +0100 Subject: [PATCH 2/7] Vulkan transitions after render: Add a missing case that seems like it could be common? --- ext/native/thin3d/VulkanQueueRunner.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index d34cca692d..628d95150b 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -467,7 +467,8 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c vkCmdEndRenderPass(cmd); // Transition the framebuffer if requested. - if (fb && step.render.finalColorLayout != VK_IMAGE_LAYOUT_UNDEFINED) { + // Don't need to transition it if VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL. + if (fb && step.render.finalColorLayout != VK_IMAGE_LAYOUT_UNDEFINED && step.render.finalColorLayout != VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL) { VkImageMemoryBarrier barrier{}; barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; barrier.oldLayout = fb->color.layout; @@ -490,6 +491,9 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: barrier.dstAccessMask = VK_ACCESS_SHADER_READ_BIT; break; + case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: + barrier.dstAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT; + break; default: Crash(); } From b52285287d67cc0092ccedde8b2343dbb5da3e38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 30 Nov 2017 00:40:55 +0100 Subject: [PATCH 3/7] Vulkan: Avoid duplicate image pre-transitions (actually eliminated later anyway, but a small saving) --- ext/native/thin3d/VulkanRenderManager.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index 23f48aba9d..9f310b2b4e 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -687,6 +687,12 @@ VkImageView VulkanRenderManager::BindFramebufferAsTexture(VKRFramebuffer *fb, in } } + if (!curRenderStep_->preTransitions.empty() && + curRenderStep_->preTransitions.back().fb == fb && + curRenderStep_->preTransitions.back().targetLayout == VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL) { + // We're done. + return fb->color.imageView; + } curRenderStep_->preTransitions.push_back({ fb, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL }); return fb->color.imageView; } From b0c42f708199a3b02837fcc015ffb456fec6865f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 30 Nov 2017 00:59:54 +0100 Subject: [PATCH 4/7] Fix a java exception around the GPS stuff --- android/src/org/ppsspp/ppsspp/LocationHelper.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/android/src/org/ppsspp/ppsspp/LocationHelper.java b/android/src/org/ppsspp/ppsspp/LocationHelper.java index 9a7b231c8f..c6f4797d61 100644 --- a/android/src/org/ppsspp/ppsspp/LocationHelper.java +++ b/android/src/org/ppsspp/ppsspp/LocationHelper.java @@ -20,17 +20,18 @@ class LocationHelper implements LocationListener { void startLocationUpdates() { Log.d(TAG, "startLocationUpdates"); if (!mLocationEnable) { - boolean isGPSEnabled = mLocationManager.isProviderEnabled(LocationManager.GPS_PROVIDER); - boolean isNetworkEnabled = mLocationManager.isProviderEnabled(LocationManager.NETWORK_PROVIDER); - + boolean isGPSEnabled = false; + boolean isNetworkEnabled = false; try { + isGPSEnabled = mLocationManager.isProviderEnabled(LocationManager.GPS_PROVIDER); + isNetworkEnabled = mLocationManager.isProviderEnabled(LocationManager.NETWORK_PROVIDER); mLocationManager.requestLocationUpdates(LocationManager.GPS_PROVIDER, 1000, 0, this); mLocationManager.requestLocationUpdates(LocationManager.NETWORK_PROVIDER, 1000, 0, this); mLocationEnable = true; } catch (SecurityException e) { Log.e(TAG, "Cannot start location updates: " + e.toString()); } - if(!isGPSEnabled && !isNetworkEnabled) { + if (!isGPSEnabled && !isNetworkEnabled) { Log.i(TAG, "No location provider found"); // TODO: notify user } From 0207739d76b475ae295af49eca42fe351cd387c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 30 Nov 2017 01:07:03 +0100 Subject: [PATCH 5/7] Can't call functions through known-nil pointers, even if they don't touch local data - LLVM's optimizer might have done something stupid. --- Core/MIPS/ARM/ArmJit.cpp | 15 --------------- Core/MIPS/ARM/ArmJit.h | 1 - Core/MIPS/ARM64/Arm64Jit.cpp | 14 -------------- Core/MIPS/ARM64/Arm64Jit.h | 1 - Core/MIPS/IR/IRJit.cpp | 14 -------------- Core/MIPS/IR/IRJit.h | 1 - Core/MIPS/JitCommon/JitCommon.cpp | 16 ++++++++++++++++ Core/MIPS/JitCommon/JitCommon.h | 3 ++- Core/MIPS/MIPS.cpp | 2 +- Core/MIPS/MIPS/MipsJit.cpp | 15 --------------- Core/MIPS/MIPS/MipsJit.h | 1 - Core/MIPS/x86/Jit.cpp | 15 --------------- Core/MIPS/x86/Jit.h | 1 - 13 files changed, 19 insertions(+), 80 deletions(-) diff --git a/Core/MIPS/ARM/ArmJit.cpp b/Core/MIPS/ARM/ArmJit.cpp index 507238f459..0a7c3ad538 100644 --- a/Core/MIPS/ARM/ArmJit.cpp +++ b/Core/MIPS/ARM/ArmJit.cpp @@ -106,21 +106,6 @@ void ArmJit::DoState(PointerWrap &p) } } -// This is here so the savestate matches between jit and non-jit. -void ArmJit::DoDummyState(PointerWrap &p) -{ - auto s = p.Section("Jit", 1, 2); - if (!s) - return; - - bool dummy = false; - p.Do(dummy); - if (s >= 2) { - dummy = true; - p.Do(dummy); - } -} - void ArmJit::FlushAll() { gpr.FlushAll(); diff --git a/Core/MIPS/ARM/ArmJit.h b/Core/MIPS/ARM/ArmJit.h index 511ce5fce1..77b4c9ea51 100644 --- a/Core/MIPS/ARM/ArmJit.h +++ b/Core/MIPS/ARM/ArmJit.h @@ -39,7 +39,6 @@ public: virtual ~ArmJit(); void DoState(PointerWrap &p) override; - void DoDummyState(PointerWrap &p) override; const JitOptions &GetJitOptions() { return jo; } diff --git a/Core/MIPS/ARM64/Arm64Jit.cpp b/Core/MIPS/ARM64/Arm64Jit.cpp index fab227f303..ae9b6b8f4e 100644 --- a/Core/MIPS/ARM64/Arm64Jit.cpp +++ b/Core/MIPS/ARM64/Arm64Jit.cpp @@ -98,20 +98,6 @@ void Arm64Jit::DoState(PointerWrap &p) { } } -// This is here so the savestate matches between jit and non-jit. -void Arm64Jit::DoDummyState(PointerWrap &p) { - auto s = p.Section("Jit", 1, 2); - if (!s) - return; - - bool dummy = false; - p.Do(dummy); - if (s >= 2) { - dummy = true; - p.Do(dummy); - } -} - void Arm64Jit::FlushAll() { gpr.FlushAll(); diff --git a/Core/MIPS/ARM64/Arm64Jit.h b/Core/MIPS/ARM64/Arm64Jit.h index b2d853c3b2..e8874fa0b5 100644 --- a/Core/MIPS/ARM64/Arm64Jit.h +++ b/Core/MIPS/ARM64/Arm64Jit.h @@ -39,7 +39,6 @@ public: virtual ~Arm64Jit(); void DoState(PointerWrap &p) override; - void DoDummyState(PointerWrap &p) override; const JitOptions &GetJitOptions() { return jo; } diff --git a/Core/MIPS/IR/IRJit.cpp b/Core/MIPS/IR/IRJit.cpp index 26f66bc6ad..b36c40f110 100644 --- a/Core/MIPS/IR/IRJit.cpp +++ b/Core/MIPS/IR/IRJit.cpp @@ -50,20 +50,6 @@ void IRJit::DoState(PointerWrap &p) { frontend_.DoState(p); } -// This is here so the savestate matches between jit and non-jit. -void IRJit::DoDummyState(PointerWrap &p) { - auto s = p.Section("Jit", 1, 2); - if (!s) - return; - - bool dummy = false; - p.Do(dummy); - if (s >= 2) { - dummy = true; - p.Do(dummy); - } -} - void IRJit::ClearCache() { ILOG("IRJit: Clearing the cache!"); blocks_.Clear(); diff --git a/Core/MIPS/IR/IRJit.h b/Core/MIPS/IR/IRJit.h index 84851df7f3..67fa3a993c 100644 --- a/Core/MIPS/IR/IRJit.h +++ b/Core/MIPS/IR/IRJit.h @@ -127,7 +127,6 @@ public: virtual ~IRJit(); void DoState(PointerWrap &p) override; - void DoDummyState(PointerWrap &p) override; const JitOptions &GetJitOptions() { return jo; } diff --git a/Core/MIPS/JitCommon/JitCommon.cpp b/Core/MIPS/JitCommon/JitCommon.cpp index 55409c77c6..f7792a110c 100644 --- a/Core/MIPS/JitCommon/JitCommon.cpp +++ b/Core/MIPS/JitCommon/JitCommon.cpp @@ -21,6 +21,8 @@ #include "ext/udis86/udis86.h" #include "Common/StringUtils.h" +#include "Common/ChunkFile.h" + #include "Core/Util/DisArm64.h" #include "Core/Config.h" @@ -46,6 +48,20 @@ namespace MIPSComp { jit->Compile(currentMIPS->pc); } + void DoDummyJitState(PointerWrap &p) { + // This is here so the savestate matches between jit and non-jit. + auto s = p.Section("Jit", 1, 2); + if (!s) + return; + + bool dummy = false; + p.Do(dummy); + if (s >= 2) { + dummy = true; + p.Do(dummy); + } + } + JitInterface *CreateNativeJit(MIPSState *mips) { #if PPSSPP_ARCH(ARM) return new MIPSComp::ArmJit(mips); diff --git a/Core/MIPS/JitCommon/JitCommon.h b/Core/MIPS/JitCommon/JitCommon.h index 9e91664713..6296506a01 100644 --- a/Core/MIPS/JitCommon/JitCommon.h +++ b/Core/MIPS/JitCommon/JitCommon.h @@ -125,7 +125,6 @@ namespace MIPSComp { virtual JitBlockCache *GetBlockCache() = 0; virtual void InvalidateCacheAt(u32 em_address, int length = 4) = 0; virtual void DoState(PointerWrap &p) = 0; - virtual void DoDummyState(PointerWrap &p) = 0; virtual void RunLoopUntil(u64 globalticks) = 0; virtual void Compile(u32 em_address) = 0; virtual void ClearCache() = 0; @@ -147,5 +146,7 @@ namespace MIPSComp { extern JitInterface *jit; + void DoDummyJitState(PointerWrap &p); + JitInterface *CreateNativeJit(MIPSState *mips); } diff --git a/Core/MIPS/MIPS.cpp b/Core/MIPS/MIPS.cpp index f046d05271..2302928968 100644 --- a/Core/MIPS/MIPS.cpp +++ b/Core/MIPS/MIPS.cpp @@ -262,7 +262,7 @@ void MIPSState::DoState(PointerWrap &p) { if (MIPSComp::jit) MIPSComp::jit->DoState(p); else - MIPSComp::jit->DoDummyState(p); + MIPSComp::DoDummyJitState(p); p.DoArray(r, sizeof(r) / sizeof(r[0])); p.DoArray(f, sizeof(f) / sizeof(f[0])); diff --git a/Core/MIPS/MIPS/MipsJit.cpp b/Core/MIPS/MIPS/MipsJit.cpp index b669109899..48637e8f23 100644 --- a/Core/MIPS/MIPS/MipsJit.cpp +++ b/Core/MIPS/MIPS/MipsJit.cpp @@ -66,21 +66,6 @@ void MipsJit::DoState(PointerWrap &p) } } -// This is here so the savestate matches between jit and non-jit. -void MipsJit::DoDummyState(PointerWrap &p) -{ - auto s = p.Section("Jit", 1, 2); - if (!s) - return; - - bool dummy = false; - p.Do(dummy); - if (s >= 2) { - dummy = true; - p.Do(dummy); - } -} - void MipsJit::FlushAll() { //gpr.FlushAll(); diff --git a/Core/MIPS/MIPS/MipsJit.h b/Core/MIPS/MIPS/MipsJit.h index ad158f5de0..c0ff28582d 100644 --- a/Core/MIPS/MIPS/MipsJit.h +++ b/Core/MIPS/MIPS/MipsJit.h @@ -38,7 +38,6 @@ public: MipsJit(MIPSState *mips); void DoState(PointerWrap &p) override; - void DoDummyState(PointerWrap &p) override; // Compiled ops should ignore delay slots // the compiler will take care of them by itself diff --git a/Core/MIPS/x86/Jit.cpp b/Core/MIPS/x86/Jit.cpp index 6a6b196aec..6e2ceae75e 100644 --- a/Core/MIPS/x86/Jit.cpp +++ b/Core/MIPS/x86/Jit.cpp @@ -142,21 +142,6 @@ void Jit::DoState(PointerWrap &p) { CBreakPoints::SetSkipFirst(0); } -// This is here so the savestate matches between jit and non-jit. -void Jit::DoDummyState(PointerWrap &p) { - auto s = p.Section("Jit", 1, 2); - if (!s) - return; - - bool dummy = false; - p.Do(dummy); - if (s >= 2) { - dummy = true; - p.Do(dummy); - } -} - - void Jit::GetStateAndFlushAll(RegCacheState &state) { gpr.GetState(state.gpr); fpr.GetState(state.fpr); diff --git a/Core/MIPS/x86/Jit.h b/Core/MIPS/x86/Jit.h index de8ef4d8f1..2a35212140 100644 --- a/Core/MIPS/x86/Jit.h +++ b/Core/MIPS/x86/Jit.h @@ -52,7 +52,6 @@ public: const JitOptions &GetJitOptions() { return jo; } void DoState(PointerWrap &p) override; - void DoDummyState(PointerWrap &p) override; // Compiled ops should ignore delay slots // the compiler will take care of them by itself From 0d60c3f386810b26c1744217bbb3357d5509a13f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 30 Nov 2017 01:21:03 +0100 Subject: [PATCH 6/7] Fix UI crash when keyboard navigating out of popup list. --- ext/native/ui/ui_screen.h | 2 +- ext/native/ui/viewgroup.cpp | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/ext/native/ui/ui_screen.h b/ext/native/ui/ui_screen.h index cb894999f5..fee88ce255 100644 --- a/ext/native/ui/ui_screen.h +++ b/ext/native/ui/ui_screen.h @@ -264,7 +264,7 @@ private: const char *category_; ScreenManager *screenManager_; std::string valueText_; - bool restoreFocus_; + bool restoreFocus_ = false; std::set hidden_; }; diff --git a/ext/native/ui/viewgroup.cpp b/ext/native/ui/viewgroup.cpp index 50dabcead2..5c93a4c4c3 100644 --- a/ext/native/ui/viewgroup.cpp +++ b/ext/native/ui/viewgroup.cpp @@ -1314,8 +1314,10 @@ EventReturn ListView::OnItemCallback(int num, EventParams &e) { View *focused = GetFocusedView(); OnChoice.Trigger(ev); CreateAllItems(); - if (focused) - SetFocusedView(e.v); + // TODO: At this point, focused may no longer exist, depending on what OnChoice.Trigger does. + // Disable the refocus feature for now. + // if (focused) + // SetFocusedView(e.v); return EVENT_DONE; } From b4bca7d7a0738df44f8675e2b26c4bd57cdd92cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 30 Nov 2017 01:26:59 +0100 Subject: [PATCH 7/7] VKRFramebufer: Just some checks to be slightly safer in case creation failed.. --- ext/native/thin3d/VulkanRenderManager.h | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index b8257cac86..c5e13130af 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -51,13 +51,20 @@ public: } ~VKRFramebuffer() { - vulkan_->Delete().QueueDeleteImage(color.image); - vulkan_->Delete().QueueDeleteImage(depth.image); - vulkan_->Delete().QueueDeleteImageView(color.imageView); - vulkan_->Delete().QueueDeleteImageView(depth.imageView); - vulkan_->Delete().QueueDeleteDeviceMemory(color.memory); - vulkan_->Delete().QueueDeleteDeviceMemory(depth.memory); - vulkan_->Delete().QueueDeleteFramebuffer(framebuf); + if (color.image) + vulkan_->Delete().QueueDeleteImage(color.image); + if (depth.image) + vulkan_->Delete().QueueDeleteImage(depth.image); + if (color.imageView) + vulkan_->Delete().QueueDeleteImageView(color.imageView); + if (depth.imageView) + vulkan_->Delete().QueueDeleteImageView(depth.imageView); + if (color.memory) + vulkan_->Delete().QueueDeleteDeviceMemory(color.memory); + if (depth.memory) + vulkan_->Delete().QueueDeleteDeviceMemory(depth.memory); + if (framebuf) + vulkan_->Delete().QueueDeleteFramebuffer(framebuf); } int numShadows = 1; // TODO: Support this.