Fix some Vulkan validation errors (#5452)

* Fix some validation errors and silence the annoying pipeline barrier error

* Remove bogus decref/incref on index buffer state

* Make unsafe blit opt-in rather than opt-out

* Remove Vulkan debugger messages blacklist

* Adjust GetImageUsage to not set the storage bit for multisample textures if not supported

.github/assign/audio.yml

@@ -1,8 +0,0 @@
-addReviewers: true
-
-reviewers:
-  - marysaka
-
-filterLabels:
-  include:
-    - audio

.github/assign/cpu.yml

@@ -1,11 +0,0 @@
-addReviewers: true
-
-reviewers:
-  - gdkchan
-  - riperiperi
-  - marysaka
-  - LDj3SNuD
-
-filterLabels:
-  include:
-    - cpu

.github/assign/global.yml

@@ -1,4 +0,0 @@
-addReviewers: true
-
-reviewers:
-  - Ryujinx/developers

.github/assign/gpu.yml

@@ -1,10 +0,0 @@
-addReviewers: true
-
-reviewers:
-  - gdkchan
-  - riperiperi
-  - marysaka
-
-filterLabels:
-  include:
-    - gpu

.github/assign/gui.yml

@@ -1,11 +0,0 @@
-addReviewers: true
-
-reviewers:
-  - Ack77
-  - emmauss
-  - TSRBerry
-  - marysaka
-
-filterLabels:
-  include:
-    - gui

.github/assign/horizon.yml

@@ -1,11 +0,0 @@
-addReviewers: true
-
-reviewers:
-  - gdkchan
-  - Ack77
-  - marysaka
-  - TSRBerry
-
-filterLabels:
-  include:
-    - horizon

.github/assign/infra.yml

@@ -1,9 +0,0 @@
-addReviewers: true
-
-reviewers:
-  - marysaka
-  - TSRBerry
-
-filterLabels:
-  include:
-    - infra

.github/reviewers.yml

@@ -0,0 +1,32 @@
+audio:
+  - marysaka
+
+cpu:
+  - gdkchan
+  - riperiperi
+  - marysaka
+  - LDj3SNuD
+
+gpu:
+  - gdkchan
+  - riperiperi
+  - marysaka
+
+gui:
+  - Ack77
+  - emmauss
+  - TSRBerry
+  - marysaka
+
+horizon:
+  - gdkchan
+  - Ack77
+  - marysaka
+  - TSRBerry
+
+infra:
+  - marysaka
+  - TSRBerry
+
+default:
+  - "@developers"

.github/update_reviewers.py

@@ -0,0 +1,79 @@
+from pathlib import Path
+from typing import List, Set
+from github import Github
+from github.Repository import Repository
+from github.GithubException import GithubException
+
+import sys
+import yaml
+
+
+def add_reviewers(
+    reviewers: Set[str], team_reviewers: Set[str], new_entries: List[str]
+):
+    for reviewer in new_entries:
+        if reviewer.startswith("@"):
+            team_reviewers.add(reviewer[1:])
+        else:
+            reviewers.add(reviewer)
+
+
+def update_reviewers(config, repo: Repository, pr_id: int) -> int:
+    pull_request = repo.get_pull(pr_id)
+
+    if not pull_request:
+        sys.stderr.writable(f"Unknown PR #{pr_id}\n")
+        return 1
+
+    pull_request_author = pull_request.user.login
+    reviewers = set()
+    team_reviewers = set()
+
+    for label in pull_request.labels:
+        if label.name in config:
+            add_reviewers(reviewers, team_reviewers, config[label.name])
+
+    if "default" in config:
+        add_reviewers(reviewers, team_reviewers, config["default"])
+
+    if pull_request_author in reviewers:
+        reviewers.remove(pull_request_author)
+
+    try:
+        reviewers = list(reviewers)
+        team_reviewers = list(team_reviewers)
+        print(
+            f"Attempting to assign reviewers ({reviewers}) and team_reviewers ({team_reviewers})"
+        )
+        pull_request.create_review_request(reviewers, team_reviewers)
+        return 0
+    except GithubException as e:
+        sys.stderr.write(f"Cannot assign review request for PR #{pr_id}: {e}\n")
+        return 1
+
+
+if __name__ == "__main__":
+    if len(sys.argv) != 5:
+        sys.stderr.write("usage: <token> <repo_path> <pr_id> <config_path>\n")
+        sys.exit(1)
+
+    token = sys.argv[1]
+    repo_path = sys.argv[2]
+    pr_id = int(sys.argv[3])
+    config_path = Path(sys.argv[4])
+
+    g = Github(token)
+    repo = g.get_repo(repo_path)
+
+    if not repo:
+        sys.stderr.write("Repository not found!\n")
+        sys.exit(1)
+
+    if not config_path.exists():
+        sys.stderr.write(f'Config "{config_path}" not found!\n')
+        sys.exit(1)
+
+    with open(config_path, "r") as f:
+        config = yaml.safe_load(f)
+
+    sys.exit(update_reviewers(config, repo, pr_id))

.github/workflows/pr_triage.yml

@@ -12,43 +12,23 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
+      # Grab sources to get update_reviewers.py and reviewers.yml
+      - name: Fetch sources
+        uses: actions/checkout@v3
+        with:
+          # Ensure we pin the source origin as pull_request_target run under forks.
+          fetch-depth: 0
+          repository: Ryujinx/Ryujinx
+          ref: master
+
       - name: Update labels based on changes
         uses: actions/labeler@v4
         with:
           sync-labels: true
           dot: true
 
-      - name: Auto Assign [Audio]
-        uses: kentaro-m/auto-assign-action@v1.2.5
-        with:
-          configuration-path: '.github/assign/audio.yml'
-
-      - name: Auto Assign [CPU]
-        uses: kentaro-m/auto-assign-action@v1.2.5
-        with:
-          configuration-path: '.github/assign/cpu.yml'
-
-      - name: Auto Assign [GPU]
-        uses: kentaro-m/auto-assign-action@v1.2.5
-        with:
-          configuration-path: '.github/assign/gpu.yml'
-
-      - name: Auto Assign [GUI]
-        uses: kentaro-m/auto-assign-action@v1.2.5
-        with:
-          configuration-path: '.github/assign/gui.yml'
-
-      - name: Auto Assign [Horizon]
-        uses: kentaro-m/auto-assign-action@v1.2.5
-        with:
-          configuration-path: '.github/assign/horizon.yml'
-
-      - name: Auto Assign [Infra]
-        uses: kentaro-m/auto-assign-action@v1.2.5
-        with:
-          configuration-path: '.github/assign/infra.yml'
-
-      - name: Auto Assign [Global]
-        uses: kentaro-m/auto-assign-action@v1.2.5
-        with:
-          configuration-path: '.github/assign/global.yml'
+      - name: Assign reviewers
+        run: |
+          pip3 install PyGithub
+          python3 .github/update_reviewers.py ${{ secrets.GITHUB_TOKEN }} ${{ github.repository }} ${{ github.event.pull_request.number }} .github/reviewers.yml
+        shell: bash

src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs

@@ -168,6 +168,16 @@ namespace Ryujinx.Graphics.Vulkan
             return ComponentType.Float;
         }
 
+        public ImageAspectFlags GetDepthStencilAspectFlags()
+        {
+            if (_depthStencil == null)
+            {
+                return ImageAspectFlags.None;
+            }
+
+            return _depthStencil.Info.Format.ConvertAspectFlags();
+        }
+
         public bool IsValidColorAttachment(int bindIndex)
         {
             return (uint)bindIndex < Constants.MaxRenderTargets && (_validColorAttachments & (1u << bindIndex)) != 0;
@@ -226,7 +236,7 @@ namespace Ryujinx.Graphics.Vulkan
 
             _depthStencil?.Storage.SetModification(
                 AccessFlags.DepthStencilAttachmentWriteBit,
-                PipelineStageFlags.ColorAttachmentOutputBit);
+                PipelineStageFlags.LateFragmentTestsBit);
         }
 
         public void InsertClearBarrier(CommandBufferScoped cbs, int index)

src/Ryujinx.Graphics.Vulkan/IndexBufferState.cs

@@ -152,9 +152,6 @@ namespace Ryujinx.Graphics.Vulkan
         {
             if (_buffer == from)
             {
-                _buffer.DecrementReferenceCount();
-                to.IncrementReferenceCount();
-
                 _buffer = to;
             }
         }

src/Ryujinx.Graphics.Vulkan/PipelineBase.cs

