Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix various GLFW Vulkan Linux issues #430

Merged
merged 5 commits into from
Mar 26, 2023
Merged

Fix various GLFW Vulkan Linux issues #430

merged 5 commits into from
Mar 26, 2023

Conversation

Thalhammer
Copy link
Contributor

This is an attempt to make rmlui usable on linux with vulkan.
We intend to build an app based on vulkan because we need to do lot of gpgpu/shader processing, but we also need to present a normal UI to the user.

Trying out RmlUi on linux I ran in a number of issues.

  1. The demo did not start complaining about missing extensions, which turned out to be a TODO (fixed in first commit)
  2. Vulkan validator complained about duplicate queue families (on intel igpu) Relevant Spec
    I fixed this in commit 2 by simply ignoring the second queue create if they are identical. Everything else still is the same, rmlui will simply get the same queue for compute and graphics.
  3. I modified the format selection to prefer UNORM/linear rgb over srgb. Technically srgb would be better, but this would require vast modifications to the rest of rmlui. The issue with using a srgb format without a full srgb pipeline is color missmatch, depending on the order a driver returns formats.

There are more issues though which I am not sure about:

  1. Without VK Debugging on rmlui uses VK_ATTACHMENT_LOAD_OP_DONT_CARE for the color attachment (VK_ATTACHMENT_LOAD_OP_CLEAR in debug mode). I think this is because rmlui is normally integrated into a larger app where it would get cleared by the rest of the engine, however in the demo cases this causes visual issues (elements persist unless a new element is in the same place). Given that an engine probably provides a custom renderer/backend anyway it might make more sense to always set it to VK_ATTACHMENT_LOAD_OP_CLEAR.
  2. You get a seizure without Debugging on. The image flickers and is completely garbled at some positions. I tracked this down to what I assume is a missing barrier somewhere because using vkQueueWaitIdle to wait for the rendering to complete before present hides the issue, but is obviously no real solution. From my preliminary check I could not find any issues in how fences/semaphores are setup, but I'll dig further and append an 4 commit if I find something.

Let me know what you think about it or if you want me to test something else.

@mikke89 mikke89 added the backends Platforms and renderers label Mar 23, 2023
@mikke89
Copy link
Owner

mikke89 commented Mar 23, 2023

Hey, nice to see new contributors. And sounds like an interesting project. I don't have a way to test Vulkan on Linux myself right now, so contributions here are very much welcome. All of this looks good to me.

As for number 3: Yeah, right now we assume all values and operations are done in sRGB nonlinear color space. There will be improvements here for RmlUi 6.0 (currently filter branch), where we switch to pre-multiplied alpha and proper linear space composition. For now, this approach seems reasonable to me.

I don't have any meaningful contributions to the other issues. I'll happily take further commits, or we can make more fixes in new PRs too.

Let me know when you are happy with this, and I'll merge it. I like to ensure that every commit builds correctly, so I will just squash and merge them, unless you want to fixup the commits that didn't build correctly.

@Thalhammer
Copy link
Contributor Author

Let me know when you are happy with this, and I'll merge it.

I am happy with it for the things it fixes, I can do separate PRs for the other things.

I like to ensure that every commit builds correctly, so I will just squash and merge them

This is fine, I do so on my repo's as well because it makes the history cleaner.

Would you be fine with simply always setting VK_ATTACHMENT_LOAD_OP_CLEAR on the attachment, cause I can't (except for slightly higher load when the gpu has to write some pixels twice) see any downsides and its a easy fix. I don't really like having different behaviour in debug/release builds, which makes removing the distinction even more tempting.

@mikke89
Copy link
Owner

mikke89 commented Mar 24, 2023

Would you be fine with simply always setting VK_ATTACHMENT_LOAD_OP_CLEAR on the attachment

Yeah, this seems perfectly fine to me. I haven't seen these visual issues myself on Windows, maybe there are some driver/implementation differences, in any case I'd like to ensure that it works correctly for everyone.

@Thalhammer
Copy link
Contributor Author

Thalhammer commented Mar 25, 2023

maybe there are some driver/implementation differences

Absolutely possible. Linux tends to be a bit more unforgiving in those cases. I also noticed that on my desktop PC using a discrete AMD gpu the vkQueueWaitIdle call is not required to get a correct image, so this might also be a gpu vendor thing in addition to a driver difference. Unfortunately I don't have access to a windows machine with intel/nvidia graphics so I can't really rule out which of the two is the case (or both). Also make sure to disable RMLUI_VK_DEBUG when trying to reproduce the issue because having it on (in particular using VK_VALIDATION_FEATURE_ENABLE_GPU_ASSISTED_EXT) seems to hide the issue for me.

I added the commit to change the load op so from my side this is good to merge.

PS: I'd like to optimize the rendering overhead by only re rendering the view if anything actually changed. This is easy for simple views (render on any input event), but quickly breaks down with animations, lottie and the blinking cursor for text input. Any hints on where to get started to detect if anything in the scene changed ?

@mikke89
Copy link
Owner

mikke89 commented Mar 26, 2023

Thanks a lot, these changes look good to me!

PS: I'd like to optimize the rendering overhead by only re rendering the view if anything actually changed. This is easy for simple views (render on any input event), but quickly breaks down with animations, lottie and the blinking cursor for text input. Any hints on where to get started to detect if anything in the scene changed ?

This is something other users have requested too. See e.g. this discussion thread and this post for some ideas.

I don't think there is any simple way around it, other than considering all parts of the library that can trigger rendering changes. I think you've already mentioned the most important triggers. Obvious things to check for are layout and position changes. But this is not sufficient, e.g. next up would be rendering properties, like color. And then custom elements like input and lottie. It would be nice to have a built-in way to detect this, but I hope it can be done without being too invasive.

@mikke89 mikke89 merged commit 0cffa04 into mikke89:master Mar 26, 2023
@Thalhammer Thalhammer deleted the dth-fix-glfw-vk branch March 27, 2023 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backends Platforms and renderers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants