Add fixes to issues raised in pr review:

- removed dynamic_rendering_attachment
- added declare_dynamic(_for) functions to avk::attachment which create dynamic rendering attachments
- changed graphics_pipeline_t::renderpass_pointer() back to renderpass_reference() which now returns a reference wrapper

README.md

@@ -79,7 +79,7 @@ avk::framebuffer framebuffer = myRoot.create_framebuffer(...);
 avk::semaphore imageAvailableSemaphore = ...;
 
 mRoot.record({
-      avk::command::render_pass(*graphicsPipeline->renderpass_pointer().value(), framebuffer.as_reference(), {
+      avk::command::render_pass(graphicsPipeline->renderpass_reference().value(), framebuffer.as_reference(), {
           avk::command::bind_pipeline(graphicsPipeline.as_reference()),
           avk::command::draw(3u, 1u, 0u, 0u)
       })

include/avk/attachment.hpp

@@ -3,39 +3,6 @@
 
 namespace avk
 {
-
-	/** Describes a dynamic rendering attachment. It can only be used with pipeline that has dynamic rendering enabled.
-	 *  It has fewever parameters than regular attachment since some of its values (load/store ops etc...) are set
-	 *  when starting the dynamic render pass as opposed to being declared beforehand.
-	 *	It can describe color attachments as well as depth/stencil attachments
-	 *	and holds some additional config parameters for these attachments.
-	 */
-	struct dynamic_rendering_attachment
-	{
-		/**	Declare multisampled format of an attachment for a dynamic rendering pipeline
-		 *	@param	aFormatAndSamples	Multisampled format definition: A tuple with the format of the attachment in its first element, and with the number of samples in its second element.
-		 */
-		static dynamic_rendering_attachment declare(std::tuple<vk::Format, vk::SampleCountFlagBits> aFormatAndSamples);
-		/**	Declare format of an attachment for a dynamic rendering pipeline
-		 *	@param	aFormat	 The format of the attachment
-		 */
-		static dynamic_rendering_attachment declare(vk::Format aFormat);
-
-		/**	Declare format of an attachment for a dynamic rendering pipeline
-		 *	@param	aImageView			The format of the attachment is copied from the given image view.
-		 */
-		static dynamic_rendering_attachment declare_for(const image_view_t& aImageView);
-
-		/** The color/depth/stencil format of the attachment */
-		auto format() const { return mFormat; }
-		/** True if the sample count is greater than 1 */
-		bool is_multisampled() const { return mSampleCount != vk::SampleCountFlagBits::e1; }
-		/** The sample count for this attachment. */
-		auto sample_count() const { return mSampleCount; }
-
-		vk::Format mFormat;
-		vk::SampleCountFlagBits mSampleCount;
-	};
 	/** Describes an attachment to a framebuffer or a renderpass.
 	 *	It can describe color attachments as well as depth/stencil attachments
 	 *	and holds some additional config parameters for these attachments.
@@ -93,6 +60,27 @@ namespace avk
 		 */
 		static attachment declare_for(const image_view_t& aImageView, attachment_load_config aLoadOp, subpass_usages aUsageInSubpasses, attachment_store_config aStoreOp);
 
+		/**	Declare multisampled format of an attachment for a dynamic rendering pipeline. This attachment can only be used with pipeline that has dynamic rendering enabled.
+	 	 *  It has fewever parameters than regular attachment since some of its values (load/store ops etc...) are set when starting the dynamic render pass
+		 *  as opposed to being declared beforehand..
+		 *	@param	aFormatAndSamples	Multisampled format definition: A tuple with the format of the attachment in its first element, and with the number of samples in its second element.
+		 */
+		static attachment declare_dynamic(std::tuple<vk::Format, vk::SampleCountFlagBits> aFormatAndSamples);
+
+		/**	Declare multisampled format of an attachment for a dynamic rendering pipeline. This attachment can only be used with pipeline that has dynamic rendering enabled.
+	 	 *  It has fewever parameters than regular attachment since some of its values (load/store ops etc...) are set when starting the dynamic render pass
+		 *  as opposed to being declared beforehand..
+		 *	@param	aFormat	 The format of the attachment
+		 */
+		static attachment declare_dynamic(vk::Format aFormat);
+
+		/**	Declare multisampled format of an attachment for a dynamic rendering pipeline. This attachment can only be used with pipeline that has dynamic rendering enabled.
+	 	 *  It has fewever parameters than regular attachment since some of its values (load/store ops etc...) are set when starting the dynamic render pass
+		 *  as opposed to being declared beforehand..
+		 *	@param	aImageView			The format of the attachment is copied from the given image view.
+		 */
+		static attachment declare_dynamic_for(const image_view_t& aImageView);
+
 		attachment& set_clear_color(std::array<float, 4> aColor) { mColorClearValue = aColor; return *this; }
 		attachment& set_depth_clear_value(float aDepthClear) { mDepthClearValue = aDepthClear; return *this; }
 		attachment& set_stencil_clear_value(uint32_t aStencilClear) { mStencilClearValue = aStencilClear; return *this; }
@@ -121,6 +109,8 @@ namespace avk
 		auto sample_count() const { return mSampleCount; }
 		/** True if a multisample resolve pass shall be set up. */
 		auto is_to_be_resolved() const { return mSubpassUsages.contains_resolve(); }
+		/** True if this attachment is declared for dynamic rendering pipelines ie. using one of the dynamic declare functions*/
+		bool is_for_dynamic_rendering() const { return mDynamicRenderingAttachment; }
 
 		/** Returns the stencil load operation */
 		auto get_stencil_load_op() const { return mStencilLoadOperation.value_or(mLoadOperation); }
@@ -141,5 +131,6 @@ namespace avk
 		std::array<float, 4> mColorClearValue;
 		float mDepthClearValue;
 		uint32_t mStencilClearValue;
+		bool mDynamicRenderingAttachment;
 	};
 }

include/avk/avk.hpp

@@ -762,18 +762,26 @@ namespace avk
 			std::function<void(graphics_pipeline_t&)> alterConfigFunction;
 			graphics_pipeline_config config;
 			add_config(config, renderPassAttachments, alterConfigFunction, std::move(args)...);
+			bool hasRenderPassAttachments = false;
+			bool hasDynamicRenderingAttachments = false;
+			for(const auto & attachment : renderPassAttachments)
+			{
+				if(attachment.is_for_dynamic_rendering())
+				{
+					hasDynamicRenderingAttachments = true;
+				} else {
+					hasRenderPassAttachments = true;
+				}
+			}
 
 			const bool hasValidRenderPass = config.mRenderPassSubpass.has_value() && static_cast<bool>(std::get<renderpass>(*config.mRenderPassSubpass)->handle());
-			const bool hasRenderPassAttachments = renderPassAttachments.size() > 0;
 			const bool isDynamicRenderingSet = config.mDynamicRendering == avk::cfg::dynamic_rendering::enabled;
-			// .has_value() should be enough since I don't fill in the optional unless there was dynamic_rendering_attachment provided
-			const bool hasDynamicRenderingAttachments = config.mDynamicRenderingAttachments.has_value();
 			// Check all invalid configurations when dynamic rendering is set
 			if (isDynamicRenderingSet )
 			{
 				if(hasValidRenderPass) 		 		{ throw avk::runtime_error("Dynamic rendering does not accept renderpasses! They are set dynamically during rendering!"); }
-				if(hasRenderPassAttachments) 		{ throw avk::runtime_error("Usage of avk::attachment is not allowed when dynamic rendering is enabled! Use avk::dynamic_rendering_attachment instead!"); }
-				if(!hasDynamicRenderingAttachments) { throw avk::runtime_error("Dynamic rendering enabled but no avk::dynamic_rendering_attachments provided! Please provide at least one attachment!"); }
+				if(hasRenderPassAttachments) 		{ throw avk::runtime_error("Only avk::attachments created by declare_dynamic(_for) functions are allowed when dynamic rendering is enabled!"); }
+				if(!hasDynamicRenderingAttachments) { throw avk::runtime_error("Dynamic rendering enabled but no avk::attachmenst created by declare_dynamic(_for) functions provided! Please provide at least one attachment!"); }
 			} 
 			// Check all invalid configurations when normal rendering (with renderpasses) is used
 			else 
@@ -785,6 +793,8 @@ namespace avk
 			// ^ that was the sanity check. See if we have to build the renderpass from the attachments:
 			if (hasRenderPassAttachments) {
 				add_config(config, renderPassAttachments, alterConfigFunction, create_renderpass(std::move(renderPassAttachments)));
+			} else {
+				config.mDynamicRenderingAttachments = std::move(renderPassAttachments);
 			}
 
 			// 2. CREATE PIPELINE according to the config

include/avk/graphics_pipeline.hpp

@@ -1,5 +1,6 @@
 #pragma once
 #include "avk/avk.hpp"
+#include <functional>
 
 namespace avk
 {
@@ -17,10 +18,9 @@ namespace avk
 		~graphics_pipeline_t() = default;
 
 		[[nodiscard]] auto renderpass() const { return mRenderPass; }
-		// TODO(msakmary) I can also keep the consistent naming aka renderpass_reference() return std::optional<std::reference_wrapper>> I feel like this is way cleaner?
-		[[nodiscard]] auto renderpass_pointer() const -> std::optional<const avk::renderpass_t *>
+		[[nodiscard]] auto renderpass_reference() const -> std::optional<std::reference_wrapper<const avk::renderpass_t>>
 		{
-			if(mRenderPass.has_value()) { return &(mRenderPass.value().get()); }
+			if(mRenderPass.has_value()) { return std::cref(mRenderPass.value().get()); }
 			else 						{ return std::nullopt; }
 		} 
 		// TODO(msakmary) Perhaps I just return std::optional<vk::RenderPass> here? It would probably be more readable then declval

include/avk/graphics_pipeline_config.hpp

@@ -615,8 +615,8 @@ namespace avk
 		~graphics_pipeline_config() = default;
 
 		cfg::pipeline_settings mPipelineSettings; // TODO: Handle settings!
-		std::optional<std::vector<avk::dynamic_rendering_attachment>> mDynamicRenderingAttachments;
 		std::optional<std::tuple<renderpass, uint32_t>> mRenderPassSubpass;
+		std::optional<std::vector<avk::attachment>> mDynamicRenderingAttachments;
 		std::vector<input_binding_to_location_mapping> mInputBindingLocations;
 		cfg::primitive_topology mPrimitiveTopology;
 		std::vector<shader_info> mShaderInfos;
@@ -681,19 +681,6 @@ namespace avk
 		add_config(aConfig, aAttachments, aFunc, std::move(args)...);
 	}
 
-	// Add a dynamic rendering attachment which are used when dynamic_rendering is enabled for this pipeline
-	template <typename... Ts>
-	void add_config(graphics_pipeline_config& aConfig, std::vector<avk::attachment>& aAttachments, std::function<void(graphics_pipeline_t&)>& aFunc, avk::dynamic_rendering_attachment aDynAttachment, Ts... args)
-	{
-		if(aConfig.mDynamicRenderingAttachments.has_value())
-		{
-			aConfig.mDynamicRenderingAttachments.value().push_back(aDynAttachment);
-		} else {
-			aConfig.mDynamicRenderingAttachments = std::vector{aDynAttachment};
-		}
-		add_config(aConfig, aAttachments, aFunc, std::move(args)...);
-	}
-
 	// Add a renderpass attachment to the (temporary) attachments vector and later build renderpass from them
 	template <typename... Ts>
 	void add_config(graphics_pipeline_config& aConfig, std::vector<avk::attachment>& aAttachments, std::function<void(graphics_pipeline_t&)>& aFunc, avk::attachment aAttachment, Ts... args)

src/avk.cpp

@@ -1418,25 +1418,6 @@ namespace avk
 #pragma endregion
 
 #pragma region dynamic rendering attachment definitions
-	dynamic_rendering_attachment dynamic_rendering_attachment::declare(std::tuple<vk::Format, vk::SampleCountFlagBits> aFormatAndSamples)
-	{
-		return dynamic_rendering_attachment{
-			std::get<vk::Format>(aFormatAndSamples),
-			std::get<vk::SampleCountFlagBits>(aFormatAndSamples),
-		};
-	}
-
-	dynamic_rendering_attachment dynamic_rendering_attachment::declare(vk::Format aFormat)
-	{
-		return declare({aFormat, vk::SampleCountFlagBits::e1});
-	}
-
-	dynamic_rendering_attachment dynamic_rendering_attachment::declare_for(const image_view_t& aImageView)
-	{
-		const auto& imageConfig = aImageView.get_image().create_info();
-		const auto format = imageConfig.format;
-		return declare(format);
-	}
 #pragma endregion
 
 #pragma region attachment definitions
@@ -1449,7 +1430,8 @@ namespace avk
 			{},      {},
 			std::move(aUsageInSubpasses),
 			{ 0.0, 0.0, 0.0, 0.0 },
-			1.0f, 0u
+			1.0f, 0u,
+			false
 		};
 	}
 
@@ -1465,6 +1447,28 @@ namespace avk
 		auto result = declare({format, imageConfig.samples}, aLoadOp, std::move(aUsageInSubpasses), aStoreOp);
 		return result;
 	}
