VideoBackends / VideoCommon: expose ability to view shader/texture debug names

[iwubcode]

This gives the ability to view an application defined name for our Shaders/Textures in applications like RenderDoc. Currently nothing takes advantage of this but a future feature will. Breaking it out to make it easier to review.

[iwubcode]

I fixed most of the lint errors but the last error isn't matching my local formatting (I tried both clang-format 12.0.0 and 12.0.1).

[leoetlino]

I fixed most of the lint errors but the last error isn't matching my local formatting (I tried both clang-format 12.0.0 and 12.0.1).

Are you using Visual Studio's bundled copy of clang-format? If so, we should probably update our required clang-format version to 12...

[iwubcode]

Are you using Visual Studio's bundled copy of clang-format? If so, we should probably update our required clang-format version to 12...

Yeah I'm using VS bundled clang-format which is 12.0.0. I thought maybe it was out of date so I grabbed the latest (12.0.1) from the prebuilt llvm release. I didn't think about the buildbot version being too old!

[leoetlino]

Reviewed 32 of 32 files at r1, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @iwubcode)

---

Source/Core/VideoBackends/D3D/DXShader.h, line 7 at r1 (raw file):

#include <memory>
#include <optional>

<optional> seems to be unused

---

Source/Core/VideoBackends/D3D/DXShader.cpp, line 16 at r1 (raw file):

  if (!name.empty())
  {
    m_shader->SetPrivateData(WKPDID_D3DDebugObjectName, static_cast<UINT>(name.size()),

For anyone wondering, SetPrivateData performs a copy of the string so there are no lifetime issues here.

---

Source/Core/VideoBackends/D3D/DXTexture.cpp, line 20 at r1 (raw file):

DXTexture::DXTexture(const TextureConfig& config, ComPtr<ID3D11Texture2D> texture,
                     std::string_view name)
    : AbstractTexture(config), m_texture(std::move(texture)), m_name(name)

Just wondering, why do we need to store the name in this class (DXTexture) and not in DXShader?

Come to think of it, does SetPrivateData require that the name is null-terminated? If so, DXShader would have to store the name in a std::string to make sure it is null-terminated.

---

Source/Core/VideoBackends/OGL/OGLShader.h, line 20 at r1 (raw file):

public:
  explicit OGLShader(ShaderStage stage, GLenum gl_type, GLuint gl_id, std::string source,
                     std::string name);

#include <string>

---

Source/Core/VideoBackends/Software/SWRenderer.h, line 24 at r1 (raw file):

  std::unique_ptr<AbstractTexture> CreateTexture(const TextureConfig& config,
                                                 std::string_view name) override;

missing include for string_view

---

Source/Core/VideoBackends/Vulkan/VKRenderer.h, line 39 at r1 (raw file):

  std::unique_ptr<AbstractTexture> CreateTexture(const TextureConfig& config,
                                                 std::string_view name) override;

include

---

Source/Core/VideoBackends/Vulkan/VKShader.cpp, line 63 at r1 (raw file):
According to Vulkan docs:

VUID-VkDebugUtilsObjectNameInfoEXT-pObjectName-parameter

If pObjectName is not NULL, pObjectName must be a null-terminated UTF-8 string

So it seems you need a std::string to ensure the name is null-terminated. The docs aren't clear about the required lifetime for the string you pass in though...

[iwubcode]

Thank you @leoetlino for reviewing! I'll admit, I didn't find a majority of the graphics api documentation very clear about lifetime requirements which is why I opted for the design I did. Unfortunately, I guess I wasn't very thorough, so thank you for catching the issues you did :) You were right about the null termination, I fixed up the apis to be consistent.

[leoetlino]

Reviewed 7 of 7 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @iwubcode)

---

Source/Core/VideoBackends/Vulkan/VKShader.cpp, line 21 at r2 (raw file):

  if (!m_name.empty())
  {
    VkDebugUtilsObjectNameInfoEXT nameInfo = {};

name_info

---

Source/Core/VideoBackends/Vulkan/VKShader.cpp, line 36 at r2 (raw file):

  if (!m_name.empty())
  {
    VkDebugUtilsObjectNameInfoEXT nameInfo = {};

name_info

[leoetlino]

Rebasing should fix the lint failure

[leoetlino]

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @iwubcode)

[iwubcode]

Thanks @leoetlino for reviewing!