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

[Vulkan 1.2] Implement Bindless rendering #467

Draft
wants to merge 18 commits into
base: 1.20.x
Choose a base branch
from

Conversation

thr3343
Copy link
Contributor

@thr3343 thr3343 commented Jul 24, 2024

(Fixes several design flaws from #461, and allows Bindless mode to be disabled if unsupported)

Enhances VulkanMod's internal engine (i.e. the renderer) by utilizing Vulkan 1.2 features to implement bindless rendering

For compat reasons this is handled with two modes, Bindless and Non-Bindless mode

Bindless mode

  • Uniform memory usage decreased by up to 100x
  • Reduced Push Constant usage
  • Reduced CPU overhead
  • Immutable Samplers and Inline Uniform blocks are enabled
  • Entity rendering performance increased slightly

Non-Bindless (Bindful) mode

  • Used if Bindless mode is unsupported
  • Bindless optimizations, Immutable Samplers and Inline Uniform blocks disabled
  • The additional optimizatons are still used however
  • Otherwise perfomance should be identical to upstream

Additional optimizatons

  • Many uniforms optimized into PushConstants
  • Chunk rendering performnce is increased slightly

Minor visual bugs

  • New textures rendered for the first time are skipped for 1 frame; causes very minor texture flicker:
  • Graphics menu navigation arrows use the incorrect fill color
  • Sky Color transitions between biomes are delayed by one frame

Edit: Removed mention of Anisotropic Filtering and MSAA; recently found that they do not require bindless rendering, so those features will likely show up later in a separate PR

@Vixea
Copy link

Vixea commented Jul 24, 2024

Is it not possible to use inline uniforms without a descriptor indexing?
I know im asking a lot considering im not writing anything(yet) but i would like to know about that

@thr3343
Copy link
Contributor Author

thr3343 commented Jul 24, 2024

Is it not possible to use inline uniforms without descriptor index?

This is difficult to explain well, so hope this makes sense

The Inline Uniform Block feature is mostly used for convenience, and does not really improve performance much afaik
They allow global uniforms to be shared between shaders much more easily
And mostly contain immutables that don't change very frequently (e.g. Enchant Glint and FogStart/FogEnd)
They help alot with deduping uniforms, which reduces uniform memory usage and the complexity of the bindless code path

They can actually be used without Descriptor Indexing, but doing that adds code bloat and increases the complexity of descriptor management, which is something i wanted to avoid if possible

@seji1
Copy link
Contributor

seji1 commented Jul 24, 2024

Yet another (not crash) log. Now i'm getting really weird errors (had a lot of personal info and launching in ide doesn't give any errors at all, so submitting through mclo) https://mclo.gs/lkJXFf4.

Also got this one, but not sure if it's useful
hs_err_pid19273.log

@Vixea
Copy link

Vixea commented Jul 24, 2024

Is it not possible to use inline uniforms without descriptor index?

This is difficult to explain well, so hope this makes sense

The Inline Uniform Block feature is mostly used for convenience, and does not really improve performance much afaik
They allow global uniforms to be shared between shaders much more easily
And mostly contain immutables that don't change very frequently (e.g. Enchant Glint and FogStart/FogEnd)
They help alot with deduping uniforms, which reduces uniform memory usage and the complexity of the bindless code path

They can actually be used without Descriptor Indexing, but doing that adds code bloat and increases the complexity of descriptor management, which is something i wanted to avoid if possible

Ahh fair enough as long as it doesnt do anything to memory complexity I dont expect a perf increase though any memory improvement on riscv can in turn increase perf because rn the memory controllers are rather weak(which will change soon - more powerful socs are coming out). Not that riscv is a supported platform with vulkan mod and even if it was theres some issues with either lwjgl or gradle/fabric that make it where if you specify any linux x86 binaries it will just always use them even if you also specify the riscv variant. Which is an obvious blocker.

@Vixea
Copy link

Vixea commented Jul 25, 2024

Oh no runtimeDeacriptorArray is also not supported, welp.
Time to yell at some driver devs /hj

@seji1
Copy link
Contributor

seji1 commented Jul 25, 2024

Hi, game seems to be actually playable now, just some minor glitches:

Stars in daytime
image

Broken loading screen (no logo and progress bar when launching the game, appears properly only in fade animation after game launched and other reloads like resourcepacks)
image

Also generating new chunks seems to be much slower. When flying on 0.4.4 with 12 chunks render distance it can keep up easily, but on this pr it can't
image

Visual glitch when changing menu for the first time is gone

Logs don't seem to have anything useful.

@thr3343
Copy link
Contributor Author

thr3343 commented Jul 25, 2024

@seji1 thanks for testing this
The Chunk load regressions might be related to memory barrier regressions from this commit, which is not the fault of this PR: 7789db1
Its hardware specific as the chunk load regressions doesn't happen on AMD though, so will mention it to Collateral

Edit: I was wrong, I incorrectly added UploadManager.syncUpload() calls due to a bad merge, which slowed down chunk loading

The texture and splash load screen issues are due to shader glitches, and only happen on Non-Bindless mode (Bindless mode is fine)

Will try to get them fixed later today hopefully

@seji1

This comment was marked as outdated.

@R1chterScale
Copy link

As just a single data point, on my system (7900XT+RADV and 5600X) there are no performance regressions for frame rate at 64 render distance (no improvements either but given this is still in progress I wasn't expecting any)

@thr3343
Copy link
Contributor Author

thr3343 commented Jul 27, 2024

As just a single data point, on my system (7900XT+RADV and 5600X) there are no performance regressions for frame rate at 64 render distance (no improvements either but given this is still in progress I wasn't expecting any)

(Sorry this is a bit long, wanted to clarify some details)

That's good to know that there are no performance regressions

This PR is close to complete, so unfortunately their probably won't be any additional optimizations though

To explain the lack of performance improvement, Bindless Rendering is mostly focused on modernizing VulkanMod, adding support for future features (MSAA + Anisotropic Filtering) and removing major CPU bottlenecks with chunk and entity rendering

My CPU is quite weak, so the improvement with Bindless Rendering is noticeable
But on stronger CPUs, the GPU is likely already fully maxed out and utilized, so Bindless Rendering likely won't help performance too much in those cases

@R1chterScale
Copy link

Didn't realise it was for CPU bound situations, apologies. I turned off Indirect Draw to force myself into a CPU bound situation and there does appear to be a small improvement even on my system, going from 190->200 FPS roughly.

@thr3343
Copy link
Contributor Author

thr3343 commented Jul 27, 2024

Didn't realise it was for CPU bound situations, apologies

That's OK, this is my bad as Bindless rendering is a complicated concept and has alot of applications, which makes it difficult to explain properly

So in a sense this PR is more of a engine/render rewrite

There are actually some GPU improvements as well, but they are mostly micro-optimizations and small tweaks
(e.g. reducing shader memory usage, using faster memory paths for some rendering steps e.g.)

@thr3343 thr3343 marked this pull request as draft August 25, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants