Merge pull request #4 from hrydgard/master

Update compat.ini

.github/ISSUE_TEMPLATE

@@ -9,6 +9,6 @@ You can remove this stuff - just don't forget to answer the questions.  They're
 ### What *should* happen?
 
 
-### What hardware, operating system, and PPSSPP version?  On desktop, GPU matters for graphical issues.
-
+### What hardware, operating system, and PPSSPP version?  On desktop and mobile phone, GPU matters for graphical issues.
+### Please test newest version in https://buildbot.orphis.net/ppsspp/
 

CMakeLists.txt

@@ -91,7 +91,7 @@ if(ANDROID OR WIN32 OR (UNIX AND NOT ARM_NO_VULKAN))
 endif()
 
 list(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake/Modules)
-if(NOT (ARM64 AND MACOSX))
+if(NOT ((ARM64 AND MACOSX) OR IOS))
 	list(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake/sdl)
 endif()
 
@@ -198,7 +198,7 @@ if(USING_EGL)
 	set(OPENGL_LIBRARIES ${OPENGL_LIBRARIES} ${EGL_LIBRARIES})
 endif()
 
-if(NOT LIBRETRO)
+if(NOT LIBRETRO AND NOT IOS)
 	find_package(SDL2)
 endif()
 include(FindThreads)
@@ -281,11 +281,11 @@ if(NOT MSVC)
 
 	set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -g -D_DEBUG")
 	set(CMAKE_CXX_FLAGS_MINSIZEREL "${CMAKE_CXX_FLAGS_MINSIZEREL} -Os -D_NDEBUG")
-	set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3 -D_NDEBUG")
+	set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O2 -D_NDEBUG")
 	set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} -O2 -g -D_NDEBUG")
 	set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -g -D_DEBUG")
 	set(CMAKE_C_FLAGS_MINSIZEREL "${CMAKE_C_FLAGS_MINSIZEREL} -Os -D_NDEBUG")
-	set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -O3 -D_NDEBUG")
+	set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -O2 -D_NDEBUG")
 	set(CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} -O2 -g -D_NDEBUG")
 	#TODO: Remove this and include the file properly everywhere it makes sense
 	# First step is too use the macros everywhere
@@ -767,10 +767,10 @@ include_directories(ext/cityhash)
 
 if(NOT MSVC)
 	# These can be fast even for debug.
-	set_target_properties(udis86 PROPERTIES COMPILE_FLAGS "-O3")
-	set_target_properties(cityhash PROPERTIES COMPILE_FLAGS "-O3")
+	set_target_properties(udis86 PROPERTIES COMPILE_FLAGS "-O2")
+	set_target_properties(cityhash PROPERTIES COMPILE_FLAGS "-O2")
 	if(NOT ZLIB_FOUND)
-		set_target_properties(zlib PROPERTIES COMPILE_FLAGS "-O3")
+		set_target_properties(zlib PROPERTIES COMPILE_FLAGS "-O2")
 	endif()
 endif()
 
@@ -1037,7 +1037,7 @@ if(WIN32)
 	target_link_libraries(Common winmm d3d9 dsound)
 endif()
 
-if(TARGET SDL2::SDL2)
+if(TARGET SDL2::SDL2 AND NOT IOS)
 	target_link_libraries(Common SDL2::SDL2)
 endif()
 
@@ -2168,11 +2168,12 @@ if(IOS)
 	set_source_files_properties(${RSRC_XIB_FILES}
 		PROPERTIES MACOSX_PACKAGE_LOCATION Resources
 	)
-	if(CMAKE_GENERATOR STREQUAL "Xcode")
-		set(APP_DIR_NAME "$(TARGET_BUILD_DIR)/$(FULL_PRODUCT_NAME)")
-	else()
+	#This breaks in modern XCode. Not sure when it worked...
+	#if(CMAKE_GENERATOR STREQUAL "Xcode")
+	#	set(APP_DIR_NAME "$(TARGET_BUILD_DIR)/$(FULL_PRODUCT_NAME)")
+	#else()
 		set(APP_DIR_NAME "$<TARGET_FILE_DIR:PPSSPP>")
