Skip to content

zink: revert broken single sampled rendering to multisampled targets handling

Antonino Maniscalco requested to merge antonino/mesa:revert_singles into main

Reverts commits from !22626 (merged) and !22736 (merged)

We should also reopen #6302

That patch worked by just setting rasterizationSamples to 1 but still rendering to multisampled targets. This seems to be what d3d12 does, however I've been reading the Vulkan spec very carefully and I think those patches will never be in spec, also #8911 (closed) may be caused by them (also not sure whether !22736 (merged) fixed the hangs?).

According to https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkGraphicsPipelineCreateInfo-multisampledRenderToSingleSampled-06853 it seems like doing this could be allowed using certain extensions:

  • VK_AMD_mixed_attachment_samples (seems to be supported by RADV)
  • VK_NV_framebuffer_mixed_samples
  • VK_EXT_multisampled_render_to_single_sampled (though this one is meant to do the exact opposite so I'm too sure about it).

So maybe we could consider conditioning to the availability of such extensions instead of reverting?

Also I've been working on implementing this properly lately and considered the following solutions:

  • using shaderStorageImageMultisample device feature and a pass to do an image store instead of exporting the fragment. This would be non trivial and I'm afraid would add to much spaghetti (for the image descriptor and other logic).
  • the obvious way, that is, rendering to a different (single sampled) image (or more likely, to serveral different images) and then blitting to the target(s).

However in the meantime I feel like those patches should be reverted. Even though they make some tests pass on CI I'm now reasoonably convinced they do so by relying on UB.

Edited by Antonino Maniscalco

Merge request reports