[WIP] rsx: Use a sane reference counting model

rpcs3/Emu/RSX/Common/surface_store.h

@@ -134,12 +134,24 @@ namespace rsx
 		GcmTileInfo *tile = nullptr;
 		rsx::surface_antialiasing write_aa_mode = rsx::surface_antialiasing::center_1_sample;
 
+		render_target_descriptor() {}
+
+		virtual ~render_target_descriptor()
+		{
+			if (old_contents)
+			{
+				// Cascade resource derefs
+				LOG_ERROR(RSX, "Resource was destroyed whilst holding a resource reference!");
+			}
+		}
+
 		virtual image_storage_type get_surface() = 0;
 		virtual u16 get_surface_width() const = 0;
 		virtual u16 get_surface_height() const = 0;
 		virtual u16 get_rsx_pitch() const = 0;
 		virtual u16 get_native_pitch() const = 0;
 		virtual bool is_depth_surface() const = 0;
+		virtual void release_ref(image_storage_type) const = 0;
 
 		u8 get_bpp() const
 		{
@@ -179,9 +191,17 @@ namespace rsx
 			return true;
 		}
 
+		void clear_rw_barrier()
+		{
+			release_ref(old_contents.source);
+			old_contents = {};
+		}
+
 		template<typename T>
 		void set_old_contents(T* other)
 		{
+			verify(HERE), !old_contents;
+
 			if (!other || other->get_rsx_pitch() != this->get_rsx_pitch())
 			{
 				old_contents = {};
@@ -190,14 +210,29 @@ namespace rsx
 
 			old_contents = {};
 			old_contents.source = other;
+			other->add_ref();
 		}
 
 		template<typename T>
 		void set_old_contents_region(const T& region, bool normalized)
 		{
+			if (old_contents)
+			{
+				// This can happen when doing memory splits
+				auto old_surface = static_cast<decltype(region.source)>(old_contents.source);
+				if (old_surface->last_use_tag > region.source->last_use_tag)
+				{
+					return;
+				}
+
+				clear_rw_barrier();
+			}
+
 			// NOTE: This method will not perform pitch verification!
-			verify(HERE), region.source;
+			verify(HERE), !old_contents, region.source, region.source != this;
+
 			old_contents = region.template cast<image_storage_type>();
+			region.source->add_ref();
 
 			// Reverse normalization process if needed
 			if (normalized)
@@ -298,7 +333,11 @@ namespace rsx
 
 			read_aa_mode = write_aa_mode;
 			dirty = false;
-			old_contents = {};
+
+			if (old_contents.source)
+			{
+				clear_rw_barrier();
+			}
 		}
 
 		// Returns the rect area occupied by this surface expressed as an 8bpp image with no AA
@@ -712,7 +751,9 @@ namespace rsx
 					if (Traits::rtt_has_format_width_height(rtt, color_format, width, height, true))
 					{
 						new_surface_storage = std::move(rtt);
-
+#ifndef INCOMPLETE_SURFACE_CACHE_IMPL
+						Traits::notify_surface_reused(new_surface_storage);
+#endif
 						if (old_surface)
 						{
 							// Exchange this surface with the invalidated one
@@ -843,7 +884,9 @@ namespace rsx
 					if (Traits::ds_has_format_width_height(ds, depth_format, width, height, true))
 					{
 						new_surface_storage = std::move(ds);
-
+#ifndef INCOMPLETE_SURFACE_CACHE_IMPL
+						Traits::notify_surface_reused(new_surface_storage);
+#endif
 						if (old_surface)
 						{
 							//Exchange this surface with the invalidated one
@@ -1440,5 +1483,29 @@ namespace rsx
 		{
 			cache_tag = rsx::get_shared_tag();
 		}
+
+		void invalidate_all()
+		{
+			// Unbind and invalidate all resources
+			auto free_resource_list = [&](auto &data)
+			{
+				for (auto &e : data)
+				{
+					Traits::notify_surface_invalidated(e.second);
+					invalidated_resources.push_back(std::move(e.second));
+				}
+
+				data.clear();
+			};
+
+			free_resource_list(m_render_targets_storage);
+			free_resource_list(m_depth_stencil_storage);
+
+			m_bound_depth_stencil = std::make_pair(0, nullptr);
+			for (auto &rtt : m_bound_render_targets)
+			{
+				rtt = std::make_pair(0, nullptr);
+			}
+		}
 	};
 }

rpcs3/Emu/RSX/GL/GLGSRender.cpp

@@ -1013,6 +1013,7 @@ void GLGSRender::on_exit()
 	zcull_ctrl.release();
 
 	m_prog_buffer.clear();
+	m_rtts.destroy();
 
 	for (auto &fbo : m_framebuffer_cache)
 	{
@@ -1800,10 +1801,9 @@ void GLGSRender::flip(int buffer, bool emu_flip)
 	auto removed_textures = m_rtts.free_invalidated();
 	m_framebuffer_cache.remove_if([&](auto& fbo)
 	{
-		if (fbo.deref_count >= 2) return true; // Remove if stale
+		if (fbo.unused_check_count() >= 2) return true; // Remove if stale
 		if (fbo.references_any(removed_textures)) return true; // Remove if any of the attachments is invalid
 
-		fbo.deref_count++;
 		return false;
 	});
 

rpcs3/Emu/RSX/GL/GLRenderTargets.cpp

@@ -298,11 +298,17 @@ void GLGSRender::init_buffers(rsx::framebuffer_creation_context context, bool sk
 
 	framebuffer_status_valid = false;
 
+	if (m_draw_fbo)
+	{
+		// Release resource
+		static_cast<gl::framebuffer_holder*>(m_draw_fbo)->release();
+	}
+
 	for (auto &fbo : m_framebuffer_cache)
 	{
 		if (fbo.matches(color_targets, depth_stencil_target))
 		{
-			fbo.reset_refs();
+			fbo.add_ref();
 
 			m_draw_fbo = &fbo;
 			m_draw_fbo->bind();
@@ -316,6 +322,8 @@ void GLGSRender::init_buffers(rsx::framebuffer_creation_context context, bool sk
 	if (!framebuffer_status_valid)
 	{
 		m_framebuffer_cache.emplace_back();
+		m_framebuffer_cache.back().add_ref();
+
 		m_draw_fbo = &m_framebuffer_cache.back();
 		m_draw_fbo->create();
 		m_draw_fbo->bind();
@@ -609,6 +617,8 @@ void gl::render_target::memory_barrier(gl::command_context& cmd, bool force_init
 	if (!rsx::pitch_compatible(this, src_texture))
 	{
 		LOG_TRACE(RSX, "Pitch mismatch, could not transfer inherited memory");
+
+		clear_rw_barrier();
 		return;
 	}
 

rpcs3/Emu/RSX/GL/GLRenderTargets.h

@@ -124,6 +124,11 @@ namespace gl
 			}
 		}
 
+		void release_ref(texture* t) const override
+		{
+			static_cast<gl::render_target*>(t)->release();
+		}
+
 		texture* get_surface() override
 		{
 			return (gl::texture*)this;
@@ -184,6 +189,7 @@ struct gl_render_target_traits
 
 		result->set_cleared(false);
 		result->queue_tag(address);
+		result->add_ref();
 		return result;
 	}
 
@@ -209,6 +215,7 @@ struct gl_render_target_traits
 
 		result->set_cleared(false);
 		result->queue_tag(address);
+		result->add_ref();
 		return result;
 	}
 
@@ -225,6 +232,7 @@ struct gl_render_target_traits
 			const auto new_h = rsx::apply_resolution_scale(prev.height, true, ref->get_surface_height());
 
 			sink.reset(new gl::render_target(new_w, new_h, internal_format));
+			sink->add_ref();
 		}
 
 		prev.target = sink.get();
@@ -257,10 +265,10 @@ struct gl_render_target_traits
 		info->bpp = surface->get_bpp();
 	}
 
-	static void prepare_rtt_for_drawing(gl::command_context&, gl::render_target *rtt) { rtt->reset_refs(); }
+	static void prepare_rtt_for_drawing(gl::command_context&, gl::render_target*) {}
 	static void prepare_rtt_for_sampling(gl::command_context&, gl::render_target*) {}
 	
-	static void prepare_ds_for_drawing(gl::command_context&, gl::render_target *ds) { ds->reset_refs(); }
+	static void prepare_ds_for_drawing(gl::command_context&, gl::render_target*) {}
 	static void prepare_ds_for_sampling(gl::command_context&, gl::render_target*) {}
 
 	static
@@ -279,8 +287,11 @@ struct gl_render_target_traits
 	}
 
 	static
-	void notify_surface_invalidated(const std::unique_ptr<gl::render_target>&)
-	{}
+	void notify_surface_invalidated(const std::unique_ptr<gl::render_target>& surface)
+	{
+		if (surface->old_contents) surface->clear_rw_barrier();
+		surface->release();
+	}
 
 	static
 	void notify_surface_persist(const std::unique_ptr<gl::render_target>& surface)
@@ -288,24 +299,30 @@ struct gl_render_target_traits
 		surface->save_aa_mode();
 	}
 
+	static
+	void notify_surface_reused(const std::unique_ptr<gl::render_target>& surface)
+	{
+		surface->add_ref();
+	}
+
 	static
 	bool rtt_has_format_width_height(const std::unique_ptr<gl::render_target> &rtt, rsx::surface_color_format format, size_t width, size_t height, bool check_refs=false)
 	{
-		if (check_refs) //TODO
+		if (check_refs && rtt->has_refs())
 			return false;
 
-		auto internal_fmt = rsx::internals::sized_internal_format(format);
+		const auto internal_fmt = rsx::internals::sized_internal_format(format);
 		return rtt->get_internal_format() == internal_fmt && rtt->matches_dimensions((u16)width, (u16)height);
 	}
 
 	static
-	bool ds_has_format_width_height(const std::unique_ptr<gl::render_target> &rtt, rsx::surface_depth_format, size_t width, size_t height, bool check_refs=false)
+	bool ds_has_format_width_height(const std::unique_ptr<gl::render_target> &ds, rsx::surface_depth_format format, size_t width, size_t height, bool check_refs=false)
 	{
-		if (check_refs) //TODO
+		if (check_refs && ds->has_refs())
 			return false;
 
-		// TODO: check format
-		return rtt->matches_dimensions((u16)width, (u16)height);
+		const auto internal_fmt = rsx::internals::surface_depth_format_to_gl(format).internal_format;
+		return ds->get_internal_format() == internal_fmt && ds->matches_dimensions((u16)width, (u16)height);
 	}
 
 	// Note : pbo breaks fbo here so use classic texture copy
@@ -353,18 +370,23 @@ struct gl_render_target_traits
 
 struct gl_render_targets : public rsx::surface_store<gl_render_target_traits>
 {
+	void destroy()
+	{
+		invalidate_all();
+		invalidated_resources.clear();
+	}
+
 	std::vector<GLuint> free_invalidated()
 	{
 		std::vector<GLuint> removed;
 		invalidated_resources.remove_if([&](auto &rtt)
 		{
-			if (rtt->deref_count >= 2)
+			if (rtt->unused_check_count() >= 2)
 			{
 				removed.push_back(rtt->id());
 				return true;
 			}
 
-			rtt->deref_count++;
 			return false;
 		});
 

rpcs3/Emu/RSX/VK/VKGSRender.cpp

@@ -2334,8 +2334,7 @@ void VKGSRender::advance_queued_frames()
 	//Remove stale framebuffers. Ref counted to prevent use-after-free
 	m_framebuffers_to_clean.remove_if([](std::unique_ptr<vk::framebuffer_holder>& fbo)
 	{
-		if (fbo->deref_count >= 2) return true;
-		fbo->deref_count++;
+		if (fbo->unused_check_count() >= 2) return true;
 		return false;
 	});
 
@@ -3102,17 +3101,23 @@ void VKGSRender::prepare_rtts(rsx::framebuffer_creation_context context)
 	auto vk_depth_format = (layout.zeta_address == 0) ? VK_FORMAT_UNDEFINED : vk::get_compatible_depth_surface_format(m_device->get_formats_support(), layout.depth_format);
 	m_current_renderpass_id = vk::get_render_pass_location(vk::get_compatible_surface_format(layout.color_format).first, vk_depth_format, (u8)m_draw_buffers.size());
 
-	//Search old framebuffers for this same configuration
+	// Search old framebuffers for this same configuration
 	bool framebuffer_found = false;
 	const auto fbo_width = rsx::apply_resolution_scale(layout.width, true);
 	const auto fbo_height = rsx::apply_resolution_scale(layout.height, true);
 
+	if (m_draw_fbo)
+	{
+		// Release old ref
+		m_draw_fbo->release();
+	}
+
 	for (auto &fbo : m_framebuffers_to_clean)
 	{
 		if (fbo->matches(bound_images, fbo_width, fbo_height))
 		{
 			m_draw_fbo.swap(fbo);
-			m_draw_fbo->reset_refs();
+			m_draw_fbo->add_ref();
 			framebuffer_found = true;
 			break;
 		}
@@ -3159,6 +3164,7 @@ void VKGSRender::prepare_rtts(rsx::framebuffer_creation_context context)
 			m_framebuffers_to_clean.push_back(std::move(m_draw_fbo));
 
 		m_draw_fbo.reset(new vk::framebuffer_holder(*m_device, current_render_pass, fbo_width, fbo_height, std::move(fbo_images)));
+		m_draw_fbo->add_ref();
 	}
 
 	set_viewport();
@@ -3199,10 +3205,10 @@ void VKGSRender::reinitialize_swapchain()
 		present(&ctx);
 	}
 
-	//Remove any old refs to the old images as they are about to be destroyed
-	m_framebuffers_to_clean.clear();
+	// Remove any old refs to the old images as they are about to be destroyed
+	//m_framebuffers_to_clean;
 
-	//Rebuild swapchain. Old swapchain destruction is handled by the init_swapchain call
+	// Rebuild swapchain. Old swapchain destruction is handled by the init_swapchain call
 	if (!m_swapchain->init(new_width, new_height))
 	{
 		LOG_WARNING(RSX, "Swapchain initialization failed. Request ignored [%dx%d]", new_width, new_height);
@@ -3539,7 +3545,7 @@ void VKGSRender::flip(int buffer, bool emu_flip)
 			if (fbo->attachments[0]->info.image == target_image)
 			{
 				direct_fbo.swap(fbo);
-				direct_fbo->reset_refs();
+				direct_fbo->add_ref();
 				m_framebuffers_to_clean.erase(It);
 				break;
 			}
@@ -3549,6 +3555,7 @@ void VKGSRender::flip(int buffer, bool emu_flip)
 		{
 			swap_image_view.push_back(std::make_unique<vk::image_view>(*m_device, target_image, VK_IMAGE_VIEW_TYPE_2D, m_swapchain->get_surface_format(), vk::default_component_map(), subres));
 			direct_fbo.reset(new vk::framebuffer_holder(*m_device, single_target_pass, m_client_width, m_client_height, std::move(swap_image_view)));
+			direct_fbo->add_ref();
 		}
 
 		if (has_overlay)
@@ -3588,6 +3595,8 @@ void VKGSRender::flip(int buffer, bool emu_flip)
 		}
 
 		vk::change_image_layout(*m_current_command_buffer, target_image, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, present_layout, subres);
+
+		direct_fbo->release();
 		m_framebuffers_to_clean.push_back(std::move(direct_fbo));
 	}
 

rpcs3/Emu/RSX/VK/VKOverlays.h

@@ -312,7 +312,7 @@ namespace vk
 				auto fbo = It->get();
 				if (fbo->matches(test, target->width(), target->height()))
 				{
-					fbo->deref_count = 0;
+					fbo->add_ref();
 					return fbo;
 				}
 			}
@@ -338,6 +338,7 @@ namespace vk
 			auto result = fbo.get();
 			framebuffer_resources.push_back(std::move(fbo));
 
+			result->add_ref();
 			return result;
 		}
 
@@ -381,7 +382,10 @@ namespace vk
 		void run(vk::command_buffer &cmd, u16 w, u16 h, vk::image* target, const std::vector<VkImageView>& src, VkRenderPass render_pass, std::list<std::unique_ptr<vk::framebuffer_holder>>& framebuffer_resources)
 		{
 			vk::framebuffer *fbo = get_framebuffer(target, render_pass, framebuffer_resources);
+
 			run(cmd, w, h, fbo, src, render_pass);
+
+			static_cast<vk::framebuffer_holder*>(fbo)->release();
 		}
 
 		void run(vk::command_buffer &cmd, u16 w, u16 h, vk::image* target, VkImageView src, VkRenderPass render_pass, std::list<std::unique_ptr<vk::framebuffer_holder>>& framebuffer_resources)

rpcs3/Emu/RSX/VK/VKRenderTargets.h

@@ -53,6 +53,11 @@ namespace vk
 			return !!(aspect() & VK_IMAGE_ASPECT_DEPTH_BIT);
 		}
 
+		void release_ref(vk::image* t) const override
+		{
+			static_cast<vk::render_target*>(t)->release();
+		}
+
 		bool matches_dimensions(u16 _width, u16 _height) const
 		{
 			//Use forward scaling to account for rounding and clamping errors
@@ -100,6 +105,8 @@ namespace vk
 			if (!rsx::pitch_compatible(this, src_texture))
 			{
 				LOG_TRACE(RSX, "Pitch mismatch, could not transfer inherited memory");
+
+				clear_rw_barrier();
 				return;
 			}
 
@@ -197,6 +204,7 @@ namespace rsx
 			rtt->queue_tag(address);
 			rtt->dirty = true;
 
+			rtt->add_ref();
 			return rtt;
 		}
 
@@ -239,6 +247,7 @@ namespace rsx
 			ds->queue_tag(address);
 			ds->dirty = true;
 
+			ds->add_ref();
 			return ds;
 		}
 
@@ -263,6 +272,8 @@ namespace rsx
 					VK_IMAGE_TILING_OPTIMAL,
 					ref->info.usage,
 					ref->info.flags));
+
+				sink->add_ref();
 			}
 
 			prev.target = sink.get();
@@ -300,9 +311,6 @@ namespace rsx
 		static void prepare_rtt_for_drawing(vk::command_buffer& cmd, vk::render_target *surface)
 		{
 			surface->change_layout(cmd, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL);
-
-			//Reset deref count
-			surface->deref_count = 0;
 			surface->frame_tag = 0;
 		}
 
@@ -314,9 +322,6 @@ namespace rsx
 		static void prepare_ds_for_drawing(vk::command_buffer& cmd, vk::render_target *surface)
 		{
 			surface->change_layout(cmd, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL);
-
-			//Reset deref count
-			surface->deref_count = 0;
 			surface->frame_tag = 0;
 		}
 
@@ -342,6 +347,9 @@ namespace rsx
 		{
 			surface->frame_tag = vk::get_current_frame_id();
 			if (!surface->frame_tag) surface->frame_tag = 1;
+
+			if (surface->old_contents) surface->clear_rw_barrier();
+			surface->release();
 		}
 
 		static void notify_surface_persist(const std::unique_ptr<vk::render_target> &surface)
@@ -349,9 +357,14 @@ namespace rsx
 			surface->save_aa_mode();
 		}
 
+		static void notify_surface_reused(const std::unique_ptr<vk::render_target> &surface)
+		{
+			surface->add_ref();
+		}
+
 		static bool rtt_has_format_width_height(const std::unique_ptr<vk::render_target> &rtt, surface_color_format format, size_t width, size_t height, bool check_refs=false)
 		{
-			if (check_refs && rtt->deref_count == 0) //Surface may still have read refs from data 'copy'
+			if (check_refs && rtt->has_refs()) // Surface may still have read refs from data 'copy'
 				return false;
 
 			VkFormat fmt = vk::get_compatible_surface_format(format).first;
@@ -365,7 +378,7 @@ namespace rsx
 
 		static bool ds_has_format_width_height(const std::unique_ptr<vk::render_target> &ds, surface_depth_format format, size_t width, size_t height, bool check_refs=false)
 		{
-			if (check_refs && ds->deref_count == 0) //Surface may still have read refs from data 'copy'
+			if (check_refs && ds->has_refs()) //Surface may still have read refs from data 'copy'
 				return false;
 
 			if (ds->matches_dimensions((u16)width, (u16)height))
@@ -418,8 +431,7 @@ namespace rsx
 	{
 		void destroy()
 		{
-			m_render_targets_storage.clear();
-			m_depth_stencil_storage.clear();
+			invalidate_all();
 			invalidated_resources.clear();
 		}
 
@@ -430,10 +442,9 @@ namespace rsx
 			{
 				verify(HERE), rtt->frame_tag != 0;
 
-				if (rtt->deref_count >= 2 && rtt->frame_tag < last_finished_frame)
+				if (rtt->unused_check_count() >= 2 && rtt->frame_tag < last_finished_frame)
 					return true;
 
-				rtt->deref_count++;
 				return false;
 			});
 		}

rpcs3/Emu/RSX/VK/VKTextureCache.h

@@ -555,7 +555,6 @@ namespace vk
 					verify(HERE), section.dst_z == 0;
 
 					u16 dst_x = section.dst_x, dst_y = section.dst_y;
-					auto xform = section.xform;
 					vk::image* _dst;
 
 					if (LIKELY(src_image->info.format == dst->info.format))

rpcs3/Emu/RSX/rsx_utils.h

@@ -33,11 +33,38 @@ namespace rsx
 	extern atomic_t<u64> g_rsx_shared_tag;
 
 	//Base for resources with reference counting
-	struct ref_counted
+	class ref_counted
 	{
-		u8 deref_count = 0;
+		atomic_t<s32> ref_count{ 0 }; // References held
+		atomic_t<u8> idle_time{ 0 };  // Number of times the resource has been tagged idle
 
-		void reset_refs() { deref_count = 0; }
+	public:
+		void add_ref()
+		{
+			ref_count++;
+			idle_time = 0;
+		}
+
+		void release()
+		{
+			ref_count--;
+		}
+
+		bool has_refs()
+		{
+			return (ref_count > 0);
+		}
+
+		// Returns number of times the resource has been checked without being used in-between checks
+		u8 unused_check_count()
+		{
+			if (ref_count)
+			{
+				return 0;
+			}
+
+			return idle_time++;
+		}
 	};
 
 	/**