-	endif()
+	#endif()
 	add_custom_command(TARGET PPSSPP POST_BUILD
 		COMMAND mkdir -p \"${APP_DIR_NAME}\"
 		COMMAND tar -c -C ${CMAKE_CURRENT_BINARY_DIR} --exclude .DS_Store --exclude .git assets *.png | tar -x -C \"${APP_DIR_NAME}\"
@@ -2185,7 +2186,6 @@ if(IOS)
 		RESOURCE "ios/Settings.bundle"
 		RESOURCE "MoltenVK/iOS/Frameworks"
 		XCODE_ATTRIBUTE_IPHONEOS_DEPLOYMENT_TARGET ${DEPLOYMENT_TARGET}
-		XCODE_ATTRIBUTE_TARGETED_DEVICE_FAMILY "iPhone/iPad"
 		XCODE_ATTRIBUTE_CLANG_ENABLE_OBJC_ARC YES
 		XCODE_ATTRIBUTE_ENABLE_BITCODE NO
 		XCODE_ATTRIBUTE_CODE_SIGN_IDENTITY "-"

Common/GPU/Vulkan/VulkanRenderManager.cpp

@@ -346,8 +346,8 @@ void VulkanRenderManager::DestroyBackbuffers() {
 	StopThread();
 	vulkan_->WaitUntilQueueIdle();
 
-	for (uint32_t i = 0; i < swapchainImageCount_; i++) {
-		vulkan_->Delete().QueueDeleteImageView(swapchainImages_[i].view);
+	for (auto &image : swapchainImages_) {
+		vulkan_->Delete().QueueDeleteImageView(image.view);
 	}
 	swapchainImages_.clear();
 

Core/HLE/sceKernelModule.cpp

@@ -117,6 +117,8 @@ static const char * const blacklistedModules[] = {
 	"sceMD5_Library",
 };
 
+struct WriteVarSymbolState;
+
 struct VarSymbolImport {
 	char moduleName[KERNELOBJECT_MAX_NAME_LENGTH + 1];
 	u32 nid;
@@ -150,7 +152,7 @@ struct FuncSymbolExport {
 	u32 nid;
 };
 
-void ImportVarSymbol(const VarSymbolImport &var);
+void ImportVarSymbol(WriteVarSymbolState &state, const VarSymbolImport &var);
 void ExportVarSymbol(const VarSymbolExport &var);
 void UnexportVarSymbol(const VarSymbolExport &var);
 
@@ -378,11 +380,11 @@ class PSPModule : public KernelObject {
 		ImportFuncSymbol(func, reimporting, GetName());
 	}
 
-	void ImportVar(const VarSymbolImport &var) {
+	void ImportVar(WriteVarSymbolState &state, const VarSymbolImport &var) {
 		// Keep track and actually hook it up if possible.
 		importedVars.push_back(var);
 		impExpModuleNames.insert(var.moduleName);
-		ImportVarSymbol(var);
+		ImportVarSymbol(state, var);
 	}
 
 	void ExportFunc(const FuncSymbolExport &func) {
@@ -557,22 +559,22 @@ void __KernelModuleShutdown()
 // Sometimes there are multiple LO16's or HI16's per pair, even though the ABI says nothing of this.
 // For multiple LO16's, we need the original (unrelocated) instruction data of the HI16.
 // For multiple HI16's, we just need to set each one.
-struct HI16RelocInfo
-{
+struct HI16RelocInfo {
 	u32 addr;
 	u32 data;
 };
-static void WriteVarSymbol(u32 exportAddress, u32 relocAddress, u8 type, bool reverse = false)
-{
-	// We have to post-process the HI16 part, since it might be +1 or not depending on the LO16 value.
-	static u32 lastHI16ExportAddress = 0;
-	static std::vector<HI16RelocInfo> lastHI16Relocs;
-	static bool lastHI16Processed = true;
+// We have to post-process the HI16 part, since it might be +1 or not depending on the LO16 value.
+// For that purpose, we use this state to track HI16s to adjust.
+struct WriteVarSymbolState {
+	u32 lastHI16ExportAddress = 0;
+	std::vector<HI16RelocInfo> lastHI16Relocs;
+	bool lastHI16Processed = true;
+};
 
+static void WriteVarSymbol(WriteVarSymbolState &state, u32 exportAddress, u32 relocAddress, u8 type, bool reverse = false) {
 	u32 relocData = Memory::Read_Instruction(relocAddress, true).encoding;
 
-	switch (type)
-	{
+	switch (type) {
 	case R_MIPS_NONE:
 		WARN_LOG_REPORT(LOADER, "Var relocation type NONE - %08x => %08x", exportAddress, relocAddress);
 		break;
@@ -601,22 +603,22 @@ static void WriteVarSymbol(u32 exportAddress, u32 relocAddress, u8 type, bool re
 	*/
 
 	case R_MIPS_HI16:
-		if (lastHI16ExportAddress != exportAddress) {
-			if (!lastHI16Processed && lastHI16Relocs.size() >= 1) {
-				WARN_LOG_REPORT(LOADER, "Unsafe unpaired HI16 variable relocation @ %08x / %08x", lastHI16Relocs[lastHI16Relocs.size() - 1].addr, relocAddress);
+		if (state.lastHI16ExportAddress != exportAddress) {
+			if (!state.lastHI16Processed && !state.lastHI16Relocs.empty()) {
+				WARN_LOG_REPORT(LOADER, "Unsafe unpaired HI16 variable relocation @ %08x / %08x", state.lastHI16Relocs[state.lastHI16Relocs.size() - 1].addr, relocAddress);
 			}
 
-			lastHI16ExportAddress = exportAddress;
-			lastHI16Relocs.clear();
+			state.lastHI16ExportAddress = exportAddress;
+			state.lastHI16Relocs.clear();
 		}
 
 		// After this will be an R_MIPS_LO16.  If that addition overflows, we need to account for it in HI16.
 		// The R_MIPS_LO16 and R_MIPS_HI16 will often be *different* relocAddress values.
 		HI16RelocInfo reloc;
 		reloc.addr = relocAddress;
 		reloc.data = Memory::Read_Instruction(relocAddress, true).encoding;
-		lastHI16Relocs.push_back(reloc);
-		lastHI16Processed = false;
+		state.lastHI16Relocs.push_back(reloc);
+		state.lastHI16Processed = false;
 		break;
 
 	case R_MIPS_LO16:
@@ -632,27 +634,26 @@ static void WriteVarSymbol(u32 exportAddress, u32 relocAddress, u8 type, bool re
 			}
 
 			// The ABI requires that these come in pairs, at least.
-			if (lastHI16Relocs.empty()) {
+			if (state.lastHI16Relocs.empty()) {
 				ERROR_LOG_REPORT(LOADER, "LO16 without any HI16 variable import at %08x for %08x", relocAddress, exportAddress);
 			// Try to process at least the low relocation...
-			} else if (lastHI16ExportAddress != exportAddress) {
-				ERROR_LOG_REPORT(LOADER, "HI16 and LO16 imports do not match at %08x for %08x (should be %08x)", relocAddress, lastHI16ExportAddress, exportAddress);
+			} else if (state.lastHI16ExportAddress != exportAddress) {
+				ERROR_LOG_REPORT(LOADER, "HI16 and LO16 imports do not match at %08x for %08x (should be %08x)", relocAddress, state.lastHI16ExportAddress, exportAddress);
 			} else {
 				// Process each of the HI16.  Usually there's only one.
-				for (auto it = lastHI16Relocs.begin(), end = lastHI16Relocs.end(); it != end; ++it)
-				{
+				for (auto &reloc : state.lastHI16Relocs) {
 					if (!reverse) {
-						full = (it->data << 16) + offsetLo + exportAddress;
+						full = (reloc.data << 16) + offsetLo + exportAddress;
 					} else {
-						full = (it->data << 16) + offsetLo - exportAddress;
+						full = (reloc.data << 16) + offsetLo - exportAddress;
 					}
 					// The low instruction will be a signed add, which means (full & 0x8000) will subtract.
 					// We add 1 in that case so that it ends up the right value.
 					u16 high = (full >> 16) + ((full & 0x8000) ? 1 : 0);
-					Memory::Write_U32((it->data & ~0xFFFF) | high, it->addr);
-					currentMIPS->InvalidateICache(it->addr, 4);
+					Memory::Write_U32((reloc.data & ~0xFFFF) | high, reloc.addr);
+					currentMIPS->InvalidateICache(reloc.addr, 4);
 				}
-				lastHI16Processed = true;
+				state.lastHI16Processed = true;
 			}
 
 			// With full set above (hopefully), now we just need to correct the low instruction.
@@ -668,7 +669,7 @@ static void WriteVarSymbol(u32 exportAddress, u32 relocAddress, u8 type, bool re
 	currentMIPS->InvalidateICache(relocAddress, 4);
 }
 
-void ImportVarSymbol(const VarSymbolImport &var) {
+void ImportVarSymbol(WriteVarSymbolState &state, const VarSymbolImport &var) {
 	if (var.nid == 0) {
 		// TODO: What's the right thing for this?
 		ERROR_LOG_REPORT(LOADER, "Var import with nid = 0, type = %d", var.type);
@@ -688,9 +689,9 @@ void ImportVarSymbol(const VarSymbolImport &var) {
 		}
 
 		// Look for exports currently loaded modules already have.  Maybe it's available?
-		for (auto it = module->exportedVars.begin(), end = module->exportedVars.end(); it != end; ++it) {
-			if (it->Matches(var)) {
-				WriteVarSymbol(it->symAddr, var.stubAddr, var.type);
+		for (const auto &exported : module->exportedVars) {
+			if (exported.Matches(var)) {
+				WriteVarSymbol(state, exported.symAddr, var.stubAddr, var.type);
 				return;
 			}
 		}
@@ -709,10 +710,11 @@ void ExportVarSymbol(const VarSymbolExport &var) {
 		}
 
 		// Look for imports currently loaded modules already have, hook it up right away.
-		for (auto it = module->importedVars.begin(), end = module->importedVars.end(); it != end; ++it) {
-			if (var.Matches(*it)) {
+		WriteVarSymbolState state;
+		for (auto &imported : module->importedVars) {
+			if (var.Matches(imported)) {
 				INFO_LOG(LOADER, "Resolving var %s/%08x", var.moduleName, var.nid);
-				WriteVarSymbol(var.symAddr, it->stubAddr, it->type);
+				WriteVarSymbol(state, var.symAddr, imported.stubAddr, imported.type);
 			}
 		}
 	}
@@ -727,10 +729,11 @@ void UnexportVarSymbol(const VarSymbolExport &var) {
 		}
 
 		// Look for imports modules that are *still* loaded have, and reverse them.
-		for (auto it = module->importedVars.begin(), end = module->importedVars.end(); it != end; ++it) {
-			if (var.Matches(*it)) {
+		WriteVarSymbolState state;
+		for (auto &imported : module->importedVars) {
+			if (var.Matches(imported)) {
 				INFO_LOG(LOADER, "Unresolving var %s/%08x", var.moduleName, var.nid);
-				WriteVarSymbol(var.symAddr, it->stubAddr, it->type, true);
+				WriteVarSymbol(state, var.symAddr, imported.stubAddr, imported.type, true);
 			}
 		}
 	}
@@ -1051,12 +1054,13 @@ static bool KernelImportModuleFuncs(PSPModule *module, u32 *firstImportStubAddr,
 					continue;
 				}
 
+				WriteVarSymbolState state;
 				u32_le *varRef = (u32_le *)Memory::GetPointer(varRefsPtr);
 				for (; *varRef != 0; ++varRef) {
 					var.nid = nid;
 					var.stubAddr = (*varRef & 0x03FFFFFF) << 2;
 					var.type = *varRef >> 26;
-					module->ImportVar(var);
+					module->ImportVar(state, var);
 				}
 			}
 		} else if (entry->numVars > 0 && !reimporting) {

Core/HLE/sceNetAdhoc.cpp

@@ -300,15 +300,16 @@ static void __AdhocctlNotify(u64 userdata, int cyclesLate) {
 				CoreTiming::ScheduleEvent(usToCycles(500) - cyclesLate, adhocctlNotifyEvent, userdata);
 				return;
 			}
-			else
+			else if (req.opcode != OPCODE_LOGIN)
 				result = ERROR_NET_ADHOCCTL_BUSY;
 		}
 	}
 	else
 		result = ERROR_NET_ADHOCCTL_WLAN_SWITCH_OFF;
 
+	u32 waitVal = __KernelGetWaitValue(threadID, error);
 	__KernelResumeThreadFromWait(threadID, result);
-	DEBUG_LOG(SCENET, "Returning (WaitID: %d, error: %d) Result (%08x) of sceNetAdhocctl - State: %d", waitID, error, (int)result, adhocctlState);
+	DEBUG_LOG(SCENET, "Returning (WaitID: %d, error: %d) Result (%08x) of sceNetAdhocctl - Opcode: %d, State: %d", waitID, error, (int)result, waitVal, adhocctlState);
 
 	// We are done with this request
 	adhocctlRequests.erase(uid);

GPU/Common/FragmentShaderGenerator.cpp

@@ -778,10 +778,21 @@ bool GenerateFragmentShader(const FShaderID &id, char *buffer, const ShaderLangu
 				// When testing against 0 (common), we can avoid some math.
 				// 0.002 is approximately half of 1.0 / 255.0.
 				if (colorTestFunc == GE_COMP_NOTEQUAL) {
-					WRITE(p, "  if (v.r < 0.002 && v.g < 0.002 && v.b < 0.002) %s\n", discardStatement);
+					if (compat.shaderLanguage == GLSL_VULKAN) {
+						// Old workaround for Adreno driver bug. We could make this the main path actually
+						// since the math is roughly equivalent given the non-negative inputs.
+						WRITE(p, "  if (v.r + v.g + v.b < 0.002) %s\n", discardStatement);
+					} else {
+						WRITE(p, "  if (v.r < 0.002 && v.g < 0.002 && v.b < 0.002) %s\n", discardStatement);
+					}
 				} else if (colorTestFunc != GE_COMP_NEVER) {
-					// Anything else is a test for == 0.
-					WRITE(p, "  if (v.r > 0.002 || v.g > 0.002 || v.b > 0.002) %s\n", discardStatement);
+					if (compat.shaderLanguage == GLSL_VULKAN) {
+						// See the GE_COMP_NOTEQUAL case.
+						WRITE(p, "  if (v.r + v.g + v.b > 0.002) %s\n", discardStatement);
+					} else {
+						// Anything else is a test for == 0.
+						WRITE(p, "  if (v.r > 0.002 || v.g > 0.002 || v.b > 0.002) %s\n", discardStatement);
+					}
 				} else {
 					// NEVER has been logged as used by games, although it makes little sense - statically failing.
 					// Maybe we could discard the drawcall, but it's pretty rare.  Let's just statically discard here.

GPU/Common/GPUStateUtils.cpp

@@ -575,6 +575,9 @@ void ConvertViewportAndScissor(bool useBufferedRendering, float renderWidth, flo
 		renderHeightFactor = renderHeight / 272.0f;
 	}
 
+	_assert_(renderWidthFactor > 0.0);
+	_assert_(renderHeightFactor > 0.0);
+
 	renderX = gstate_c.curRTOffsetX;
 
 	// Scissor
@@ -694,8 +697,11 @@ void ConvertViewportAndScissor(bool useBufferedRendering, float renderWidth, flo
 
 		out.viewportX = left * renderWidthFactor + displayOffsetX;
 		out.viewportY = top * renderHeightFactor + displayOffsetY;
-		out.viewportW = (right - left) * renderWidthFactor;
-		out.viewportH = (bottom - top) * renderHeightFactor;
+
+		// The calculations should end up with zero or positive values, but let's protect against any
+		// precision issues. See #13921.
+		out.viewportW = std::max(0.0f, (right - left) * renderWidthFactor);
+		out.viewportH = std::max(0.0f, (bottom - top) * renderHeightFactor);
 
 		// The depth viewport parameters are the same, but we handle it a bit differently.
 		// When clipping is enabled, depth is clamped to [0, 65535].  And minz/maxz discard.