+
+	attachment attachment::declare_dynamic(std::tuple<vk::Format, vk::SampleCountFlagBits> aFormatAndSamples)
+	{
+		return attachment{
+			.mFormat = std::get<vk::Format>(aFormatAndSamples),
+			.mSampleCount = std::get<vk::SampleCountFlagBits>(aFormatAndSamples),
+			.mSubpassUsages = subpass_usages(subpass_usage_type::create_unused()),
+			.mDynamicRenderingAttachment = true
+		};
+	}
+
+	attachment attachment::declare_dynamic(vk::Format aFormat)
+	{
+		return declare_dynamic({aFormat, vk::SampleCountFlagBits::e1});
+	}
+
+	attachment attachment::declare_dynamic_for(const image_view_t& aImageView)
+	{
+		const auto& imageConfig = aImageView.get_image().create_info();
+		const auto format = imageConfig.format;
+		return declare_dynamic(format);
+	}
 #pragma endregion
 
 #pragma region acceleration structure definitions
@@ -4378,7 +4382,7 @@ namespace avk
 		{
 			aPreparedPipeline.mMultisampleStateCreateInfo
 				.setRasterizationSamples(
-					aPreparedPipeline.renderpass_pointer().value()->num_samples_for_subpass(aPreparedPipeline.subpass_id().value()))
+					aPreparedPipeline.renderpass_reference().value().get().num_samples_for_subpass(aPreparedPipeline.subpass_id().value()))
 				.setPSampleMask(nullptr);
 		}
 
@@ -4393,7 +4397,7 @@ namespace avk
 		assert(static_cast<bool>(aPreparedPipeline.layout_handle()));
 
 		const void * pNext = dynamicRenderingEnabled ? &(aPreparedPipeline.mRenderingCreateInfo.value()) : nullptr;
-		VkRenderPass render_pass{};
+		VkRenderPass render_pass = VK_NULL_HANDLE;
 		if(!dynamicRenderingEnabled)
 		{
 			render_pass = (aPreparedPipeline.mRenderPass.value())->handle();
@@ -4488,7 +4492,7 @@ namespace avk
 		const bool dynamicRenderingEnabled = aConfig.mDynamicRendering == avk::cfg::dynamic_rendering::enabled;
 
 		{
-			if(!dynamic_rendering_enabled)
+			if(!dynamicRenderingEnabled)
 			{
 				assert(aConfig.mRenderPassSubpass.has_value());
 				auto [rp, sp] = std::move(aConfig.mRenderPassSubpass.value());
@@ -4678,8 +4682,8 @@ namespace avk
 			}
 
 			// Iterate over all color target attachments and set a color blending config 
-			size_t blending_config_num;
-			if (!dynamic_rendering_enabled)
+			size_t blendingConfigNum;
+			if (!dynamicRenderingEnabled)
 			{
 				const auto & renderPassVal = result.mRenderPass.value();
 				if (result.subpass_id() >= renderPassVal->attachment_descriptions().size()) {
@@ -4690,24 +4694,24 @@ namespace avk
 						+ std::to_string(result.subpass_id().value()) +
 						") indicates. I.e. the subpass index is out of bounds.");
 				}
-				blending_config_num = renderPassVal->color_attachments_for_subpass(result.subpass_id().value()).size(); /////////////////// TODO: (doublecheck or) FIX this section (after renderpass refactoring)
+				blendingConfigNum = renderPassVal->color_attachments_for_subpass(result.subpass_id().value()).size(); /////////////////// TODO: (doublecheck or) FIX this section (after renderpass refactoring)
 			} 
 			// Renderpasses and Subpasses are not supported when dynamic rendering is enabled
 			// Instead we read size of the dynamic_rendering_attachments provided
 			else 
 			{
 				blendingConfigNum = 0;
 
-				for(const auto & dyn_rendering_attachment : aConfig.mDynamicRenderingAttachments.value())
+				for(const auto & dynRenderingAttachment : aConfig.mDynamicRenderingAttachments.value())
 				{
-					if(!is_depth_format(dyn_rendering_attachment.format()))
+					if(!is_depth_format(dynRenderingAttachment.format()))
 					{
-						blending_config_num++;
+						blendingConfigNum++;
 					}
 				}
 			}
-			result.mBlendingConfigsForColorAttachments.reserve(blending_config_num); // Important! Otherwise the vector might realloc and .data() will become invalid!
-			for (size_t i = 0; i < blending_config_num; ++i) {
+			result.mBlendingConfigsForColorAttachments.reserve(blendingConfigNum); // Important! Otherwise the vector might realloc and .data() will become invalid!
+			for (size_t i = 0; i < blendingConfigNum; ++i) {
 				// Do we have a specific blending config for color attachment i?
 #if defined(_MSC_VER) && _MSC_VER < 1930
 				auto configForI = aConfig.mColorBlendingPerAttachment
@@ -4755,7 +4759,7 @@ namespace avk
 		// TODO: Can the settings be inferred from the renderpass' color attachments (as they are right now)? If they can't, how to handle this situation?
 		{ /////////////////// TODO: FIX this section (after renderpass refactoring)
 			vk::SampleCountFlagBits numSamples = vk::SampleCountFlagBits::e1;
-			if(!dynamic_rendering_enabled)
+			if(!dynamicRenderingEnabled)
 			{
 				numSamples = result.mRenderPass.value()->num_samples_for_subpass(result.subpass_id().value());
 			} else {
@@ -4865,19 +4869,19 @@ namespace avk
 			.setPPushConstantRanges(result.mPushConstantRanges.data());
 
 		// 15. Set Rendering info if dynamic rendering is enabled
-		if(dynamic_rendering_enabled)
+		if(dynamicRenderingEnabled)
 		{
 			std::vector<vk::Format> depth_attachments;
 			std::vector<vk::Format> stencil_attachments;
-			for(const auto & dynamic_rendering_attachment : aConfig.mDynamicRenderingAttachments.value())
+			for(const auto & dynamicRenderingAttachment : aConfig.mDynamicRenderingAttachments.value())
 			{
-				if(is_depth_format(dynamic_rendering_attachment.format()))
+				if(is_depth_format(dynamicRenderingAttachment.format()))
 				{
-					depth_attachments.push_back(dynamic_rendering_attachment.format());
-				} else if (is_stencil_format(dynamic_rendering_attachment.format())) {
-					stencil_attachments.push_back(dynamic_rendering_attachment.format());
+					depth_attachments.push_back(dynamicRenderingAttachment.format());
+				} else if (is_stencil_format(dynamicRenderingAttachment.format())) {
+					stencil_attachments.push_back(dynamicRenderingAttachment.format());
 				} else {
-					result.mDynamicRenderingColorFormats.push_back(dynamic_rendering_attachment.format());
+					result.mDynamicRenderingColorFormats.push_back(dynamicRenderingAttachment.format());
 				}
 			}
 			if(depth_attachments.size() > 1)   { throw avk::runtime_error("Provided multiple depth attachments! Only one is supported!"); }
@@ -4895,7 +4899,7 @@ namespace avk
 			aAlterConfigBeforeCreation(result);
 		}
 
-		assert (aConfig.mRenderPassSubpass.has_value() || dynamic_rendering_enabled);
+		assert (aConfig.mRenderPassSubpass.has_value() || dynamicRenderingEnabled);
 		rewire_config_and_create_graphics_pipeline(result);
 		return result;
 	}