Fix #120038: Possible fix for macOS buildbot test crash #120432

Open
Jason Fielder wants to merge 1 commits from Jason-Fielder/blender:Fix_120038_PossibleFixForBuildBotCrash into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
3 changed files with 5 additions and 9 deletions
Showing only changes of commit 93307db7d6 - Show all commits

View File

@ -20,7 +20,7 @@ using namespace blender::gpu;
namespace blender::gpu {
/* Counter for active command buffers. */
int MTLCommandBufferManager::num_active_cmd_bufs = 0;
std::atomic<int> MTLCommandBufferManager::num_active_cmd_bufs = 0;
/* -------------------------------------------------------------------- */
/** \name MTLCommandBuffer initialization and render coordination.

View File

@ -552,7 +552,7 @@ class MTLCommandBufferManager {
public:
/* Counter for active command buffers. */
static int num_active_cmd_bufs;
static std::atomic<int> num_active_cmd_bufs;
private:
/* Associated Context and properties. */

View File

@ -38,12 +38,8 @@ using namespace blender;
using namespace blender::gpu;
/* Fire off a single dispatch per encoder. Can make debugging view clearer for texture resources
* associated with each dispatch. */
#if defined(NDEBUG)
Review

IIRC when tests are run on the buildbot it is done on a build where asserts are enabled. In that sense this doesn't make a change in behavior on the buildbot.

@brecht is my recollection correct?

IIRC when tests are run on the buildbot it is done on a build where asserts are enabled. In that sense this doesn't make a change in behavior on the buildbot. @brecht is my recollection correct?

The only purpose of NDEBUG is to turn asserts off, following the C standard.

It is independent of debug / release build configurations, despite the name.

So, this does affect behavior of builds using asserts.

The only purpose of `NDEBUG` is to turn asserts off, following the C standard. It is independent of debug / release build configurations, despite the name. So, this does affect behavior of builds using asserts.

Thanks Brecht, then I do believe we should still land this patch regardless, as it should not be changing GPU dispatch behavior on this flag.

(The flag is still situationally used to wait and validate execution of GPU work, but this can be considered a valid assertion that everything has executed correctly).

Thanks Brecht, then I do believe we should still land this patch regardless, as it should not be changing GPU dispatch behavior on this flag. (The flag is still situationally used to wait and validate execution of GPU work, but this can be considered a valid assertion that everything has executed correctly).
# define MTL_DEBUG_SINGLE_DISPATCH_PER_ENCODER 0
#else
# define MTL_DEBUG_SINGLE_DISPATCH_PER_ENCODER 1
#endif
* associated with each dispatch. NOTE: This should only be used during active debugging. */
#define MTL_DEBUG_SINGLE_DISPATCH_PER_ENCODER 0
/* Debug option to bind null buffer for missing UBOs.
* Enabled by default. TODO: Ensure all required UBO bindings are present. */
@ -2724,7 +2720,7 @@ void present(MTLRenderPassDescriptor *blit_descriptor,
/* Decrement count */
MTLCommandBufferManager::num_active_cmd_bufs--;
MTL_LOG_INFO("Active command buffers: %d", MTLCommandBufferManager::num_active_cmd_bufs);
MTL_LOG_INFO("Active command buffers: %d", int(MTLCommandBufferManager::num_active_cmd_bufs));
/* Drawable count and latency management. */
MTLContext::max_drawables_in_flight--;