Add diagnostic logging to Forward+ clustered renderer#116398
Add diagnostic logging to Forward+ clustered renderer#116398glkdrlgkrlzflnjkgh wants to merge 4 commits intogodotengine:masterfrom
Conversation
Signed-off-by: Callum Nicoll <xcallumnicx@outlook.com>
|
This PR adds more detail to what is logged when the render pipeline creation in 3D forward+ clustered wirth vulkan fails |
|
OH. sorry about the code formatting. |
AR-DEV-1
left a comment
AR-DEV-1
left a comment
There was a problem hiding this comment.
Please remove the comments & set up pre-commit hooks & formatting locally.
Take a look at https://contributing.godotengine.org/en/latest/engine/guidelines/code_style.html for more information.
|
Can you describe why this additional print is needed? Are you trying to troubleshoot a crash that occurs on a player's device? |
Signed-off-by: Callum Nicoll <xcallumnicx@outlook.com>
|
I have ran clang-format and setup the pre commit hooks. |
|
I have updated this PR with a new commit. |
| ERR_FAIL_COND_MSG( | ||
| pipeline.is_null(), | ||
| vformat("Forward+ pipeline creation failed for key %llu (version=%d, flags=%u, ubershader=%d).", | ||
| (unsigned long long)p_pipeline_key.hash(), |
There was a problem hiding this comment.
Build error here. It is not clear to Godot on why you want to convert it. The error:
conversion from 'const unsigned long long' to 'Variant' is ambiguous
There was a problem hiding this comment.
How would I convert a unsigned long long to a Variant in a non-ambigious way?
(Since I am relatively new to the godot codebase. and I've never really contributed to any big open source projects before.)
There was a problem hiding this comment.
You cast it to int64_t. Variant is uint64_t for your information. You could do something like:
uint64_t my_u_long = 123456789012345ULL;
Variant v = (int64_t)my_u_long; // SafeThere was a problem hiding this comment.
I have cast p_pipeline_key's hash return to int64_t
There was a problem hiding this comment.
Will compile the engine again and send this to the branch.
There was a problem hiding this comment.
Also. I opened a project (the 3D platformer starter by Kenney) and I got this in the console. no crash. but i think its unrelated to my changes. ERROR: Bug: Dictionary::operator[] used when there was no value for the given key "colormap". Please report.
There was a problem hiding this comment.
Should I resolve this now
There was a problem hiding this comment.
Im pretty much ready to re-request a review
Signed-off-by: Callum Nicoll <xcallumnicx@outlook.com>
Signed-off-by: Callum Nicoll <xcallumnicx@outlook.com>
|
good. 20/20 checks passed. now all i need is someone to review it. |
|
I am not sure how those changes occured. Although I could diff and take a look?
…________________________________
From: Hugo Locurcio ***@***.***>
Sent: 07 March 2026 5:53 PM
To: godotengine/godot ***@***.***>
Cc: Callum ***@***.***>; Author ***@***.***>
Subject: Re: [godotengine/godot] Add diagnostic logging to Forward+ clustered renderer (PR #116398)
@Calinou requested changes on this pull request.
You should undo the changes that were done to platform/web/package-lock.json, as they are unrelated.
—
Reply to this email directly, view it on GitHub<#116398 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A3FHBBKZYGKKSXLZUGIONHT4PROZJAVCNFSM6AAAAACVMCHJ5GVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTSMBZGIZDEOJQGM>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
The file change to the package lock was caused by my version of npm, I think it could be newer than the one that was originally used to generate the file. sorry about that. |
5 participants
This PR adds logging to the forward+ clustered renderer when it fails to be created. the second modified file? I honestly have no idea what modified it.