Support for custom vulkan effects

[ammen99]

As people following the Matrix channel likely already know, I have been working on custom Vulkan effects for Wayfire. Currently, this requires installing a wlroots 0.20 fork with minimal changes (it can be installed alongside any other wlroots release without conflicts): https://gitlab.freedesktop.org/ammen99/wlroots/-/tree/vulkan-effects?ref_type=heads

To enable vulkan effects, make sure that you:

1. Install the wlroots fork described above
2. Pass -Dvulkan_effects=true when building Wayfire

Currently supported are wobbly and view-3d, which means that plugins like wrot and switcher also are supposed to work, alongside wobbly. The entire vulkan effects support is currently guarded behind #if WF_HAS_VULKANFX and is meant as experimental, i.e big breaking changes might be required in the future - though hopefully, this won't be the case ...

Marking this as draft because this currently requires wlroots 0.20, so it will be mergeable after that. Feedback really welcome in case anyone wants to review and/or test this new functionality.

[mark-herbert42]

does wayfire build with wlroots 0.20 already?

[soreau]

does wayfire build with wlroots 0.20 already?

Wayfire master doesn't quite yet but it's a very small patch and I expect we'll update to wlroots 0.20.x as soon as it's released.

[mark-herbert42]

there is a small patch to make it build. but there is a need for some more patch to eliminate crash caused by this change:

https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/5107

it is not crashing immediately but after some use , i could not find the solution for this and right now use 20-rc4 with 5107 patched out.

[mark-herbert42]

Tested further. The request is crashing because of this
https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/5107
so compilation is fixed - but this API change is not, wayfire is simply crashing bacause of assertion in input-method, but just removing assertion does not help. The wlroots functions now pass NULL to wayland instead of data pointer, so the logic is to be changed (I do not understand how unfortunately - and there is no similar code in sway or labwc to refer) .

The second point - wobbly gives artifacts on transparent windows. Making window smaller makes artifacts less frequent, but as you see in video on 3.1K screen wobbly is totally unusable together with alfa under vulkan.

wobbly-vulkan.mp4

[killown]

This will reduce CPU usage. The patch still needs work which I wont touch it, I was just curious about the cpu usage, though, on output 1, dragged woobly view disappears. With this patch the view is likely being drawn off-screen, while output 2 works fine, the view takes only 12% cpu usage while dragged in a 75hz monitor, before was around 33% .

This is exactly the gap descriptor heap closes in my theory:
Is the GPU still reading this descriptor set from the previous frame???
If yes: stall CPU until GPU signals done
If no: proceed

I must admit that I don't have the whole picture of how it would be, for me would be like this:
CPU: heap_memory[offset_N] = descriptor_data -> plain memory write, no driver involved
GPU: reads from heap_memory[offset_N] ---> no driver involved also

So the normal usage would drop around 7% as good as gles2.

You were correct that the descriptor set isn't the sole cause of the CPU usage, but I still believe there's additional overhead elsewhere based on these descriptors.

~/G/wayfire ❯❯❯ git diff                                                                                                ✘ 130 remotes/origin/vulkan-helpers ✱
diff --git a/src/api/wayfire/vulkan.hpp b/src/api/wayfire/vulkan.hpp
index 7fb703dc..8f35bbba 100644
--- a/src/api/wayfire/vulkan.hpp
+++ b/src/api/wayfire/vulkan.hpp
@@ -250,6 +250,7 @@ class image_descriptor_set_pool_t : public std::enable_shared_from_this<image_de
         std::shared_ptr<gpu_buffer_t> matrix_buffer;
 
         wf::signal::connection_t<command_buffer_t::reset_signal> reset_listener;
+        wf::signal::connection_t<wf::texture_t::destroy_signal> texture_destroy_listener;
         command_buffer_t *owner = nullptr;
 
         std::shared_ptr<wf::texture_t> last_texture;
