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.
• `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.
`core/os/os.h` doesn't use `core/io/image.h`. It just brings
transitive dependencies. Lots of dependencies because `core/os/os.h`
is transitively included in almost every file of godot
Also added `core/io/image.h` into files^1 where `Ref<Image>` and `core/os/os.h`
were used to prevent obscure errors involving `Ref<Image>`
^1 except those which include `core/io/image_loader.h` or `core/io/image.h` by
corresponding .h file with the same name
Signed-off-by: Yevhen Babiichuk (DustDFG) <dfgdust@gmail.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
This reduces memory usage a bit in case multiple placeholders were
requested, e.g. when using multiple NoiseTextures with no noise property
defined.
The placeholder texture's appearance was also changed from a plain magenta
color to a checkerboard alternating between magenta and black pixels.
This makes it easier to spot when the placeholder texture ends up
being used in a complex scene (usually by accident).
The texture's dimensions remain identical to keep the physical size
identical in 2D.
- Implements asynchronous transfer queues from PR #87590.
- Adds ubershaders that can run with specialization constants specified as push constants.
- Pipelines with specialization constants can compile in the background.
- Added monitoring for pipeline compilations.
- Materials and shaders can now be created asynchronously on background threads.
- Meshes that are loaded on background threads can also compile pipelines as part of the loading process.
This adds support in all backends, but the Compatibility renderer works the best.
Mobile and Forward+ can only support one directional light shader (the first in the tree)
While the Compatibility renderer supports any number of shadows.
Co-authored-by: Clay John <claynjohn@gmail.com>
This improves shadow quality by reducing the visibility of the noisy
pattern caused by dithering.
This jittering also applies when FSR2 is enabled, as it provides its own
form of temporal antialiasing.
Co-authored-by: Clay John <claynjohn@gmail.com>
Features:
- Debug-only tracking of objects by type. See
get_driver_allocs_by_object_type et al.
- Debug-only Breadcrumb info for debugging GPU crashes and device lost
- Performance report per frame from get_perf_report
- Some VMA calls had to be modified in order to insert the necessary
memory callbacks
Functionality marked as "debug-only" is only available in debug or dev
builds.
Misc fixes:
- Early break optimization in RenderingDevice::uniform_set_create
============================
The work was performed by collaboration of TheForge and Google. I am
merely splitting it up into smaller PRs and cleaning it up.
Before this change, a skeleton that was not updated every frame would
result in a difference of 2+ between last_change and frame index every
frame, which would disable the buffer rotation and set motion vectors to
zero. This results in significant visual artifacts for FSR2 that are
especially prominent on the characters that move together with the view
such as the main character in third person mode.
This is a significant problem for high refresh rate displays: at 120 Hz,
we are effectively guaranteed to skip skeleton updates every other frame
with skeleton update happening during physics processing, and the lack
of physics interpolation for skeletons. This happens by default in TPS
demo when FSR2 is enabled.
In other places where motion vectors are disabled, such as multi-mesh
and mesh rendering (where previous transform is updated), the logic
effectively allows for a single-frame gap in updates, because it
compares the frame where the update happened (which is the current frame
if updates are consistent) with the current frame, so the latency of 0
means "update just happened", but both multi-mesh and mesh transform
updates permit a latency of 1 as well.
Here, however, last_change is updated *after* the frame processing has
concluded, so a zero-latency update has a distance of 1. Allowing a
distance of 2 (latency 1) reduces the severity of the problem and aligns
the logic with transform updates.
Note that the problem will still happen when refresh rate is noticeably
higher than physics rate times 2. For example, it still happens at 240
Hz. However, a longer latency allowance is inconsistent with other
transforms and could lead to issues, so ideally long term physical
interpolation of skeleton transforms would completely solve this.
Before this change, using FSR2 resulted in the following error when the
effect was destroyed:
ERROR: Attempted to free invalid ID: 662734928609453
at: _free_internal (servers/rendering/rendering_device.cpp:4957)
This happened because ACCUMULATE and ACCUMULATE_SHARPEN passes shared
the same shader_version object but had different pipeline IDs. When
version_free was called for ACCUMULATE pass, it destroyed pipelines
created from that version, including the pipeline for the
ACCUMULATE_SHARPEN pass.
Using a unique version could work around this problem, but it's easier
to rely on version_free destroying the created pipelines through the
dependency mechanism.
- Use negative clip space values to fix reversed rotations in reflections
- Use constant -z view vector when raymarching to fix perspective in reflections
Use correct shadow sampling for omni and spot lights
Disable transmittance if shadows are disabled
Correct DirectionalLight transmittance bias to match shadow bias (its still pretty sensitive though)
Using 2.2.7.dev217+g10c2abcf.
Had to add `colour` to the ignore list as we used it as an alias/keyword for the
documentation of color-related APIs.
Also ignore recommendations to change `thirdparty` to either `third-party` or
`third party`, which are correct but we use the former fairly consistently.
Random-access access to `List` when iterating is `O(n^2)` (`O(n)` when
accessing a single element)
* Removed subscript operator, in favor of a more explicit `get`
* Added conversion from `Iterator` to `ConstIterator`
* Remade existing operations into other solutions when applicable
Initially 3d had premulT alpha as a keyword.
Since Canvas item uses mixed premul and premult as keywords,
3D is changed as well to keep consistency with 2D.
Unfortunately this keeps inconsistency with the internal ENUM.
* Servers now use WorkerThreadPool for background computation.
* This helps keep the number of threads used fixed at all times.
* It also ensures everything works on HTML5 with threads.
* And makes it easier to support disabling threads for also HTML5.
CommandQueueMT now syncs with the servers via the WorkerThreadPool
yielding mechanism, which makes its classic main sync semaphore
superfluous.
Also, some warnings about calls that kill performance when using
threaded rendering are removed because there's a mechanism that
warns about that in a more general fashion.
Co-authored-by: Pedro J. Estébanez <pedrojrulez@gmail.com>
Dario
Matias N. Goldberg
clayjohn
landervr
Thaddeus Crews
CrazyRoka
Wagner
ze2j
Florent Guiocheau
Bastiaan Olij
Chaosus
BlueCube3310
Chris
David House
jadeharley2
tetrapod00
EnlightenedOne
Stuart Carnie
Adam Scott
Yevhen Babiichuk (DustDFG)
Hugo Locurcio
Lain
m-pranav-r
Nazarii
Patrick Owen
Rémi Verschelde
ywmaa
Rie
Jiang Yiheng
David Snopek
Alexis Breust
ComycSans
viksl
Radiant
rune-scape
Alula
Rémi Verschelde
Aaron Franke
jsjtxietian
kleonc
Ricardo Buring
Arseny Kapoulkine
kevinkuo52
kobewi
Hannah Crawford
Sen
Feiyue Zhang
Pedro J. Estébanez
Alexander Hartmann
Garteal
Mert Kasar
bruvzg
ueshita
ChristopheClaustre
Rudolph Bester
Rob Blanckaert
A Thousand Ships
QbieShay
Chris Clyne
tactical-fluke
cosparks
Oxi
Juan Linietsky
Khasehemwy