merge with previous frames feedback

ref_vk/TODO.md

@@ -1,6 +1,5 @@
 # Real next
 >=E223
-- [ ] previous frame resources reference
 - [ ] what if new meatpipe has different image format for a creatable image?
 
 # Programmable render
@@ -498,3 +497,14 @@
 # 2023-01-22 E222
 - [x] refcount meatpipe created images
 - [x] rake yuri primary ray
+
+# 2023-01-28 E223
+- [x] previous frame resources reference
+		- specification:
+			- [x] I: prev_ -> resource flag + pair index
+			- [ ] II: new section in json
+		- internals:
+			- [x] I: create a new image for prev_, track its source; swap them each frame
+						Result is meh: too much indirection, hard to follow, many things need manual fragile updates.
+			- [ ] II: create tightly coupled image pair[2], read from [frame%2] write to [frame%2+1]
+			- [ ] III: like (I) but with more general resource management: i.e. resource object for prev_ points to its source

ref_vk/sebastian.py

@@ -405,6 +405,9 @@ class NameIndex:
 		self.__name_to_index[name] = index
 		return index
 
+	def __iter__(self):
+		return iter(self.__all)
+
 	def serialize(self, out):
 		out.writeArray(self.__all)
 
@@ -412,8 +415,9 @@ class NameIndex:
 class Resources:
 	def __init__(self):
 		self.__storage = NameIndex()
+		self.__map = None
 
-	def getIndex(self, name, node):
+	def getIndex(self, name, node, dependency = None):
 		index = self.__storage.getIndex(name)
 
 		if index >= 0:
@@ -421,25 +425,54 @@ class Resources:
 			res.checkSameType(node)
 			return index
 
-		return self.__storage.put(name, self.Resource(name, node))
+		return self.__storage.put(name, self.Resource(name, node, dependency))
+
+	def getMappedIndex(self, index):
+		return self.__map.get(index, index)
+
+	def __sortDependencies(self):
+		assert(not self.__map)
+		self.__map = dict()
+
+		for i, r in enumerate(self.__storage):
+			dep = r.dependency
+			if not dep:
+				continue
+
+			assert(dep != i)
+			if dep < i:
+				continue
+
+			assert(i not in self.__map)
+			assert(dep not in self.__map)
+
+			self.__map[i] = dep
+			self.__map[dep] = i
+			r.dependency = i
 
 	def serialize(self, out):
+		self.__sortDependencies()
 		self.__storage.serialize(out)
 
 	class Resource:
-		def __init__(self, name, node):
+		def __init__(self, name, node, dependency = None):
 			self.__name = name
-			self.__type = node.getType()
-
-			#TODO: count, etc
+			self.__type = node.getType() if node else None
+			self.dependency = dependency
 
 		def checkSameType(self, node):
+			if not self.__type:
+				self.__type = node.getType()
+				return
+
 			if self.__type != node.getType():
 				raise Exception('Conflicting types for resource "%s": %s != %s' % (self.__name, self.__type, type))
 
 		def serialize(self, out):
 			out.writeString(self.__name)
 			self.__type.serialize(out)
+			if self.__type.is_image:
+				out.writeU32((self.dependency + 1) if self.dependency is not None else 0)
 
 resources = Resources()
 
@@ -462,17 +495,25 @@ class Binding:
 	STAGE_MESH_BIT_NV = 0x00000080
 	STAGE_SUBPASS_SHADING_BIT_HUAWEI = 0x00004000
 
+	# TODO same values for meatpipe.c too
 	WRITE_BIT = 0x80000000
+	CREATE_BIT = 0x40000000
 
 	def __init__(self, node):
 		self.write = node.name.startswith('out_')
+		self.create = self.write
 		self.index = node.binding
 		self.descriptor_set = node.descriptor_set
 		self.stages = 0
 
-		resource_name = removeprefix(node.name, 'out_')
-		self.__resource_index = resources.getIndex(resource_name, node)
+		prev_name = removeprefix(node.name, 'prev_') if node.name.startswith('prev_') else None
+		prev_resource_index = resources.getIndex(prev_name, None) if prev_name else None
 
+		resource_name = removeprefix(node.name, 'out_') if self.write else node.name
+		self.__resource_index = resources.getIndex(resource_name, node, prev_resource_index)
+
+		if prev_resource_index is not None:
+			self.create = True
 
 		assert(self.descriptor_set >= 0)
 		assert(self.descriptor_set < 255)
@@ -506,9 +547,13 @@ class Binding:
 		return self.__str__()
 
 	def serialize(self, out):
-		header = (Binding.WRITE_BIT if self.write else 0) | (self.descriptor_set << 8) | self.index
+		header = (self.descriptor_set << 8) | self.index
+		if self.write:
+			header |= Binding.WRITE_BIT
+		if self.create:
+			header |= Binding.CREATE_BIT
 		out.writeU32(header)