diff --git a/src/core/vulkan.cpp b/src/core/vulkan.cpp
index 3e0e28a5..5e9d1b95 100644
--- a/src/core/vulkan.cpp
+++ b/src/core/vulkan.cpp
@@ -239,12 +239,35 @@ VkDescriptorSet image_descriptor_set_pool_t::get_descriptor_set(
 
         // Cleanup potential old connections
         entry.reset_listener.disconnect();
+        entry.texture_destroy_listener.disconnect();
+
+        if (entry.image_view != VK_NULL_HANDLE)
+        {
+            vkDestroyImageView(context->get_device(), entry.image_view, nullptr);
+            entry.image_view = VK_NULL_HANDLE;
+        }
+
+        if (entry.last_texture)
+        {
+            if (auto it = lookup_table.find(entry.last_texture->get_wlr_texture());
+                (it != lookup_table.end()) && (it->second == index))
+            {
+                lookup_table.erase(it);
+            }
+        }
 
         lookup_table[texture->get_wlr_texture()] = index;
         entry.last_texture = texture;
         entry.owner = &cmd_buf;
 
-        entry.reset_listener = [this, &entry, index] (command_buffer_t::reset_signal *data)
+        entry.reset_listener = [&entry] (command_buffer_t::reset_signal*)
+        {
+            entry.owner = nullptr;
+            entry.reset_listener.disconnect();
+        };
+        cmd_buf.connect(&entry.reset_listener);
+
+        entry.texture_destroy_listener = [this, &entry, index] (wf::texture_t::destroy_signal*)
         {
             if (entry.image_view != VK_NULL_HANDLE)
             {
@@ -258,11 +281,11 @@ VkDescriptorSet image_descriptor_set_pool_t::get_descriptor_set(
                 lookup_table.erase(it);
             }
 
-            entry.reset_listener.disconnect();
+            entry.texture_destroy_listener.disconnect();
             entry.last_texture.reset();
             entry.owner = nullptr;
         };
-        cmd_buf.connect(&entry.reset_listener);
+        texture->connect(&entry.texture_destroy_listener);
 
         // We need to keep the descriptor pool alive until the command buffer is reset.
         cmd_buf.bound_descriptor_pools.push_back(this->shared_from_this());
@@ -272,15 +295,33 @@ VkDescriptorSet image_descriptor_set_pool_t::get_descriptor_set(
         return entry.set;
     };
 
-    if (auto it = lookup_table.find(texture->get_wlr_texture());it != lookup_table.end())
+    const auto& reuse_set = [&] (size_t index) -> VkDescriptorSet
+    {
+        auto& entry = *allocated_sets[index];
+
+        entry.reset_listener.disconnect();
+        entry.owner = &cmd_buf;
+
+        entry.reset_listener = [&entry] (command_buffer_t::reset_signal*)
+        {
+            entry.owner = nullptr;
+            entry.reset_listener.disconnect();
+        };
+        cmd_buf.connect(&entry.reset_listener);
+
+        cmd_buf.bound_descriptor_pools.push_back(this->shared_from_this());
+        return entry.set;
+    };
+
+    if (auto it = lookup_table.find(texture->get_wlr_texture()); it != lookup_table.end())
     {
         auto& allocated = allocated_sets[it->second];
         if ((texture->get_source_box() == allocated->last_texture->get_source_box()) &&
             (texture->get_transform() == allocated->last_texture->get_transform()) &&
-            (allocated->owner == &cmd_buf))
+            (texture->get_color_transform() == allocated->last_texture->get_color_transform()))
         {
             // Fast path: we already have a descriptor set for this texture, just reuse it.
-            return allocated->set;
+            return reuse_set(it->second);
         }
     }
 
@@ -851,4 +892,4 @@ vulkan_render_state_t& vulkan_render_state_t::get()
     auto& core_impl = wf::get_core_impl();
     return *core_impl.vulkan_state;
 }
-} // namespace wf
+} // namespace wf
\ No newline at end of file