This does not resolve the errors noted in #101696, as the project is
incorrectly binding incompatible uniform set descriptors, as set 0
and set 1 have a single `readonly` image and set 2 has a single
`writeonly` texture. The `RenderingDevice` is reporting the errors with
Metal, as it uses a different `_reflect_spirv`, which correctly
determines the read / write attribute. A separate PR will be required
to fix the shared `_reflect_spirv`
Helps #101696
This changes the default shader loading strategy, implemented in the
Metal driver, to compile the `MTLLibrary` on demand when the pipeline
is created, which reduces cold startup time on IPHONE target OSs.
Normally, the `MTLLibrary` is compiled from Metal source asynchronously
when Godot calls
`RenderingDeviceDriverMetal::shader_create_from_bytecode`; however, this
changes this behaviour on mobile platforms to do it on demand when the
pipeline is created, as noted in #96052, Godot will ask to create
many more shaders from bytecode than are initially required. Mobile
OSs like iOS are limited to compiling to shader libraries concurrently,
which results in a significant bottleneck.
This is not the default for macOS, as it can concurrently compile many
shaders at once, resulting in faster startup times for the Godot editor.
VMA handles memory allocation on certain devices better than our custom VK code, so we might as well use it
Co-authored-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
This avoids crashing on devices when a number of varyings greater than the device limit is used.
For now this accurately prints an error when compiling the shader, but the error text only pops up in the editor if the number of user varyings is above the limit.
PR #90993 needed to get rid of VMA_MEMORY_USAGE_AUTO_PREFER_HOST because
we no longer used vmaCreateBuffer so we could specify the allocation
callbacks.
This however resulted in the wrong memory pool being chosen, causing
signficant performance slowdown.
Indicate additional preferred flags to help VMA select the proper pool.
Fixes#101905
This is a followup to PR #101344 (commit
0e06eb80bc).
Some of them were not an issue because Godot was initializing all
members, but they were "fixed" just in case since it could become a
problem in the future.
Valgrind was specifically complaining about HashMapData &
GlobalPipelineData.
This change improves performance of the AgX tonemapper by allowing two matrix multiplications to be combined into one. This comes at the cost of loss of color information that could be correctly interpreted as positive RGB values in the Rec. 2020 color space. Additionally, an insignificant amount of error is intentionally introduced to the input color value to prevent the need for a second max function call before log2. The final negative color clipping has been removed to allow the tonemapper to return negative RGB values, similar to other tonemappers in Godot.
Most important is a fix for an occasional crash due to a use-after-free
bug.
A number of API availability declarations were updated to include tvOS.
The code is now simplified and generic for all platforms, which makes
way for future tvOS support.
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
The purpose of this code is to expose the necessary
functions for users and engine devs to develop tooling
for properly timing and seeking inside particles.
Co-authored-by: Hugo Locurcio <hugo.locurcio@hugo.pro>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: Tomasz Chabora <kobewi4e@gmail.com>
Co-authored-by: Rémi Verschelde <rverschelde@gmail.com>
This changes the polynomial function so that a lower input always results in a lower output and vice-versa. Additionally, the new function returns a value that is much closer to 1.0 when given an input of 1.0.
Technical implementation notes:
- Moved linearization step to before the outset matrix is applied and
changed polynomial contrast curve approximation.
- This does *not* implement Blender's chroma rotation to address hue shift.
This hue rotation was found to have a significant performance impact.
- Improved performance by combining the AgX outset matrix with the Rec 2020 matrix.
Co-authored-by: Allen Pestaluky <allenpestaluky@gmail.com>
Co-authored-by: Clay John <claynjohn@gmail.com>
Moves the initialization of the geo_normal value
to after the user shader code, so that the normal
supplied by the user is actually used instead of
the interpolated vertex normal.
Fixes two errors related to the normal, tangent,
and bitangent vectors, namely normals not always
being inverted on backfaces, and normalization
being reversed from what MikkTSpace expects.
These misaligned accesses are shown in all of our CI hooks. It turned
out to not be difficult to fix.
It is likely that this will improve performance for aarch64.
Fixes#99968 and prevents the error spam referenced in comments of #18732.
Also fixes a number of memory leaks that occur when an audio device is reinitialized or fails to reinitialize and gates reinitialization attempts to around 1 per second instead of ~1000 attempts per second.
Co-authored-by: Kusok <118438257+kus04e4ek@users.noreply.github.com>
The work was performed by collaboration of TheForge and Google. I am
merely splitting it up into smaller PRs and cleaning it up.
This is the most "risky" PR so far because the previous ones have been
miscellaneous stuff aimed at either [improve
debugging](https://github.com/godotengine/godot/pull/90993) (e.g. device
lost), [improve Android
experience](https://github.com/godotengine/godot/pull/96439) (add Swappy
for better Frame Pacing + Pre-Transformed Swapchains for slightly better
performance), or harmless [ASTC
improvements](https://github.com/godotengine/godot/pull/96045) (better
performance by simply toggling a feature when available).
However this PR contains larger modifications aimed at improving
performance or reducing memory fragmentation. With greater
modifications, come greater risks of bugs or breakage.
Changes introduced by this PR:
TBDR GPUs (e.g. most of Android + iOS + M1 Apple) support rendering to
Render Targets that are not backed by actual GPU memory (everything
stays in cache). This works as long as load action isn't `LOAD`, and
store action must be `DONT_CARE`. This saves VRAM (it also makes
painfully obvious when a mistake introduces a performance regression).
Of particular usefulness is when doing MSAA and keeping the raw MSAA
content is not necessary.
Some GPUs get faster when the sampler settings are hard-coded into the
GLSL shaders (instead of being dynamically bound at runtime). This
required changes to the GLSL shaders, PSO creation routines, Descriptor
creation routines, and Descriptor binding routines.
- `bool immutable_samplers_enabled = true`
Setting it to false enforces the old behavior. Useful for debugging bugs
and regressions.
Immutable samplers requires that the samplers stay... immutable, hence
this boolean is useful if the promise gets broken. We might want to turn
this into a `GLOBAL_DEF` setting.
Instead of creating dozen/hundreds/thousands of `VkDescriptorSet` every
frame that need to be freed individually when they are no longer needed,
they all get freed at once by resetting the whole pool. Once the whole
pool is no longer in use by the GPU, it gets reset and its memory
recycled. Descriptor sets that are created to be kept around for longer
or forever (i.e. not created and freed within the same frame) **must
not** use linear pools. There may be more than one pool per frame. How
many pools per frame Godot ends up with depends on its capacity, and
that is controlled by
`rendering/rendering_device/vulkan/max_descriptors_per_pool`.
- **Possible improvement for later:** It should be possible for Godot
to adapt to how many descriptors per pool are needed on a per-key basis
(i.e. grow their capacity like `std::vector` does) after rendering a few
frames; which would be better than the current solution of having a
single global value for all pools (`max_descriptors_per_pool`) that the
user needs to tweak.
- `bool linear_descriptor_pools_enabled = true`
Setting it to false enforces the old behavior. Useful for debugging bugs
and regressions.
Setting it to false is required when workarounding driver bugs (e.g.
Adreno 730).
A ridiculous optimization. Ridiculous because the original code
should've done this in the first place. Previously Godot was doing the
following:
1. Create a command buffer **pool**. One per frame.
2. Create multiple command buffers from the pool in point 1.
3. Call `vkBeginCommandBuffer` on the cmd buffer in point 2. This
resets the cmd buffer because Godot requests the
`VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT` flag.
4. Add commands to the cmd buffers from point 2.
5. Submit those commands.
6. On frame N + 2, recycle the buffer pool and cmd buffers from pt 1 &
2, and repeat from step 3.
The problem here is that step 3 resets each command buffer individually.
Initially Godot used to have 1 cmd buffer per pool, thus the impact is
very low.
But not anymore (specially with Adreno workarounds to force splitting
compute dispatches into a new cmd buffer, more on this later). However
Godot keeps around a very low amount of command buffers per frame.
The recommended method is to reset the whole pool, to reset all cmd
buffers at once. Hence the new steps would be:
1. Create a command buffer **pool**. One per frame.
2. Create multiple command buffers from the pool in point 1.
3. Call `vkBeginCommandBuffer` on the cmd buffer in point 2, which is
already reset/empty (see step 6).
4. Add commands to the cmd buffers from point 2.
5. Submit those commands.
6. On frame N + 2, recycle the buffer pool and cmd buffers from pt 1 &
2, call `vkResetCommandPool` and repeat from step 3.
**Possible issues:** @dariosamo added `transfer_worker` which creates a
command buffer pool:
```cpp
transfer_worker->command_pool =
driver->command_pool_create(transfer_queue_family,
RDD::COMMAND_BUFFER_TYPE_PRIMARY);
```
As expected, validation was complaining that command buffers were being
reused without being reset (that's good, we now know Validation Layers
will warn us of wrong use).
I fixed it by adding:
```cpp
void RenderingDevice::_wait_for_transfer_worker(TransferWorker
*p_transfer_worker) {
driver->fence_wait(p_transfer_worker->command_fence);
driver->command_pool_reset(p_transfer_worker->command_pool); //
! New line !
```
**Secondary cmd buffers are subject to the same issue but I didn't alter
them. I talked this with Dario and he is aware of this.**
Secondary cmd buffers are currently disabled due to other issues (it's
disabled on master).
- `bool RenderingDeviceCommons::command_pool_reset_enabled`
Setting it to false enforces the old behavior. Useful for debugging bugs
and regressions.
There's no other reason for this boolean. Possibly once it becomes well
tested, the boolean could be removed entirely.
Adds `command_bind_render_uniform_sets` and
`add_draw_list_bind_uniform_sets` (+ compute variants).
It performs the same as `add_draw_list_bind_uniform_set` (notice
singular vs plural), but on multiple consecutive uniform sets, thus
reducing graph and draw call overhead.
- `bool descriptor_set_batching = true;`
Setting it to false enforces the old behavior. Useful for debugging bugs
and regressions.
There's no other reason for this boolean. Possibly once it becomes well
tested, the boolean could be removed entirely.
Godot currently does the following:
1. Fill the entire cmd buffer with commands.
2. `submit()`
- Wait with a semaphore for the swapchain.
- Trigger a semaphore to indicate when we're done (so the swapchain
can submit).
3. `present()`
The optimization opportunity here is that 95% of Godot's rendering is
done offscreen.
Then a fullscreen pass copies everything to the swapchain. Godot doesn't
practically render directly to the swapchain.
The problem with this is that the GPU has to wait for the swapchain to
be released **to start anything**, when we could start *much earlier*.
Only the final blit pass must wait for the swapchain.
TheForge changed it to the following (more complicated, I'm simplifying
the idea):
1. Fill the entire cmd buffer with commands.
2. In `screen_prepare_for_drawing` do `submit()`
- There are no semaphore waits for the swapchain.
- Trigger a semaphore to indicate when we're done.
3. Fill a new cmd buffer that only does the final blit to the
swapchain.
4. `submit()`
- Wait with a semaphore for the submit() from step 2.
- Wait with a semaphore for the swapchain (so the swapchain can
submit).
- Trigger a semaphore to indicate when we're done (so the swapchain
can submit).
5. `present()`
Dario discovered this problem independently while working on a different
platform.
**However TheForge's solution had to be rewritten from scratch:** The
complexity to achieve the solution was high and quite difficult to
maintain with the way Godot works now (after Übershaders PR).
But on the other hand, re-implementing the solution became much simpler
because Dario already had to do something similar: To fix an Adreno 730
driver bug, he had to implement splitting command buffers. **This is
exactly what we need!**. Thus it was re-written using this existing
functionality for a new purpose.
To achieve this, I added a new argument, `bool p_split_cmd_buffer`, to
`RenderingDeviceGraph::add_draw_list_begin`, which is only set to true
by `RenderingDevice::draw_list_begin_for_screen`.
The graph will split the draw list into its own command buffer.
- `bool split_swapchain_into_its_own_cmd_buffer = true;`
Setting it to false enforces the old behavior. This might be necessary
for consoles which follow an alternate solution to the same problem.
If not, then we should consider removing it.
PR #90993 added `shader_destroy_modules()` but it was not actually in
use.
This PR adds several places where `shader_destroy_modules()` is called
after initialization to free up memory of SPIR-V structures that are no
longer needed.
Note: This commit ties the IPUnix to the UNIX_SOCKET_UNAVAILABLE define,
disabling it when set. It is maybe not semantically correct (getifaddrs)
is not part of the "socket" API, but it's reasonable to expect that a
platform not supporting Unix-style sockets, would also not support other
Unix network functions.
Fixes#99592
The following variables were set too early, and later code never ran..
prev_shader, prev_variant, prev_spec_constants variables
These variables were shared in two different branch conditions but were
updated inside the first one, so the second could never be true..
The condition to update lighting also should check prev_spec_constants
since it updates that uniform.
When a file is opened with a wrong case, it can work on the developer system but break on a user system with a case-sensitive filesystem.
This will display a warning when it happens.
CAVEATS: It will also display the warning if a symlink is in the path.
Adapt warning if the file is a symlink. Avoid warning on symlinks.
Fix memory leak and avoid `lstat` usage.
Avoid exposing real_path when not in TOOLS_ENABLED mode.
Wayland in particular sets surface_capabilities.currentExtent.width to
the special value 0xFFFFFFFF, which is valid per spec.
Fixes#98779
It may also fix misc issues when resizing on all platforms.
Superseedes PR #98780 , thanks to user tdaven for the original patch.
PR #98780 would break Android support as it did not account that width
and height might need to be swapped.
Replaced manual swap by Godot's SWAP(), which indicates intention much
easier.
• `modernize-use-default-member-init` and `readability-redundant-member-init`
• Minor adjustments to `.clang-tidy` to improve syntax & remove redundancies
- Buffers changing their usage are no longer treated as write usage unless the API requires it.
- Draw lists are not treated as being dependent on each other if their regions do not intersect despite both being write commands.
- Particles were tweaked to use different unused buffers to reduce dependencies.
Fixes an issue introduced in #96439 (see
https://github.com/godotengine/godot/pull/96439#issuecomment-2447288702)
Godot was relying on Java's
activity.getWindowManager().getDefaultDisplay().getRotation(); to apply
pre-rotation but this is wrong.
First, getRotation() may temporarily return a different value from the
correct one; which is what was causing the splash screen to be upside
down. It would return -90 instead of 90 for the first rendered frame.
But unfortunately, the splash screen is just one frame rendered for a
very long time, so the error lingered for a long time for everyone to
see.
Second, to determine what rotation to use, we should be looking at what
Vulkan told us, which is the value we pass to
VkSurfaceTransformFlagBitsKHR::preTransform.
This commit removes the now-unnecessary
screen_get_internal_current_rotation() function (which was introduced by
#96439) and now saves the preTransform value in the swapchain.
- Adds Swappy for Android for stable frame pacing
- Implements pre-transformed Swapchain so that Godot's compositor is in
charge of rotating the screen instead of Android's compositor
(performance optimization for phones that don't have HW rotator)
============================
The work was performed by collaboration of TheForge and Google. I am
merely splitting it up into smaller PRs and cleaning it up.
Changes from original PR:
- Removed "display/window/frame_pacing/android/target_frame_rate" option
to use Engine::get_max_fps instead.
- Target framerate can be changed at runtime using Engine::set_max_fps.
- Swappy is enabled by default.
- Added documentation.
- enable_auto_swap setting is replaced with swappy_mode.
Thaddeus Crews
Pāvels Nadtočajevs
Stuart Carnie
clayjohn
Rémi Verschelde
devloglogan
Evan Todd
Alex
kleonc
Matias N. Goldberg
Lukas Tenbrink
BlueCube3310
Rudolph Bester
Chaosus
MathdudeMan
Adam Scott
Julian Todd
Brandon Haley
thimenesup
Allen Pestaluky
scgm0
David House
QbieShay
Yufeng Ying
Hugo Locurcio
Aarni Koskela
Ricardo Subtil
Stayd
ze2j
Alex Threlfo
Hendrik Brucker
kobewi
alessand10
Andrew_Shobbrook
Ramón Grill
HP van Braam
landervr
Dario
Wagner
Fabio Alessandrelli
mrsaturnsan
0x0ACB
Jamie Pate
A Thousand Ships
Chris
mrsaturnsan
tetrapod00
nikitalita
Valentin Cocaud
Pedro J. Estébanez
Azeez Abass
Trevor Davenport
EnlightenedOne
Yevhen Babiichuk (DustDFG)
maxime.chambefort