@@ -245,13 +245,28 @@ namespace Ryujinx.Graphics.Vulkan
 
         public unsafe void ClearRenderTargetDepthStencil(int layer, int layerCount, float depthValue, bool depthMask, int stencilValue, int stencilMask)
         {
-            // TODO: Use stencilMask (fully)
+            // TODO: Use stencilMask (fully).
 
             if (FramebufferParams == null || !FramebufferParams.HasDepthStencil)
             {
                 return;
             }
 
+            var clearValue = new ClearValue(null, new ClearDepthStencilValue(depthValue, (uint)stencilValue));
+            var flags = depthMask ? ImageAspectFlags.DepthBit : 0;
+
+            if (stencilMask != 0)
+            {
+                flags |= ImageAspectFlags.StencilBit;
+            }
+
+            flags &= FramebufferParams.GetDepthStencilAspectFlags();
+
+            if (flags == ImageAspectFlags.None)
+            {
+                return;
+            }
+
             if (_renderPass == null)
             {
                 CreateRenderPass();
@@ -259,14 +274,6 @@ namespace Ryujinx.Graphics.Vulkan
 
             BeginRenderPass();
 
-            var clearValue = new ClearValue(null, new ClearDepthStencilValue(depthValue, (uint)stencilValue));
-            var flags = depthMask ? ImageAspectFlags.DepthBit : 0;
-
-            if (stencilMask != 0)
-            {
-                flags |= ImageAspectFlags.StencilBit;
-            }
-
             var attachment = new ClearAttachment(flags, 0, clearValue);
             var clearRect = FramebufferParams.GetClearRect(ClearScissor, layer, layerCount);
 
@@ -935,7 +942,7 @@ namespace Ryujinx.Graphics.Vulkan
 
             SignalStateChange();
 
-            if (_program.IsCompute)
+            if (internalProgram.IsCompute)
             {
                 EndRenderPass();
             }

src/Ryujinx.Graphics.Vulkan/TextureStorage.cs

@@ -78,7 +78,7 @@ namespace Ryujinx.Graphics.Vulkan
 
             var sampleCountFlags = ConvertToSampleCountFlags(gd.Capabilities.SupportedSampleCounts, (uint)info.Samples);
 
-            var usage = GetImageUsage(info.Format, info.Target, gd.Capabilities.SupportsShaderStorageImageMultisample);
+            var usage = GetImageUsage(info.Format, info.Target, gd.Capabilities.SupportsShaderStorageImageMultisample, forceStorage: true);
 
             var flags = ImageCreateFlags.CreateMutableFormatBit;
 
@@ -291,7 +291,7 @@ namespace Ryujinx.Graphics.Vulkan
             }
         }
 
-        public static ImageUsageFlags GetImageUsage(Format format, Target target, bool supportsMsStorage)
+        public static ImageUsageFlags GetImageUsage(Format format, Target target, bool supportsMsStorage, bool forceStorage = false)
         {
             var usage = DefaultUsageFlags;
 
@@ -304,7 +304,7 @@ namespace Ryujinx.Graphics.Vulkan
                 usage |= ImageUsageFlags.ColorAttachmentBit;
             }
 
-            if (format.IsImageCompatible() && (supportsMsStorage || !target.IsMultisample()))
+            if (((forceStorage && !format.IsDepthOrStencil()) || format.IsImageCompatible()) && (supportsMsStorage || !target.IsMultisample()))
             {
                 usage |= ImageUsageFlags.StorageBit;
             }

src/Ryujinx.Graphics.Vulkan/TextureView.cs

@@ -116,7 +116,14 @@ namespace Ryujinx.Graphics.Vulkan
                 return new Auto<DisposableImageView>(new DisposableImageView(gd.Api, device, imageView), null, storage.GetImage());
             }
 
-            _imageView = CreateImageView(componentMapping, subresourceRange, type, ImageUsageFlags.SampledBit);
+            ImageUsageFlags shaderUsage = ImageUsageFlags.SampledBit;
+
+            if (info.Format.IsImageCompatible())
+            {
+                shaderUsage |= ImageUsageFlags.StorageBit;
+            }
+
+            _imageView = CreateImageView(componentMapping, subresourceRange, type, shaderUsage);
 
             // Framebuffer attachments and storage images requires a identity component mapping.
             var identityComponentMapping = new ComponentMapping(
@@ -378,7 +385,7 @@ namespace Ryujinx.Graphics.Vulkan
 
             bool isDepthOrStencil = dst.Info.Format.IsDepthOrStencil();
 
-            if (VulkanConfiguration.UseSlowSafeBlitOnAmd && (_gd.Vendor == Vendor.Amd || _gd.IsMoltenVk))
+            if (!VulkanConfiguration.UseUnsafeBlit || (_gd.Vendor != Vendor.Nvidia && _gd.Vendor != Vendor.Intel))
             {
                 _gd.HelperShader.Blit(
                     _gd,

src/Ryujinx.Graphics.Vulkan/VulkanConfiguration.cs

@@ -3,7 +3,7 @@
     static class VulkanConfiguration
     {
         public const bool UseFastBufferUpdates = true;
-        public const bool UseSlowSafeBlitOnAmd = true;
+        public const bool UseUnsafeBlit = true;
         public const bool UsePushDescriptors = false;
 
         public const bool ForceD24S8Unsupported = false;

src/Ryujinx.Graphics.Vulkan/VulkanDebugMessenger.cs

@@ -10,17 +10,6 @@ namespace Ryujinx.Graphics.Vulkan
 {
     class VulkanDebugMessenger : IDisposable
     {
-        private static readonly string[] _excludedMessages = {
-            // NOTE: Done on purpose right now.
-            "UNASSIGNED-CoreValidation-Shader-OutputNotConsumed",
-            // TODO: Figure out if fixable
-            "VUID-vkCmdDrawIndexed-None-04584",
-            // TODO: Might be worth looking into making this happy to possibly optimize copies.
-            "UNASSIGNED-CoreValidation-DrawState-InvalidImageLayout",
-            // TODO: Fix this, it's causing too much noise right now.
-            "VUID-VkSubpassDependency-srcSubpass-00867",
-        };
-
         private readonly Vk _api;
         private readonly Instance _instance;
         private readonly GraphicsDebugLevel _logLevel;
@@ -108,14 +97,6 @@ namespace Ryujinx.Graphics.Vulkan
         {
             var msg = Marshal.PtrToStringAnsi((IntPtr)pCallbackData->PMessage);
 
-            foreach (string excludedMessagePart in _excludedMessages)
-            {
-                if (msg.Contains(excludedMessagePart))
-                {
-                    return 0;
-                }
-            }
-
             if (messageSeverity.HasFlag(DebugUtilsMessageSeverityFlagsEXT.ErrorBitExt))
             {
                 Logger.Error?.Print(LogClass.Gpu, msg);

src/Ryujinx.Graphics.Vulkan/VulkanInitialization.cs

@@ -377,8 +377,8 @@ namespace Ryujinx.Graphics.Vulkan
                 ShaderFloat64 = supportedFeatures.ShaderFloat64,
                 ShaderImageGatherExtended = supportedFeatures.ShaderImageGatherExtended,
                 ShaderStorageImageMultisample = supportedFeatures.ShaderStorageImageMultisample,
-                // ShaderStorageImageReadWithoutFormat = true,
-                // ShaderStorageImageWriteWithoutFormat = true,
+                ShaderStorageImageReadWithoutFormat = supportedFeatures.ShaderStorageImageReadWithoutFormat,
+                ShaderStorageImageWriteWithoutFormat = supportedFeatures.ShaderStorageImageWriteWithoutFormat,
                 TessellationShader = supportedFeatures.TessellationShader,
                 VertexPipelineStoresAndAtomics = supportedFeatures.VertexPipelineStoresAndAtomics,
                 RobustBufferAccess = useRobustBufferAccess,