Build: replace Blender specific DEBUG by standard NDEBUG #115774

Brecht Van Lommel · 2023-12-04T23:06:32+01:00

Brecht Van Lommel commented

2023-12-04 23:06:32 +01:00

Build: replace Blender specific DEBUG by standard NDEBUG

NDEBUG is part of the C standard and disables asserts. Only this will
now be used to decide if asserts are enabled.

DEBUG was a Blender specific define, that has now been removed.

_DEBUG is a Visual Studio define for builds in Debug configuration.
Blender defines this for all platforms. This is still used in a few
places in the draw code, and in external libraries Bullet and Mantaflow.

Build: replace Blender specific DEBUG by standard NDEBUG NDEBUG is part of the C standard and disables asserts. Only this will now be used to decide if asserts are enabled. DEBUG was a Blender specific define, that has now been removed. _DEBUG is a Visual Studio define for builds in Debug configuration. Blender defines this for all platforms. This is still used in a few places in the draw code, and in external libraries Bullet and Mantaflow.

👍 4

Brecht Van Lommel commented

2023-12-04 23:10:27 +01:00

@blender-bot build

Brecht Van Lommel force-pushed ndebug from fcc77f4ff8 to d1f1f1387e

2023-12-05 02:35:41 +01:00

Compare

Brecht Van Lommel referenced this pull request

2023-12-05 02:39:47 +01:00

CMake: Allow asserts to be enabled for release builds. #115071

Brecht Van Lommel commented

2023-12-05 02:49:42 +01:00

Guessing some developers that might have opinions on this: @ideasman42 @mont29 @JacquesLucke @HooglyBoogly

I'm trying to move forward #115071, so that we can enable asserts on the buildbot. To me it seems that we should either define both DEBUG and NDEBUG as exact opposites of each other, or only define NDEBUG as this PR does.

I don't feel very strongly about it either way, just slightly prefer using only the standard C NDEBUG.

Guessing some developers that might have opinions on this: @ideasman42 @mont29 @JacquesLucke @HooglyBoogly I'm trying to move forward #115071, so that we can enable asserts on the buildbot. To me it seems that we should either define both DEBUG and NDEBUG as exact opposites of each other, or only define NDEBUG as this PR does. I don't feel very strongly about it either way, just slightly prefer using only the standard C NDEBUG.

Brecht Van Lommel commented

2023-12-05 02:52:09 +01:00

@fclem @pragma37 I'm not confident at all about the changes in 15e079fd4f421edd529edadc546153294c95a9f7. But if we are going to allow asserts outside the Debug configuration, we need to clarify somehow what happens with these somewhat unusual uses of the DEBUG flag. Which CMake option(s) do you want to control this?

@ideasman42 the BMesh threading changes in 5fb1285ca9fa8629059b735ad60775da3c017407 are undoing a change made by you. Not sure if you feel strongly about keeping threading always enabled in Debug configuration for this handful of BMesh functions, and if so which CMake option it should be tied to. Nowadays we have threading in many more places and doing it just here seems a bit strange.

@fclem @pragma37 I'm not confident at all about the changes in 15e079fd4f421edd529edadc546153294c95a9f7. But if we are going to allow asserts outside the Debug configuration, we need to clarify somehow what happens with these somewhat unusual uses of the DEBUG flag. Which CMake option(s) do you want to control this? @ideasman42 the BMesh threading changes in 5fb1285ca9fa8629059b735ad60775da3c017407 are undoing a change made by you. Not sure if you feel strongly about keeping threading always enabled in Debug configuration for this handful of BMesh functions, and if so which CMake option it should be tied to. Nowadays we have threading in many more places and doing it just here seems a bit strange.

👍 1

Bastien Montagne commented

2023-12-05 11:06:53 +01:00

Am totally up to keep only one debug flag, and indeed keeping the C standard NDEBUG one makes the most sense imo.

So big +1 for this cleanup from me.

Am totally up to keep only one debug flag, and indeed keeping the C standard `NDEBUG` one makes the most sense imo. So big +1 for this cleanup from me.

Miguel Pozo commented

2023-12-05 15:52:17 +01:00

I think WITH_DRAW_DEBUG is supposed to be specifically about Draw Manager features.
So I guess it's ok to use it directly in the DrawManager (@fclem ?), but I don't think it makes too much sense to use it for the Workbench case.
That one is there only to be able to debug f12 renders in NSight, so a simple #define USE_GPU_FINISH_DELIMITER 0 and then #if USE_GPU_FINISH_DELIMITER would be ok if you don't want to use #ifndef NDEBUG.

I think `WITH_DRAW_DEBUG` is supposed to be specifically about Draw Manager features. So I guess it's ok to use it directly in the `DrawManager` (@fclem ?), but I don't think it makes too much sense to use it for the Workbench case. That one is there only to be able to debug f12 renders in NSight, so a simple `#define USE_GPU_FINISH_DELIMITER 0` and then `#if USE_GPU_FINISH_DELIMITER` would be ok if you don't want to use `#ifndef NDEBUG`.

Brecht Van Lommel force-pushed ndebug from d1f1f1387e to 6ba31a0863

2023-12-06 15:36:31 +01:00

Compare

Brecht Van Lommel changed title from ~~WIP: Build: replace Blender specific DEBUG by standard NDEBUG~~ to Build: replace Blender specific DEBUG by standard NDEBUG

2023-12-06 15:37:06 +01:00

Brecht Van Lommel commented

2023-12-06 16:03:52 +01:00

I've left the draw manager behavior unchanged now so I can merge this and test buildbot side changes.

The draw manager is using _DEBUG instead of the removed DEBUG. Still probably a good idea to eliminate different behavior depending on the build configuration, but no longer a blocker for #115071.

I've left the draw manager behavior unchanged now so I can merge this and test buildbot side changes. The draw manager is using `_DEBUG` instead of the removed `DEBUG`. Still probably a good idea to eliminate different behavior depending on the build configuration, but no longer a blocker for #115071.

Brecht Van Lommel merged commit e06561a27a into main

2023-12-06 16:05:26 +01:00

Brecht Van Lommel referenced this issue from a commit

2023-12-06 16:05:27 +01:00

Build: replace Blender specific DEBUG by standard NDEBUG

Brecht Van Lommel deleted branch ndebug

2023-12-06 16:05:28 +01:00

Brian Savery (AMD) referenced this issue from a commit

2023-12-28 20:07:43 +01:00

Build: replace Blender specific DEBUG by standard NDEBUG

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Build: replace Blender specific DEBUG by standard NDEBUG #115774