-		out.writeU32(self.__resource_index)
+		out.writeU32(resources.getMappedIndex(self.__resource_index))
 		out.writeU32(self.stages)
 
 class Shader:

ref_vk/shaders/denoiser.comp

@@ -20,6 +20,8 @@ layout(set = 0, binding = 6, rgba16f) uniform readonly image2D emissive;
 layout(set = 0, binding = 7, rgba32f) uniform readonly image2D position_t;
 layout(set = 0, binding = 8, rgba32f) uniform readonly image2D prev_position_t;
 
+layout(set = 0, binding = 9, rgba16f) uniform readonly image2D prev_dest;
+
 //layout(set = 0, binding = 7, rgba32f) uniform readonly image2D position_t;
 //layout(set = 0, binding = 8, rgba16f) uniform readonly image2D normals_gs;
 
@@ -210,6 +212,9 @@ void main() {
 	// DEBUG motion vectors
 	//colour = vec3(length(imageLoad(position_t, pix).rgb - imageLoad(prev_position_t, pix).rgb));
 
+	const vec4 prev_colour = imageLoad(prev_dest, pix);
+	colour = mix(colour, prev_colour.rgb, vec3(.9));
+
 	imageStore(out_dest, pix, vec4(colour, 0.));
 	//imageStore(out_dest, pix, imageLoad(light_poly, pix));
 }

ref_vk/vk_meatpipe.c

