godot
godot copied to clipboard
Add debug utilities for Vulkan
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:
- Shaders are more aggressively unloaded
- 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.
PR comments
This is the first PR from the TheForge changed. This one is the most important one because:
- It sets the foundation for the rest of the upcoming changes.
- 99% of these changes should be harmless. They are not compiled in non-debug and non-dev builds.
- They're quite verbose in terms of lines of code, so getting this out of the way really reduces the mental load.
Update
Thanks everyone for the help! Most of the issues have been addressed. Almost all CI passes now. Windows now compiles.
3 outstanding things remain:
- ~Turning breadcrumbs into a single
uint32_t. I want the enum to still be defined in rendering_device_commons.h and document it better.~ Done. - ~CI is complaining about draw_list_begin changing its signature (this is correct; although I'm not sure if the error is triggering because the bindings are wrong?). Help & Tips on this are appreciated.~
- ~I've been talking with Dario, and it appears TheForge added
RenderingDevice::shader_destroy_moduleswhich must be called by hand to free (unused) Shader modules after creating the PSO. We both agree it seems like adding a shotgun to the API just to save few kilobytes of RAM (maybe a few megabytes?). I will remove it from this PR and ask TheForge for further info (perhaps it's a lot more RAM than we think?). It may also be a better idea to destroy shader modules after each PSO creation (unless they're shared), instead of doing it at higher level.~ Done for now. We'll revisit this later.
I've made an update. Look at the "3 outstanding things remain:" in the top post where I ask for a bit of help :smile:
@DarioSamo pointed me how to make a backwards compatible C# binding (by editing rendering_device.compat.inc).
However CI is still failing.
I'm not sure if I did something wrong, but I noticed that we may be in a bit of a pickle: draw_list_begin already had a compatibility method and I suspect this is causing problems:
Previous version before my PR:
DrawListID draw_list_begin(RID p_framebuffer, InitialAction p_initial_color_action, FinalAction p_final_color_action, InitialAction p_initial_depth_action, FinalAction p_final_depth_action, const Vector<Color> &p_clear_color_values = Vector<Color>(), float p_clear_depth = 1.0, uint32_t p_clear_stencil = 0, const Rect2 &p_region = Rect2());
New Version:
DrawListID draw_list_begin(RID p_framebuffer, InitialAction p_initial_color_action, FinalAction p_final_color_action, InitialAction p_initial_depth_action, FinalAction p_final_depth_action, const Vector<Color> &p_clear_color_values = Vector<Color>(), float p_clear_depth = 1.0, uint32_t p_clear_stencil = 0, const Rect2 &p_region = Rect2(), uint32_t p_breadcrumb = 0);
That is, my version just adds uint32_t p_breadcrumb = 0 at the end.
Old version before PR #84976:
DrawListID draw_list_begin(RID p_framebuffer, InitialAction p_initial_color_action, FinalAction p_final_color_action, InitialAction p_initial_depth_action, FinalAction p_final_depth_action, const Vector<Color> &p_clear_color_values, float p_clear_depth, uint32_t p_clear_stencil, const Rect2 &p_region, const TypedArray<RID> &p_storage_textures);
That is, this version used to have const TypedArray<RID> &p_storage_textures at the end, but it got removed.
The problem is that the very old version used to have const TypedArray<RID> &p_storage_textures, then it got removed, and now I'm adding a new parameter uint32_t p_breadcrumb = 0 which are clashing.
I'm not sure if this is simply a CI bug where the compat test gets confused, or it will fail in real C#.
The only solution I can think of is to add draw_list_begin2 and mark draw_list_begin as deprecated.
All checks passed!
I've rebased to master again now that we're in 4.4.
I already know the CI will fail because running ./misc/scripts/validate_extension_api.sh /home/matias/SlowProjects/godot/bin/godot.linuxbsd.editor.dev.x86_64.llvm locally gave me this output:
./misc/scripts/validate_extension_api.sh /home/matias/SlowProjects/godot/bin/godot.linuxbsd.editor.dev.x86_64.llvm
2024-08-17 13:19:40 URL:https://raw.githubusercontent.com/godotengine/godot-cpp/godot-4.1-stable/gdextension/extension_api.json [5318978/5318978] -> "/tmp/tmp.jxZm9vNVhd" [1]
Godot Engine v4.4.dev.custom_build.1bd740d18 (2024-08-16 22:47:17 UTC) - https://godotengine.org
ERROR: New API lacks base array: enums
at: compare_dict_array (core/extension/extension_api_dump.cpp:1362)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/draw_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_begin': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
Compatibility to 4.1-stable is broken in the following ways:
Validate extension JSON: API was removed: classes/OpenXRAction
Validate extension JSON: API was removed: classes/OpenXRActionMap
Validate extension JSON: API was removed: classes/OpenXRActionSet
Validate extension JSON: API was removed: classes/OpenXRHand
Validate extension JSON: API was removed: classes/OpenXRInteractionProfile
Validate extension JSON: API was removed: classes/OpenXRInterface
Validate extension JSON: API was removed: classes/OpenXRIPBinding
Validate extension JSON: API was removed: classes/WebXRInterface
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': default_value changed value in new API, from "Array[RID]([])" to "0".
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': type changed value in new API, from "typedarray::RID" to "int".
2024-08-17 13:19:41 URL:https://raw.githubusercontent.com/godotengine/godot-cpp/godot-4.3-stable/gdextension/extension_api.json [5907082/5907082] -> "/tmp/tmp.vFi2XPNiA1" [1]
Godot Engine v4.4.dev.custom_build.1bd740d18 (2024-08-16 22:47:17 UTC) - https://godotengine.org
Compatibility to 4.3-stable is broken in the following ways:
Validate extension JSON: API was removed: classes/OpenXRAction
Validate extension JSON: API was removed: classes/OpenXRActionMap
Validate extension JSON: API was removed: classes/OpenXRActionSet
Validate extension JSON: API was removed: classes/OpenXRAPIExtension
Validate extension JSON: API was removed: classes/OpenXRCompositionLayer
Validate extension JSON: API was removed: classes/OpenXRCompositionLayerCylinder
Validate extension JSON: API was removed: classes/OpenXRCompositionLayerEquirect
Validate extension JSON: API was removed: classes/OpenXRCompositionLayerQuad
Validate extension JSON: API was removed: classes/OpenXRExtensionWrapperExtension
Validate extension JSON: API was removed: classes/OpenXRHand
Validate extension JSON: API was removed: classes/OpenXRInteractionProfile
Validate extension JSON: API was removed: classes/OpenXRInteractionProfileMetadata
Validate extension JSON: API was removed: classes/OpenXRInterface
Validate extension JSON: API was removed: classes/OpenXRIPBinding
Validate extension JSON: API was removed: classes/WebXRInterface
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments': size changed value in new API, from 9 to 10.
Validate extension JSON: Error: Hash changed for 'classes/RenderingDevice/methods/draw_list_begin', from A0225762 to FD7F8214. This means that the function has changed and no compatibility function was provided.
2024-08-17 13:19:43 URL:https://raw.githubusercontent.com/godotengine/godot-cpp/godot-4.0-stable/gdextension/extension_api.json [5175722/5175722] -> "/tmp/tmp.VCeG94saQZ" [1]
Godot Engine v4.4.dev.custom_build.1bd740d18 (2024-08-16 22:47:17 UTC) - https://godotengine.org
ERROR: New API lacks base array: enums
at: compare_dict_array (core/extension/extension_api_dump.cpp:1362)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/draw_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_begin': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
Compatibility to 4.0-stable is broken in the following ways:
Validate extension JSON: API was removed: classes/OpenXRAction
Validate extension JSON: API was removed: classes/OpenXRActionMap
Validate extension JSON: API was removed: classes/OpenXRActionSet
Validate extension JSON: API was removed: classes/OpenXRHand
Validate extension JSON: API was removed: classes/OpenXRInteractionProfile
Validate extension JSON: API was removed: classes/OpenXRInterface
Validate extension JSON: API was removed: classes/OpenXRIPBinding
Validate extension JSON: API was removed: classes/WebXRInterface
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': default_value changed value in new API, from "Array[RID]([])" to "0".
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': type changed value in new API, from "Array" to "int".
2024-08-17 13:19:45 URL:https://raw.githubusercontent.com/godotengine/godot-cpp/godot-4.2-stable/gdextension/extension_api.json [5501700/5501700] -> "/tmp/tmp.fxi2O3Si4Z" [1]
Godot Engine v4.4.dev.custom_build.1bd740d18 (2024-08-16 22:47:17 UTC) - https://godotengine.org
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/draw_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_begin': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
Compatibility to 4.2-stable is broken in the following ways:
Validate extension JSON: API was removed: classes/OpenXRAction
Validate extension JSON: API was removed: classes/OpenXRActionMap
Validate extension JSON: API was removed: classes/OpenXRActionSet
Validate extension JSON: API was removed: classes/OpenXRAPIExtension
Validate extension JSON: API was removed: classes/OpenXRExtensionWrapperExtension
Validate extension JSON: API was removed: classes/OpenXRHand
Validate extension JSON: API was removed: classes/OpenXRInteractionProfile
Validate extension JSON: API was removed: classes/OpenXRInteractionProfileMetadata
Validate extension JSON: API was removed: classes/OpenXRInterface
Validate extension JSON: API was removed: classes/OpenXRIPBinding
Validate extension JSON: API was removed: classes/WebXRInterface
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': default_value changed value in new API, from "Array[RID]([])" to "0".
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': type changed value in new API, from "typedarray::RID" to "int".
I recognized this error when going 4.3 -> 4.4:
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments': size changed value in new API, from 9 to 10.
This is expected and should be silenced because I added uint32_t p_breadcrumb = 0 to the following function:
DrawListID draw_list_begin(RID p_framebuffer, InitialAction p_initial_color_action, FinalAction p_final_color_action, InitialAction p_initial_depth_action, FinalAction p_final_depth_action, const Vector<Color> &p_clear_color_values = Vector<Color>(), float p_clear_depth = 1.0, uint32_t p_clear_stencil = 0, const Rect2 &p_region = Rect2(), uint32_t p_breadcrumb = 0);
However this error is new and I don't know what it means or whether it's safe to ignore:
Validate extension JSON: Error: Hash changed for 'classes/RenderingDevice/methods/draw_list_begin', from A0225762 to FD7F8214. This means that the function has changed and no compatibility function was provided.
I think you lost the changes to misc/extension_api_validation/4.2-stable.expected when rebasing. Pre-rebase diff: https://github.com/godotengine/godot/commit/3364acaaff8db28d24822b3fcb8b4a5693fc3c92#diff-2cc9ae1407b900dfd30f2edfeef07c9ae8cb452bac6149600a455ad1ba9f2c09
The file has been renamed to 4.2-stable_4.3-stable.expected since 4.3-stable was released, and you need to add new stuff that breaks compat with 4.3 in the new 4.3-stable.expected file.
The missing hash error is weird, I'll check later.
I think you lost the changes to misc/extension_api_validation/4.2-stable.expected when rebasing. Pre-rebase diff: https://github.com/godotengine/godot/commit/3364acaaff8db28d24822b3fcb8b4a5693fc3c92#diff-2cc9ae1407b900dfd30f2edfeef07c9ae8cb452bac6149600a455ad1ba9f2c09
Oh yes, I forgot to mention that I stripped them on purpose. A refresher: The original PR ran into a validation bug because it was the first time a function (draw_list_begin) was modified twice in the same version (when going 4.2 -> 4.3) by two different PRs.
If I add to 4.3-stable.expected:
GH-90993
--------
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments': size changed value in new API, from 9 to 10.
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': type changed value in new API, from "Array" to "int".
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': default_value changed value in new API, from "Array[RID]([])" to "0".
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': type changed value in new API, from "typedarray::RID" to "int".
draw_list_begin added a new optional debug argument called breadcrumb.
There used to be an RID argument as arg #9 which was removed in GH-84976 but now we're adding a new one in the same location.
No compatibility method was registered because the one created for GH-84976 is already handling this case.
I get:
./misc/scripts/validate_extension_api.sh /home/matias/SlowProjects/godot/bin/godot.linuxbsd.editor.dev.x86_64.llvm
2024-08-17 14:37:31 URL:https://raw.githubusercontent.com/godotengine/godot-cpp/godot-4.1-stable/gdextension/extension_api.json [5318978/5318978] -> "/tmp/tmp.Lc8vSIOSbi" [1]
Godot Engine v4.4.dev.custom_build.1bd740d18 (2024-08-16 22:47:17 UTC) - https://godotengine.org
ERROR: New API lacks base array: enums
at: compare_dict_array (core/extension/extension_api_dump.cpp:1362)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/draw_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_begin': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
Compatibility to 4.1-stable is broken in the following ways:
Validate extension JSON: API was removed: classes/OpenXRAction
Validate extension JSON: API was removed: classes/OpenXRActionMap
Validate extension JSON: API was removed: classes/OpenXRActionSet
Validate extension JSON: API was removed: classes/OpenXRHand
Validate extension JSON: API was removed: classes/OpenXRInteractionProfile
Validate extension JSON: API was removed: classes/OpenXRInterface
Validate extension JSON: API was removed: classes/OpenXRIPBinding
Validate extension JSON: API was removed: classes/WebXRInterface
2024-08-17 14:37:33 URL:https://raw.githubusercontent.com/godotengine/godot-cpp/godot-4.3-stable/gdextension/extension_api.json [5907082/5907082] -> "/tmp/tmp.PNkdLuSnW5" [1]
Godot Engine v4.4.dev.custom_build.1bd740d18 (2024-08-16 22:47:17 UTC) - https://godotengine.org
The following validation errors no longer occur (compared to 4.3-stable):
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': default_value changed value in new API, from "Array[RID]([])" to "0".
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': type changed value in new API, from "Array" to "int".
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': type changed value in new API, from "typedarray::RID" to "int".
Compatibility to 4.3-stable is broken in the following ways:
Validate extension JSON: API was removed: classes/OpenXRAction
Validate extension JSON: API was removed: classes/OpenXRActionMap
Validate extension JSON: API was removed: classes/OpenXRActionSet
Validate extension JSON: API was removed: classes/OpenXRAPIExtension
Validate extension JSON: API was removed: classes/OpenXRCompositionLayer
Validate extension JSON: API was removed: classes/OpenXRCompositionLayerCylinder
Validate extension JSON: API was removed: classes/OpenXRCompositionLayerEquirect
Validate extension JSON: API was removed: classes/OpenXRCompositionLayerQuad
Validate extension JSON: API was removed: classes/OpenXRExtensionWrapperExtension
Validate extension JSON: API was removed: classes/OpenXRHand
Validate extension JSON: API was removed: classes/OpenXRInteractionProfile
Validate extension JSON: API was removed: classes/OpenXRInteractionProfileMetadata
Validate extension JSON: API was removed: classes/OpenXRInterface
Validate extension JSON: API was removed: classes/OpenXRIPBinding
Validate extension JSON: API was removed: classes/WebXRInterface
Validate extension JSON: Error: Hash changed for 'classes/RenderingDevice/methods/draw_list_begin', from A0225762 to FD7F8214. This means that the function has changed and no compatibility function was provided.
2024-08-17 14:37:35 URL:https://raw.githubusercontent.com/godotengine/godot-cpp/godot-4.0-stable/gdextension/extension_api.json [5175722/5175722] -> "/tmp/tmp.qjpaqUWytN" [1]
Godot Engine v4.4.dev.custom_build.1bd740d18 (2024-08-16 22:47:17 UTC) - https://godotengine.org
ERROR: New API lacks base array: enums
at: compare_dict_array (core/extension/extension_api_dump.cpp:1362)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/draw_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_begin': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
Compatibility to 4.0-stable is broken in the following ways:
Validate extension JSON: API was removed: classes/OpenXRAction
Validate extension JSON: API was removed: classes/OpenXRActionMap
Validate extension JSON: API was removed: classes/OpenXRActionSet
Validate extension JSON: API was removed: classes/OpenXRHand
Validate extension JSON: API was removed: classes/OpenXRInteractionProfile
Validate extension JSON: API was removed: classes/OpenXRInterface
Validate extension JSON: API was removed: classes/OpenXRIPBinding
Validate extension JSON: API was removed: classes/WebXRInterface
2024-08-17 14:37:37 URL:https://raw.githubusercontent.com/godotengine/godot-cpp/godot-4.2-stable/gdextension/extension_api.json [5501700/5501700] -> "/tmp/tmp.5KzMdyqfYe" [1]
Godot Engine v4.4.dev.custom_build.1bd740d18 (2024-08-16 22:47:17 UTC) - https://godotengine.org
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/draw_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_begin': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
ERROR: Validate extension JSON: Missing field in current API 'classes/RenderingDevice/methods/compute_list_end': arguments. This is a bug.
at: compare_dict_array (core/extension/extension_api_dump.cpp:1443)
Compatibility to 4.2-stable is broken in the following ways:
Validate extension JSON: API was removed: classes/OpenXRAction
Validate extension JSON: API was removed: classes/OpenXRActionMap
Validate extension JSON: API was removed: classes/OpenXRActionSet
Validate extension JSON: API was removed: classes/OpenXRAPIExtension
Validate extension JSON: API was removed: classes/OpenXRExtensionWrapperExtension
Validate extension JSON: API was removed: classes/OpenXRHand
Validate extension JSON: API was removed: classes/OpenXRInteractionProfile
Validate extension JSON: API was removed: classes/OpenXRInteractionProfileMetadata
Validate extension JSON: API was removed: classes/OpenXRInterface
Validate extension JSON: API was removed: classes/OpenXRIPBinding
Validate extension JSON: API was removed: classes/WebXRInterface
There's a lot of "The following validation errors no longer occur" warnings.
IMHO the validation tool seems like is starting to fall short and needs to be improved to handle the complexities of new versions, as it seems to be completely unaware of incremental changes 4.0 -> 4.1 -> 4.2 -> 4.3 -> 4.4.
The missing hash error is weird, I'll check later.
Yeah :crying_cat_face:
IMHO the validation tool seems like is starting to fall short and needs to be improved to handle the complexities of new versions, as it seems to be completely unaware of incremental changes 4.0 -> 4.1 -> 4.2 -> 4.3 -> 4.4.
Yeah definitely. I'll see if I can find how to hack things to pass CI so we can merge this, and then how to actually improve the validation tool to be more robust.
So the compat method is needed, each change to the API needs to have a compat method, as it's used to generate a hash of the method that GDExtension uses to call it. It's not enough to have source level compatibility in C++, we need binary compatibility.
As for the validation errors caused by the differences between this PR and 4.0-stable and 4.2-stable APIs, I put the message to silence them in their respective .expected files, but that's a hack. I'll open an issue so we rework this further, I think we might have bugs either in the validation script, or in the in-engine validation checks.
- Edit: Done: https://github.com/godotengine/godot/issues/95858.
Changes in https://github.com/godotengine/godot/pull/90993/commits/a126cdc37df3275de2afae4e5fe9fc356be2ccb7, to be squashed if it passes CI.
After digging into issue #95858 a little bit, I personally think a better workaround would be to put the additional type change messages into 4.3-stable.expected, rather than in the old 4.2-stable_4.3-stable.expected and 4.0-stable_4.1-stable.expected, because we really shouldn't be changing those files after we've moved on to a new minor version.
So, basically this patch:
Patch
diff --git a/misc/extension_api_validation/4.0-stable_4.1-stable.expected b/misc/extension_api_validation/4.0-stable_4.1-stable.expected
index 2b7f9dc6623..5c3bf07fb20 100644
--- a/misc/extension_api_validation/4.0-stable_4.1-stable.expected
+++ b/misc/extension_api_validation/4.0-stable_4.1-stable.expected
@@ -349,11 +349,3 @@ Validate extension JSON: Error: Hash changed for 'classes/EditorUndoRedoManager/
Validate extension JSON: Error: Hash changed for 'classes/UndoRedo/methods/create_action', from 0AEC1BFC to E87757EB. This means that the function has changed and no compatibility function was provided.
Added a optional parameters with default values. No adjustments should be necessary.
-
-
-GH-90993
---------
-Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': type changed value in new API, from "Array" to "int".
-
-FIXME: Hack to workaround a bug in our validation logic with a method that changed arguments count, type, and default values multiple times.
-The actual last change triggering this error happened in 4.4 (GH-90993).
diff --git a/misc/extension_api_validation/4.2-stable_4.3-stable.expected b/misc/extension_api_validation/4.2-stable_4.3-stable.expected
index 3de9736c8da..ce8f24c7a96 100644
--- a/misc/extension_api_validation/4.2-stable_4.3-stable.expected
+++ b/misc/extension_api_validation/4.2-stable_4.3-stable.expected
@@ -380,12 +380,3 @@ Validate extension JSON: Error: Field 'classes/Image/methods/get_mipmap_offset/r
Type changed to int64_t to support baking large lightmaps.
No compatibility method needed, both GDExtension and C# generate it as int64_t anyway.
-
-
-GH-90993
---------
-Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': default_value changed value in new API, from "Array[RID]([])" to "0".
-Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': type changed value in new API, from "typedarray::RID" to "int".
-
-FIXME: Hack to workaround a bug in our validation logic with a method that changed arguments count, type, and default values multiple times.
-The actual last change triggering this error happened in 4.4 (GH-90993).
diff --git a/misc/extension_api_validation/4.3-stable.expected b/misc/extension_api_validation/4.3-stable.expected
index 5c869d99140..7c6a25621b0 100644
--- a/misc/extension_api_validation/4.3-stable.expected
+++ b/misc/extension_api_validation/4.3-stable.expected
@@ -19,6 +19,9 @@ These getters have been renamed to expose them. GDExtension language bindings co
GH-90993
--------
Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments': size changed value in new API, from 9 to 10.
+Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': type changed value in new API, from "Array" to "int".
+Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': default_value changed value in new API, from "Array[RID]([])" to "0".
+Validate extension JSON: Error: Field 'classes/RenderingDevice/methods/draw_list_begin/arguments/9': type changed value in new API, from "typedarray::RID" to "int".
draw_list_begin added a new optional debug argument called breadcrumb.
There used to be an RID argument as arg #9 which was removed in GH-84976 but now we're adding a new one in the same location.
But don't go ahead and change it right away - we should discuss and see if others agree (namely @akien-mga).
So the compat method is needed, each change to the API needs to have a compat method, as it's used to generate a hash of the method that GDExtension uses to call it. It's not enough to have source level compatibility in C++, we need binary compatibility.
Somehow I feel this paragraph you just wrote needs to be in https://docs.godotengine.org/en/stable/contributing/development/handling_compatibility_breakages.html
@dsnopek I agree, and tried it, but then our shell script complains that the 4.3.expected contains lines which are no longer relevant for the 4.4 API. That in itself might be the main bug to fix for this case.
@akien-mga:
I agree, and tried it, but then our shell script complains that the
4.3.expectedcontains lines which are no longer relevant for the 4.4 API. That in itself might be the main bug to fix for this case.
Those messages are technically "warnings" in that it shouldn't cause the CI to fail (at least based on my reading of the shell script). Would some extra non-fatal warnings be acceptable in a workaround for now?
If we decide (as part of the discussion on #95858) that we don't want to have those warnings at all, we can certainly remove them, but I don't personally think that should necessarily block merging this PR.
I did the changes suggested by @dsnopek, and thankfully I also rebased and that let us find that some adjustments are needed after #88199.
drivers/metal/rendering_context_driver_metal.mm:73:16: error: allocating an object of abstract class type 'RenderingDeviceDriverMetal' [2]
return memnew(RenderingDeviceDriverMetal(this));
^
./servers/rendering/rendering_device_driver.h:479:15: note: unimplemented pure virtual method 'shader_destroy_modules' in 'RenderingDeviceDriverMetal' [2]
virtual void shader_destroy_modules(ShaderID p_shader) = 0;
^
./servers/rendering/rendering_device_driver.h:716:15: note: unimplemented pure virtual method 'command_insert_breadcrumb' in 'RenderingDeviceDriverMetal' [2]
virtual void command_insert_breadcrumb(CommandBufferID p_cmd_buffer, uint32_t p_data) = 0;
^
I suppose those just need to be added to the Metal backend too.
I'll put stubs like for D3D12, and let @stuartcarnie look at it eventually.
Thanks!
WHOA!!! Thanks!