Skip to content

Commit

Permalink
[Vulkan] Fix an issue with how render passes are specified.
Browse files Browse the repository at this point in the history 
* When creating a VkRenderPass, don't combine VkAttachmentLoadOp.Load
  and VkImageLayout.Undefined. Instead, have two different render
  passes: a "clear" one, and a "don't care" one.
* This issue doesn't cause any validation errors or visible defects, but
  the Vulkan spec seems to imply that it shouldn't be done.
* This helps avoid some errors when running on MoltenVK.
  • Loading branch information
@mellinoe
mellinoe committed Mar 18, 2018
1 parent 4e25323 commit 88c8a87
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 45 deletions.
53 changes: 22 additions & 31 deletions src/Veldrid/Vk/VkCommandList.cs
View file
Expand Up @@ -42,6 +42,9 @@ internal unsafe class VkCommandList : CommandList
private readonly Queue<VkCommandBuffer> _availableCommandBuffers = new Queue<VkCommandBuffer>();
private readonly List<VkCommandBuffer> _submittedCommandBuffers = new List<VkCommandBuffer>();

// Render pass cycle state
private bool _newFramebuffer;

public VkCommandPool CommandPool => _pool;
public VkCommandBuffer CommandBuffer => _cb;
public uint SubmittedCommandBufferCount { get; private set; }
Expand Down Expand Up @@ -173,14 +176,12 @@ protected override void ClearDepthStencilCore(float depth, byte stencil)

if (_activeRenderPass != VkRenderPass.Null)
{
VkImageAspectFlags aspectMask = VkImageAspectFlags.Depth;
if (FormatHelpers.IsStencilFormat(_currentFramebuffer.DepthTarget.Value.Target.Format))
{
aspectMask |= VkImageAspectFlags.Stencil;
}
VkImageAspectFlags aspect = FormatHelpers.IsStencilFormat(_currentFramebuffer.DepthTarget.Value.Target.Format)
? VkImageAspectFlags.Depth | VkImageAspectFlags.Stencil
: VkImageAspectFlags.Depth;
VkClearAttachment clearAttachment = new VkClearAttachment
{
aspectMask = aspectMask,
aspectMask = aspect,
clearValue = clearValue
};

Expand Down Expand Up @@ -379,14 +380,14 @@ public override void End()
_commandBufferBegun = false;
_commandBufferEnded = true;

if (_activeRenderPass != VkRenderPass.Null)
if (!_currentFramebufferEverActive && _currentFramebuffer != null)
{
EndCurrentRenderPass();
BeginCurrentRenderPass();
}
else if (!_currentFramebufferEverActive && _currentFramebuffer != null)
if (_activeRenderPass != VkRenderPass.Null)
{
BeginCurrentRenderPass();
EndCurrentRenderPass();
_currentFramebuffer.TransitionToFinalLayout(_cb);
}

vkEndCommandBuffer(_cb);
Expand All @@ -395,6 +396,7 @@ public override void End()

protected override void SetFramebufferCore(Framebuffer fb)
{
_newFramebuffer = true;
if (_activeRenderPass.Handle != VkRenderPass.Null)
{
EndCurrentRenderPass();
Expand All @@ -406,6 +408,11 @@ protected override void SetFramebufferCore(Framebuffer fb)
EndCurrentRenderPass();
}

if (_currentFramebuffer != null)
{
_currentFramebuffer.TransitionToFinalLayout(_cb);
}

VkFramebufferBase vkFB = Util.AssertSubtype<Framebuffer, VkFramebufferBase>(fb);
_currentFramebuffer = vkFB;
_currentFramebufferEverActive = false;
Expand Down Expand Up @@ -461,9 +468,11 @@ private void BeginCurrentRenderPass()

if (!haveAnyAttachments || !haveAllClearValues)
{
renderPassBI.renderPass = _currentFramebuffer.RenderPassNoClear;
renderPassBI.renderPass = _newFramebuffer
? _currentFramebuffer.RenderPassNoClear_Init
: _currentFramebuffer.RenderPassNoClear_Load;
vkCmdBeginRenderPass(_cb, ref renderPassBI, VkSubpassContents.Inline);
_activeRenderPass = _currentFramebuffer.RenderPassNoClear;
_activeRenderPass = renderPassBI.renderPass;

if (haveAnyClearValues)
{
Expand Down Expand Up @@ -508,25 +517,7 @@ private void BeginCurrentRenderPass()
}
}

// Set new image layouts
foreach (FramebufferAttachment colorTarget in _currentFramebuffer.ColorTargets)
{
VkTexture vkTex = Util.AssertSubtype<Texture, VkTexture>(colorTarget.Target);
VkImageLayout layout = (vkTex.Usage & TextureUsage.Sampled) != 0
? VkImageLayout.ShaderReadOnlyOptimal
: VkImageLayout.ColorAttachmentOptimal;
vkTex.SetImageLayout(colorTarget.ArrayLayer, layout);
}

if (_currentFramebuffer.DepthTarget != null)
{
VkTexture vkDepthTex = Util.AssertSubtype<Texture, VkTexture>(_currentFramebuffer.DepthTarget.Value.Target);
VkImageLayout layout = (vkDepthTex.Usage & TextureUsage.Sampled) != 0
? VkImageLayout.ShaderReadOnlyOptimal
: VkImageLayout.DepthStencilAttachmentOptimal;

vkDepthTex.SetImageLayout(_currentFramebuffer.DepthTarget.Value.ArrayLayer, layout);
}
_newFramebuffer = false;
}

private void EndCurrentRenderPass()
Expand Down
71 changes: 59 additions & 12 deletions src/Veldrid/Vk/VkFramebuffer.cs
View file
Expand Up @@ -11,14 +11,16 @@ internal unsafe class VkFramebuffer : VkFramebufferBase
{
private readonly VkGraphicsDevice _gd;
private readonly Vulkan.VkFramebuffer _deviceFramebuffer;
private readonly VkRenderPass _renderPassNoClearLoad;
private readonly VkRenderPass _renderPassNoClear;
private readonly VkRenderPass _renderPassClear;
private readonly List<VkImageView> _attachmentViews = new List<VkImageView>();
private bool _destroyed;
private string _name;

public override Vulkan.VkFramebuffer CurrentFramebuffer => _deviceFramebuffer;
public override VkRenderPass RenderPassNoClear => _renderPassNoClear;
public override VkRenderPass RenderPassNoClear_Init => _renderPassNoClear;
public override VkRenderPass RenderPassNoClear_Load => _renderPassNoClearLoad;
public override VkRenderPass RenderPassClear => _renderPassClear;

public override uint RenderableWidth => Width;
Expand All @@ -43,16 +45,12 @@ public VkFramebuffer(VkGraphicsDevice gd, ref FramebufferDescription description
VkAttachmentDescription colorAttachmentDesc = new VkAttachmentDescription();
colorAttachmentDesc.format = vkColorTex.VkFormat;
colorAttachmentDesc.samples = vkColorTex.VkSampleCount;
colorAttachmentDesc.loadOp = VkAttachmentLoadOp.Load;
colorAttachmentDesc.loadOp = VkAttachmentLoadOp.DontCare;
colorAttachmentDesc.storeOp = VkAttachmentStoreOp.Store;
colorAttachmentDesc.stencilLoadOp = VkAttachmentLoadOp.DontCare;
colorAttachmentDesc.stencilStoreOp = VkAttachmentStoreOp.DontCare;
colorAttachmentDesc.initialLayout = VkImageLayout.Undefined;
colorAttachmentDesc.finalLayout = isPresented
? VkImageLayout.PresentSrcKHR
: (vkColorTex.Usage & TextureUsage.Sampled) != 0
? VkImageLayout.ShaderReadOnlyOptimal
: VkImageLayout.ColorAttachmentOptimal;
colorAttachmentDesc.finalLayout = VkImageLayout.ColorAttachmentOptimal;
attachments.Add(colorAttachmentDesc);

VkAttachmentReference colorAttachmentRef = new VkAttachmentReference();
Expand All @@ -69,14 +67,12 @@ public VkFramebuffer(VkGraphicsDevice gd, ref FramebufferDescription description
bool hasStencil = FormatHelpers.IsStencilFormat(vkDepthTex.Format);
depthAttachmentDesc.format = vkDepthTex.VkFormat;
depthAttachmentDesc.samples = vkDepthTex.VkSampleCount;
depthAttachmentDesc.loadOp = VkAttachmentLoadOp.Load;
depthAttachmentDesc.loadOp = VkAttachmentLoadOp.DontCare;
depthAttachmentDesc.storeOp = VkAttachmentStoreOp.Store;
depthAttachmentDesc.stencilLoadOp = hasStencil ? VkAttachmentLoadOp.Load : VkAttachmentLoadOp.DontCare;
depthAttachmentDesc.stencilLoadOp = VkAttachmentLoadOp.DontCare;
depthAttachmentDesc.stencilStoreOp = hasStencil ? VkAttachmentStoreOp.Store : VkAttachmentStoreOp.DontCare;
depthAttachmentDesc.initialLayout = VkImageLayout.Undefined;
depthAttachmentDesc.finalLayout = (vkDepthTex.Usage & TextureUsage.Sampled) != 0
? VkImageLayout.ShaderReadOnlyOptimal
: VkImageLayout.DepthStencilAttachmentOptimal;
depthAttachmentDesc.finalLayout = VkImageLayout.DepthStencilAttachmentOptimal;

depthAttachmentRef.attachment = (uint)description.ColorTargets.Length;
depthAttachmentRef.layout = VkImageLayout.DepthStencilAttachmentOptimal;
Expand Down Expand Up @@ -116,14 +112,43 @@ public VkFramebuffer(VkGraphicsDevice gd, ref FramebufferDescription description
VkResult creationResult = vkCreateRenderPass(_gd.Device, ref renderPassCI, null, out _renderPassNoClear);
CheckResult(creationResult);

for (int i = 0; i < colorAttachmentCount; i++)
{
attachments[i].loadOp = VkAttachmentLoadOp.Load;
attachments[i].initialLayout = VkImageLayout.ColorAttachmentOptimal;
}
if (DepthTarget != null)
{
attachments[attachments.Count - 1].loadOp = VkAttachmentLoadOp.Load;
attachments[attachments.Count - 1].initialLayout = VkImageLayout.DepthStencilAttachmentOptimal;
bool hasStencil = FormatHelpers.IsStencilFormat(DepthTarget.Value.Target.Format);
if (hasStencil)
{
attachments[attachments.Count - 1].stencilLoadOp = VkAttachmentLoadOp.Load;
}

}
creationResult = vkCreateRenderPass(_gd.Device, ref renderPassCI, null, out _renderPassNoClearLoad);
CheckResult(creationResult);


// Load version

if (DepthTarget != null)
{
attachments[attachments.Count - 1].loadOp = VkAttachmentLoadOp.Clear;
attachments[attachments.Count - 1].initialLayout = VkImageLayout.Undefined;
bool hasStencil = FormatHelpers.IsStencilFormat(DepthTarget.Value.Target.Format);
if (hasStencil)
{
attachments[attachments.Count - 1].stencilLoadOp = VkAttachmentLoadOp.Load;
}
}

for (int i = 0; i < colorAttachmentCount; i++)
{
attachments[i].loadOp = VkAttachmentLoadOp.Clear;
attachments[i].initialLayout = VkImageLayout.Undefined;
}

creationResult = vkCreateRenderPass(_gd.Device, ref renderPassCI, null, out _renderPassClear);
Expand Down Expand Up @@ -203,6 +228,28 @@ public VkFramebuffer(VkGraphicsDevice gd, ref FramebufferDescription description
AttachmentCount += (uint)ColorTargets.Count;
}

public override void TransitionToFinalLayout(VkCommandBuffer cb)
{
foreach (FramebufferAttachment ca in ColorTargets)
{
VkTexture vkTex = Util.AssertSubtype<Texture, VkTexture>(ca.Target);
vkTex.SetImageLayout(ca.ArrayLayer, VkImageLayout.ColorAttachmentOptimal);
if ((vkTex.Usage & TextureUsage.Sampled) != 0)
{
vkTex.TransitionImageLayout(cb, 0, 1, ca.ArrayLayer, 1, VkImageLayout.ShaderReadOnlyOptimal);
}
}
if (DepthTarget != null)
{
VkTexture vkTex = Util.AssertSubtype<Texture, VkTexture>(DepthTarget.Value.Target);
vkTex.SetImageLayout(DepthTarget.Value.ArrayLayer, VkImageLayout.DepthStencilAttachmentOptimal);
if ((vkTex.Usage & TextureUsage.Sampled) != 0)
{
vkTex.TransitionImageLayout(cb, 0, 1, DepthTarget.Value.ArrayLayer, 1, VkImageLayout.ShaderReadOnlyOptimal);
}
}
}

public override string Name
{
get => _name;
Expand Down
4 changes: 3 additions & 1 deletion src/Veldrid/Vk/VkFramebufferBase.cs
View file
Expand Up @@ -20,8 +20,10 @@ public VkFramebufferBase()
}

public abstract Vulkan.VkFramebuffer CurrentFramebuffer { get; }
public abstract VkRenderPass RenderPassNoClear { get; }
public abstract VkRenderPass RenderPassNoClear_Init { get; }
public abstract VkRenderPass RenderPassNoClear_Load { get; }
public abstract VkRenderPass RenderPassClear { get; }
public abstract uint AttachmentCount { get; }
public abstract void TransitionToFinalLayout(VkCommandBuffer cb);
}
}
2 changes: 2 additions & 0 deletions src/Veldrid/Vk/VkGraphicsDevice.cs
View file
Expand Up @@ -1077,6 +1077,7 @@ internal void ClearColorTexture(VkTexture texture, VkClearColorValue color)
VkCommandBuffer cb = pool.BeginNewCommandBuffer();
texture.TransitionImageLayout(cb, 0, texture.MipLevels, 0, texture.ArrayLayers, VkImageLayout.TransferDstOptimal);
vkCmdClearColorImage(cb, texture.OptimalDeviceImage, VkImageLayout.TransferDstOptimal, &color, 1, &range);
texture.TransitionImageLayout(cb, 0, texture.MipLevels, 0, texture.ArrayLayers, VkImageLayout.ColorAttachmentOptimal);
pool.EndAndSubmit(cb);
}

Expand All @@ -1098,6 +1099,7 @@ internal void ClearDepthTexture(VkTexture texture, VkClearDepthStencilValue clea
&clearValue,
1,
&range);
texture.TransitionImageLayout(cb, 0, texture.MipLevels, 0, texture.ArrayLayers, VkImageLayout.DepthStencilAttachmentOptimal);
pool.EndAndSubmit(cb);
}

Expand Down
13 changes: 12 additions & 1 deletion src/Veldrid/Vk/VkSwapchainFramebuffer.cs
View file
Expand Up @@ -29,7 +29,8 @@ internal unsafe class VkSwapchainFramebuffer : VkFramebufferBase

public override Vulkan.VkFramebuffer CurrentFramebuffer => _scFramebuffers[(int)_currentImageIndex].CurrentFramebuffer;

public override VkRenderPass RenderPassNoClear => _scFramebuffers[0].RenderPassNoClear;
public override VkRenderPass RenderPassNoClear_Init => _scFramebuffers[0].RenderPassNoClear_Init;
public override VkRenderPass RenderPassNoClear_Load => _scFramebuffers[0].RenderPassNoClear_Load;
public override VkRenderPass RenderPassClear => _scFramebuffers[0].RenderPassClear;

public override IReadOnlyList<FramebufferAttachment> ColorTargets => _scColorTextures[(int)_currentImageIndex];
Expand Down Expand Up @@ -147,6 +148,16 @@ private void CreateFramebuffers()
}
}

public override void TransitionToFinalLayout(VkCommandBuffer cb)
{
foreach (FramebufferAttachment ca in ColorTargets)
{
VkTexture vkTex = Util.AssertSubtype<Texture, VkTexture>(ca.Target);
vkTex.SetImageLayout(ca.ArrayLayer, VkImageLayout.ColorAttachmentOptimal);
vkTex.TransitionImageLayout(cb, 0, 1, ca.ArrayLayer, 1, VkImageLayout.PresentSrcKHR);
}
}

public override string Name
{
get => _name;
Expand Down
35 changes: 35 additions & 0 deletions src/Veldrid/Vk/VulkanUtil.cs
View file
Expand Up @@ -228,6 +228,41 @@ public static bool IsVulkanLoaded()
srcStageFlags = VkPipelineStageFlags.ColorAttachmentOutput;
dstStageFlags = VkPipelineStageFlags.Transfer;
}
else if (oldLayout == VkImageLayout.ColorAttachmentOptimal && newLayout == VkImageLayout.ShaderReadOnlyOptimal)
{
barrier.srcAccessMask = VkAccessFlags.ColorAttachmentWrite;
barrier.dstAccessMask = VkAccessFlags.ShaderRead;
srcStageFlags = VkPipelineStageFlags.ColorAttachmentOutput;
dstStageFlags = VkPipelineStageFlags.FragmentShader;
}
else if (oldLayout == VkImageLayout.DepthStencilAttachmentOptimal && newLayout == VkImageLayout.ShaderReadOnlyOptimal)
{
barrier.srcAccessMask = VkAccessFlags.DepthStencilAttachmentWrite;
barrier.dstAccessMask = VkAccessFlags.ShaderRead;
srcStageFlags = VkPipelineStageFlags.LateFragmentTests;
dstStageFlags = VkPipelineStageFlags.FragmentShader;
}
else if (oldLayout == VkImageLayout.ColorAttachmentOptimal && newLayout == VkImageLayout.PresentSrcKHR)
{
barrier.srcAccessMask = VkAccessFlags.ColorAttachmentWrite;
barrier.dstAccessMask = VkAccessFlags.MemoryRead;
srcStageFlags = VkPipelineStageFlags.ColorAttachmentOutput;
dstStageFlags = VkPipelineStageFlags.BottomOfPipe;
}
else if (oldLayout == VkImageLayout.TransferDstOptimal && newLayout == VkImageLayout.ColorAttachmentOptimal)
{
barrier.srcAccessMask = VkAccessFlags.TransferWrite;
barrier.dstAccessMask = VkAccessFlags.ColorAttachmentWrite;
srcStageFlags = VkPipelineStageFlags.Transfer;
dstStageFlags = VkPipelineStageFlags.ColorAttachmentOutput;
}
else if (oldLayout == VkImageLayout.TransferDstOptimal && newLayout == VkImageLayout.DepthStencilAttachmentOptimal)
{
barrier.srcAccessMask = VkAccessFlags.TransferWrite;
barrier.dstAccessMask = VkAccessFlags.DepthStencilAttachmentWrite;
srcStageFlags = VkPipelineStageFlags.Transfer;
dstStageFlags = VkPipelineStageFlags.LateFragmentTests;
}
else
{
Debug.Fail("Invalid image layout transition.");
Expand Down

0 comments on commit 88c8a87

Please sign in to comment.