@@ -190,7 +190,9 @@ static qboolean readBindings(load_context_t *ctx, VkDescriptorSetLayoutBinding *
 		vk_meatpipe_resource_t *res = ctx->meatpipe.resources + res_index;
 
 #define BINDING_WRITE_BIT 0x80000000u
+#define BINDING_CREATE_BIT 0x40000000u
 		const qboolean write = !!(header & BINDING_WRITE_BIT);
+		const qboolean create = !!(header & BINDING_CREATE_BIT);
 		const uint32_t descriptor_set = (header >> 8) & 0xffu;
 		const uint32_t binding = header & 0xffu;
 
@@ -216,7 +218,10 @@ static qboolean readBindings(load_context_t *ctx, VkDescriptorSetLayoutBinding *
 		pass->resource_map[i] = res_index;
 
 		if (write)
-			res->flags |= MEATPIPE_RES_WRITE | MEATPIPE_RES_CREATE; // TODO distinguish between write and create
+			res->flags |= MEATPIPE_RES_WRITE;
+
+		if (create)
+			res->flags |= MEATPIPE_RES_CREATE;
 
 		gEngine.Con_Reportf("Binding %d: %s ds=%d b=%d s=%08x res=%d type=%d write=%d\n",
 			i, name, descriptor_set, binding, stages, res_index, res->descriptor_type, write);
@@ -299,6 +304,7 @@ static qboolean readResources(load_context_t *ctx) {
 
 		if (is_image) {
 			res->image_format = READ_U32("Couldn't read image format for res %d:%s", i, res->name);
+			res->prev_frame_index = READ_U32("Couldn't read resource %d:%s previous frame index", i, res->name);
 		}
 
 		gEngine.Con_Reportf("Resource %d:%s = %08x is_image=%d image_format=%08x count=%d\n",

ref_vk/vk_meatpipe.h

@@ -16,6 +16,9 @@ typedef struct {
 	union {
 		uint32_t image_format;
 	};
+
+	// If this image is supposed to be read from previous frame
+	int prev_frame_index;
 } vk_meatpipe_resource_t;
 
 struct vk_meatpipe_pass_s;

ref_vk/vk_rtx.c

@@ -64,6 +64,7 @@ typedef struct {
 		vk_resource_t resource;
 		xvk_image_t image;
 		int refcount;
+		int source_index;
 } rt_resource_t;
 
 static struct {
@@ -236,10 +237,40 @@ static void performTracing(VkCommandBuffer cmdbuf, const perform_tracing_args_t*
 			0, 0, NULL, ARRAYSIZE(bmb), bmb, 0, NULL);
 	}
 
+	// Transfer previous frames before they had a chance of their resource-barrier metadata overwritten (as there's no guaranteed order for them)
+	for (int i = ExternalResource_COUNT; i < MAX_RESOURCES; ++i) {
+		rt_resource_t* const res = g_rtx.res + i;
+		if (!res->name[0] || !res->image.image || res->source_index <= 0)
+			continue;
+
+		ASSERT(res->source_index <= COUNTOF(g_rtx.res));
+		rt_resource_t *const src = g_rtx.res + res->source_index - 1;
+
+		// Swap resources
+		const vk_resource_t tmp_res = res->resource;
+		const xvk_image_t tmp_img = res->image;
+
+		res->resource = src->resource;
+		res->image = src->image;
+
+		// TODO this is slightly incorrect, as they technically can have different resource->type values
+		src->resource = tmp_res;
+		src->image = tmp_img;
+
+		// If there was no initial state, prepare it. (this should happen only for the first frame)
+		if (res->resource.write.pipelines == 0) {
+			// TODO is there a better way? Can image be cleared w/o explicit clear op?
+			R_VkImageClear( cmdbuf, res->image.image );
+			res->resource.write.pipelines = VK_PIPELINE_STAGE_TRANSFER_BIT;
+			res->resource.write.image_layout = VK_IMAGE_LAYOUT_GENERAL;
+			res->resource.write.access_mask = VK_ACCESS_TRANSFER_WRITE_BIT;
+		}
+	}
+
 	// Clear intra-frame resources
 	for (int i = ExternalResource_COUNT; i < MAX_RESOURCES; ++i) {
 		rt_resource_t* const res = g_rtx.res + i;
-		if (!res->name[0] || !res->image.image)
+		if (!res->name[0] || !res->image.image || res->source_index > 0)
 			continue;
 
 		res->resource.read = res->resource.write = (ray_resource_state_t){0};
@@ -282,6 +313,20 @@ static void performTracing(VkCommandBuffer cmdbuf, const perform_tracing_args_t*
 			0, 0, NULL, ARRAYSIZE(bmb), bmb, 0, NULL);
 	}
 
+	// Update image resource links after the prev_-related swap above
+	// TODO Preserve the indexes somewhere to avoid searching
+	for (int i = 0; i < g_rtx.mainpipe->resources_count; ++i) {
+		const vk_meatpipe_resource_t *mr = g_rtx.mainpipe->resources + i;
+		const int index = findResource(mr->name);
+		ASSERT(index >= 0);
+		ASSERT(index < MAX_RESOURCES);
+		rt_resource_t *const res = g_rtx.res + index;
+		const qboolean create = !!(mr->flags & MEATPIPE_RES_CREATE);
+		if (create && mr->descriptor_type == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE)
+			//ASSERT(g_rtx.mainpipe_resources[i]->value.image_object == &res->image);
+			g_rtx.mainpipe_resources[i]->value.image_object = &res->image;
+	}
+
 	R_VkMeatpipePerform(g_rtx.mainpipe, cmdbuf, (vk_meatpipe_perfrom_args_t) {
 		.frame_set_slot = args->frame_index,
 		.width = FRAME_WIDTH,
@@ -309,6 +354,8 @@ static void performTracing(VkCommandBuffer cmdbuf, const perform_tracing_args_t*
 		};
 
 		R_VkImageBlit( cmdbuf, &blit_args );
+
+		g_rtx.mainpipe_out->resource.write.image_layout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL;
 	}
 	DEBUG_END(cmdbuf);
 }
@@ -368,7 +415,6 @@ static void reloadMainpipe(void) {
 
 		const qboolean create = !!(mr->flags & MEATPIPE_RES_CREATE);
 
-		// FIXME no assert, just complain
 		if (create && mr->descriptor_type != VK_DESCRIPTOR_TYPE_STORAGE_IMAGE) {
 			gEngine.Con_Printf(S_ERROR "Only storage image creation is supported for meatpipes\n");
 			goto fail;
@@ -398,7 +444,9 @@ static void reloadMainpipe(void) {
 					.layers = 1,
 					.format = mr->image_format,
 					.tiling = VK_IMAGE_TILING_OPTIMAL,
-					.usage = VK_IMAGE_USAGE_STORAGE_BIT | (output ? VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT : 0),
+					// TODO figure out how to detect this need properly. prev_dest is not defined as "output"
+					//.usage = VK_IMAGE_USAGE_STORAGE_BIT | (output ? VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT : 0),
+					.usage = VK_IMAGE_USAGE_STORAGE_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT,
 					.has_alpha = true,
 					.is_cubemap = false,
 				};
@@ -428,6 +476,28 @@ static void reloadMainpipe(void) {
 		goto fail;
 	}
 
+	// Resolve prev_ frame resources
+	for (int i = 0; i < newpipe->resources_count; ++i) {
+		const vk_meatpipe_resource_t *mr = newpipe->resources + i;
+		if (mr->prev_frame_index <= 0)
+			continue;
+
+		ASSERT(mr->prev_frame_index < newpipe->resources_count);
+
+		const int index = findResource(mr->name);
+		ASSERT(index >= 0);
+
+		const vk_meatpipe_resource_t *pr = newpipe->resources + (mr->prev_frame_index - 1);
+
+		const int dest_index = findResource(pr->name);
+		if (dest_index < 0) {
+			gEngine.Con_Printf(S_ERROR "Couldn't find prev_ resource/slot %s for resource %s\n", pr->name, mr->name);
+			goto fail;
+		}
+
+		g_rtx.res[index].source_index = dest_index + 1;
+	}
+
 	// Loading successful
 	// Update refcounts
 	for (int i = 0; i < newpipe->resources_count; ++i) {