Merge pull request #22794 from hrydgard/settings-stale-draw-context

Fix crash after changing Display resolution on Android, and other stale draw context uses

GPU/Common/PresentationCommon.cpp

@@ -599,8 +599,7 @@ void PresentationCommon::CreateDeviceObjects() {
 	texColorRBSwizzle_ = CreatePipeline({ draw_->GetVshaderPreset(VS_TEXTURE_COLOR_2D), draw_->GetFshaderPreset(FS_TEXTURE_COLOR_2D_RB_SWIZZLE) }, false, &vsTexColBufDesc);
 	texColorBlend_ = CreatePipeline({ draw_->GetVshaderPreset(VS_TEXTURE_COLOR_2D), draw_->GetFshaderPreset(FS_TEXTURE_COLOR_2D) }, false, &vsTexColBufDesc, true);
 	overlayVdata_ = draw_->CreateBuffer(sizeof(Vertex) * 4, BufferUsageFlag::DYNAMIC | BufferUsageFlag::VERTEXDATA);
-
-	restorePostShader_ = false;
+	// Not resetting restorePostShader_: after a device loss, BeginFrame uses it to bring the post-shader back.
 }
 
 template <typename T>

GPU/GLES/ShaderManagerGLES.cpp

@@ -612,6 +612,7 @@ void ShaderManagerGLES::Clear() {
 	linkedShaderCache_.clear();
 	fsCache_.Clear();
 	vsCache_.Clear();
+	lastShader_ = nullptr;
 	lastFSID_.set_invalid();
 	lastVSID_.set_invalid();
 

GPU/GLES/TextureCacheGLES.cpp

@@ -407,6 +407,7 @@ bool TextureCacheGLES::GetCurrentTextureDebug(GPUDebugBuffer &buffer, int level,
 
 void TextureCacheGLES::DeviceLost() {
 	TextureCacheCommon::DeviceLost();
+	lastBoundTexture_ = nullptr;
 	draw_ = nullptr;
 	render_ = nullptr;
 }

GPU/Metal/DrawEngineMetal.mm

@@ -178,6 +178,10 @@ void DrawEngineMetal::DeviceLost() {
 	}
 	boundSecondary_ = nil;
 	boundDepal_ = nil;
+	// Allocated from the old render manager's push buffer.
+	baseUBO_ = MetalPushAlloc{};
+	lightUBO_ = MetalPushAlloc{};
+	uboFrame_ = (uint64_t)-1;
 	draw_ = nullptr;
 	rm_ = nullptr;
 }

GPU/Metal/GPU_Metal.mm

@@ -214,6 +214,12 @@ void GPU_Metal::DeviceLost() {
 
 void GPU_Metal::DeviceRestore(Draw::DrawContext *draw) {
 	GPUCommonHW::DeviceRestore(draw);
+
+	// DeviceLost saved the cache and then threw everything away. Load it again, or the next save would
+	// overwrite the file with only what gets drawn from now on.
+	if (shaderCachePath_.Valid()) {
+		LoadCache(shaderCachePath_);
+	}
 }
 
 void GPU_Metal::GetStats(StringWriter &w) {

GPU/Vulkan/DrawEngineVulkan.cpp

@@ -133,6 +133,10 @@ void DrawEngineVulkan::DestroyDeviceObjects() {
 void DrawEngineVulkan::DeviceLost() {
 	DestroyDeviceObjects();
 	DirtyAllUBOs();
+	// The pipelines and views these point to are gone (BeginFrame resets them too, but a flush could come first).
+	lastPipeline_ = nullptr;
+	boundDepal_ = VK_NULL_HANDLE;
+	boundSecondary_ = VK_NULL_HANDLE;
 	draw_ = nullptr;
 }
 

UI/DeveloperSettingsPages.cpp

@@ -403,9 +403,9 @@ static void BuildUIPage(SettingsPage *page) {
 	using namespace UI;
 	SettingsPageHost *host = page->Host();
 
-	UIContext *uiContext = host->PageScreenManager()->getUIContext();
-	page->Choice("Reload UI atlas").OnClick([uiContext](UI::EventParams &) {
-		uiContext->InvalidateAtlas();
+	// Looked up when clicked, the UI context is recreated along with the graphics.
+	page->Choice("Reload UI atlas").OnClick([host](UI::EventParams &) {
+		host->PageScreenManager()->getUIContext()->InvalidateAtlas();
 	});
 
 	// TODO: Move most of these strings out of the SysInfo category.

UI/GPUDriverTestScreen.cpp

@@ -239,61 +239,48 @@ GPUDriverTestScreen::GPUDriverTestScreen() {
 }
 
 GPUDriverTestScreen::~GPUDriverTestScreen() {
-	if (discardWriteDepthStencil_)
-		discardWriteDepthStencil_->Release();
-	if (discardWriteDepth_)
-		discardWriteDepth_->Release();
-	if (discardWriteStencil_)
-		discardWriteStencil_->Release();
+	DestroyDeviceObjects();
+}
 
-	if (drawTestStencilEqualDepthAlways_)
-		drawTestStencilEqualDepthAlways_->Release();
-	if (drawTestStencilNotEqualDepthAlways_)
-		drawTestStencilNotEqualDepthAlways_->Release();
-	if (drawTestStencilEqual_)
-		drawTestStencilEqual_->Release();
-	if (drawTestStencilNotEqual_)
-		drawTestStencilNotEqual_->Release();
-	if (drawTestStencilAlwaysDepthLessEqual_)
-		drawTestStencilAlwaysDepthLessEqual_->Release();
-	if (drawTestStencilAlwaysDepthGreater_)
-		drawTestStencilAlwaysDepthGreater_->Release();
-	if (drawTestDepthLessEqual_)
-		drawTestDepthLessEqual_->Release();
-	if (drawTestDepthGreater_)
-		drawTestDepthGreater_->Release();
+template <class T>
+static void ReleaseAndClear(T *&obj) {
+	if (obj) {
+		obj->Release();
+		obj = nullptr;
+	}
+}
 
-	if (discardFragShader_)
-		discardFragShader_->Release();
+void GPUDriverTestScreen::deviceLost() {
+	UIBaseDialogScreen::deviceLost();
+	DestroyDeviceObjects();
+}
 
-	// Shader test
-	if (adrenoLogicDiscardPipeline_)
-		adrenoLogicDiscardPipeline_->Release();
-	if (flatShadingPipeline_)
-		flatShadingPipeline_->Release();
-
-	if (adrenoLogicDiscardFragShader_)
-		adrenoLogicDiscardFragShader_->Release();
-	if (adrenoLogicDiscardVertShader_)
-		adrenoLogicDiscardVertShader_->Release();
-	if (flatFragShader_)
-		flatFragShader_->Release();
-	if (flatVertShader_)
-		flatVertShader_->Release();
-
-	if (samplerNearest_)
-		samplerNearest_->Release();
+// Everything is created again on the next draw, from the new draw context.
+void GPUDriverTestScreen::DestroyDeviceObjects() {
+	ReleaseAndClear(discardWriteDepthStencil_);
+	ReleaseAndClear(discardWriteDepth_);
+	ReleaseAndClear(discardWriteStencil_);
+	ReleaseAndClear(drawTestStencilEqualDepthAlways_);
+	ReleaseAndClear(drawTestStencilNotEqualDepthAlways_);
+	ReleaseAndClear(drawTestStencilEqual_);
+	ReleaseAndClear(drawTestStencilNotEqual_);
+	ReleaseAndClear(drawTestStencilAlwaysDepthLessEqual_);
+	ReleaseAndClear(drawTestStencilAlwaysDepthGreater_);
+	ReleaseAndClear(drawTestDepthLessEqual_);
+	ReleaseAndClear(drawTestDepthGreater_);
+	ReleaseAndClear(discardFragShader_);
+	ReleaseAndClear(adrenoLogicDiscardPipeline_);
+	ReleaseAndClear(flatShadingPipeline_);
+	ReleaseAndClear(adrenoLogicDiscardFragShader_);
+	ReleaseAndClear(adrenoLogicDiscardVertShader_);
+	ReleaseAndClear(flatFragShader_);
+	ReleaseAndClear(flatVertShader_);
+	ReleaseAndClear(samplerNearest_);
 }
 
 void GPUDriverTestScreen::CreateViews() {
 	using namespace Draw;
 
-	if (!samplerNearest_) {
-		DrawContext *draw = screenManager()->getDrawContext();
-		SamplerStateDesc nearestDesc{};
-		samplerNearest_ = draw->CreateSamplerState(nearestDesc);
-	}
-
 	// Don't bother with views for now.
 	using namespace UI;
 	auto di = GetI18NCategory(I18NCat::DIALOG);
@@ -631,6 +618,10 @@ void GPUDriverTestScreen::ShaderTest(UIContext &dc) {
 }
 
 void GPUDriverTestScreen::DrawForeground(UIContext &dc) {
+	if (!samplerNearest_) {
+		Draw::SamplerStateDesc nearestDesc{};
+		samplerNearest_ = screenManager()->getDrawContext()->CreateSamplerState(nearestDesc);
+	}
 	switch (tabHolder_->GetCurrentTab()) {
 	case 0:
 		DiscardTest(dc);

UI/GPUDriverTestScreen.h

@@ -18,12 +18,14 @@ public:
 
 	void CreateViews() override;
 	void DrawForeground(UIContext &dc) override;
+	void deviceLost() override;
 
 	const char *tag() const override { return "GPUDriverTest"; }
 
 private:
 	void DiscardTest(UIContext &dc);
 	void ShaderTest(UIContext &dc);
+	void DestroyDeviceObjects();
 
 	// Common objects
 	Draw::SamplerState *samplerNearest_ = nullptr;

UI/Settings/GeneralPage.cpp

@@ -154,11 +154,11 @@ static void BuildUISettingsPage(SettingsPage *page) {
 	using namespace UI;
 	SettingsPageHost *host = page->Host();
 
-	UIContext *ctx = host->PageScreenManager()->getUIContext();
-	page->Setting(&g_Config.iUIScaleFactor).OnChange([ctx](UI::EventParams &e) {
+	// The UI context is looked up when used, since it's recreated along with the graphics.
+	page->Setting(&g_Config.iUIScaleFactor).OnChange([host](UI::EventParams &e) {
 		const float dpiMul = UIScaleFactorToMultiplier(g_Config.iUIScaleFactor);
 		g_display.Recalculate(-1, -1, -1, -1, dpiMul);
-		ctx->InvalidateAtlas();
+		host->PageScreenManager()->getUIContext()->InvalidateAtlas();
 		NativeResized();
 	});
 

UI/Settings/GraphicsPage.cpp

@@ -175,8 +175,10 @@ void BuildGraphicsSettings(SettingsPage *page) {
 
 #if (PPSSPP_PLATFORM(IOS) && !PPSSPP_PLATFORM(TVOS)) || PPSSPP_PLATFORM(ANDROID)
 	// Showing the game there needs a second swapchain. Otherwise the system mirrors the screen.
-	page->Setting(&g_Config.iExternalDisplay).EnabledFunc([draw] {
-		return draw->GetDeviceCaps().multipleSwapchains;
+	// Looked up each time: the draw context is replaced when Android loses the surface, and this page isn't rebuilt.
+	page->Setting(&g_Config.iExternalDisplay).EnabledFunc([host] {
+		Draw::DrawContext *draw = host->PageScreenManager()->getDrawContext();
+		return draw && draw->GetDeviceCaps().multipleSwapchains;
 	}).OnChange([previous = g_Config.iExternalDisplay](UI::EventParams &e) mutable {
 		// Picking the current value again would rebuild the TV's output for nothing (iOS starts over on a change).
 		if (g_Config.iExternalDisplay == previous) {

UI/SystemInfoScreen.cpp

@@ -129,7 +129,7 @@ void SystemInfoScreen::CreateDeviceInfoTab(UI::LinearLayout *deviceSpecs) {
 
 	Draw::DrawContext *draw = screenManager()->getDrawContext();
 
-	systemInfo->Add(new Choice(si->T("Copy summary to clipboard"), ImageID("I_FILE_COPY")))->OnClick.Add([draw](UI::EventParams &e) { CopySummaryToClipboard(draw); });
+	systemInfo->Add(new Choice(si->T("Copy summary to clipboard"), ImageID("I_FILE_COPY")))->OnClick.Add([this](UI::EventParams &e) { CopySummaryToClipboard(screenManager()->getDrawContext()); });
 	systemInfo->Add(new InfoItem(si->T("System Name"), System_GetProperty(SYSPROP_NAME)));
 #if PPSSPP_PLATFORM(ANDROID)
 	systemInfo->Add(new InfoItem(si->T("System Version"), StringFromInt(System_GetPropertyInt(SYSPROP_SYSTEMVERSION))));