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

Logging nbl::video device call failures #684

Open
Erfan-Ahmadi opened this issue Apr 27, 2024 · 3 comments
Open

Logging nbl::video device call failures #684

Erfan-Ahmadi opened this issue Apr 27, 2024 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@Erfan-Ahmadi
Copy link
Contributor

Problem

Many device functions return a boolean to indicate a successful call to lower level api (Vulkan) function.
But the return value is often ignored making it hard to debug the cause of the problem.
Even if not ignored, simply returning false on a failure doesn't help much pinpointing the exact cause.
There is also createXXX functions that may return nullptr due to reasons such as invalid creation params
for example inside ILogicalDevice::createFramebuffer

if (!params.validate())
    return nullptr;

may fail due to 30+ reasons specified in vulkan specification and handled manually in IFramebuffer::SCreationParams::validate

that's why we need to "error log" the exact cause of the failure each time we return false on device calls

Solution

use system::logger_opt_ptr m_logger in ILogicalDevice` to log every possible scenario that fails.

@Erfan-Ahmadi Erfan-Ahmadi added the enhancement New feature or request label Apr 27, 2024
@AnastaZIuk AnastaZIuk added help wanted Extra attention is needed good first issue Good for newcomers labels Apr 27, 2024
@devshgraphicsprogramming
Copy link
Member

Btw it would be enough to just

log("Short Message: `%s` in `%s` at line %p.",system::ILogger::ELL_ERROR,__FILE__,__FUNCTION__,__LINE__);

Instead of writing custom exact log messages, because we're an Open Source project, source is there and the extended comments about why something fails are in the source anyway.

@devshgraphicsprogramming
Copy link
Member

now thinking of it, it would be useful to also have an _NBL_COMMIT_HASH_ in there too in case someone runs non-development exes without a local copy of the source

@Erfan-Ahmadi
Copy link
Contributor Author

Exactly what i was thinking about

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants