Add CaptureMask support by ccp-chargeback · Pull Request #34 · carbonengine/core

ccp-chargeback · 2026-07-17T15:31:17Z

Change includes:

The ability of a component to register for a CaptureMask, with or without specifying a color
If registering a CaptureMask without specifying a color, a color will be automatically chosen.
Get a list of Registered CaptureMasks containing their name, assigned maskBit and color
Set the Active CaptureMask, meaning what components are tracked in a Telemetry session
The active CaptureMask can be set either before or during an active Telemetry session.
An active CaptureMask can be defined either by a bitmask or a list of "component names"
A yet to be registered component from SetActiveCaptureMask(listOfNames) will be added to a "pending" list until the component is registered.
Get the combined bitmask of the active CaptureMasks (excluding the "pending" ones)
Add overloads for both: CcpTelemetryEnterZone() and the TelemetryZone constructor, that accept captureMaskBit as argument

Change contains: - Ability to call RegisterCaptureMask() for a given name and an optional Telemetry Zone display color - Automatically assign a color if one is not provided Part of: https://fenriscreations.atlassian.net/browse/PLAT-11474

…pport

Changed: - enum class Color => CcpColor - namespace CcpColor => ColorUtil

Change includes: - Expose CcpGetRegisteredCaptureMasks() from core - Add/change test coverage for CaptureMasks tests - Rename existing CaptureMasks variables to state they are for "registered" CaptureMasks Part of: https://fenriscreations.atlassian.net/browse/PLAT-11475

Will give us the option of a O(1) lookup for display color once we change telemetry to mark zones based on CaptureMask bit.

Change contains: - Two overloads of CcpSetActiveCaptureMask() function - An exported CcpGetActiveCaptureMask() function - Support for an "all" active CaptureMask - Support for lazy-register of active CaptureMasks by name via a list of pending ones. Part of: https://fenriscreations.atlassian.net/browse/PLAT-11481

Make sure we can verify the color associated with a Zone from the TracyTestClient in tests.

Done because of build problems on v143/v145 vs v141 identified by ccptoebeans.

Add CaptureMaksBit overloads for: - TelemetryZone constructor - CcpTelemetryEnterZone() Make sure tests reflect the new reality. Part of: https://fenriscreations.atlassian.net/browse/PLAT-11476

…pport # Conflicts: # include/CcpTelemetry.h

Rename: CcpGetCaptureMasks() To: CcpGetRegisteredCaptureMasks()

ccpgames-carbon · 2026-07-20T13:18:31Z

All committers have signed the CLA.

Legal hasn't advised on an updated statement yet, so we stick with `CCP ehf.`.

…es for a given color value To avoid unnecessary memory copies of the underlying string data, and to avoid surprises in seeing `CcpColorToString( CcpColor::Cyan )` return `Aqua`.

Instead, have `Color` as a default-parameter that is piped through. This alone shouldn't have required a separately overloaded implementation. Also, stop lower-casing the input name, because it causes a mismatch between what is passed into the system as "display name" versus what comes back out of the system. Additionally, remove the `all` special case handling. Enabling all capture masks can be done by passing in all capture masks. Having a sentinel value that represents a possible legal value is bound to cause surprising behaviour down the line. Sentinel values should be flagged explicitly, e.g. an alternative would have been to have an explicit function like `CcpCaptureEverything()` function. Furthermore, this simplifies initialization of pre-registered captureMasks: why run a lambda function during static initialization stage when we have initializer lists?

For type-safe constants instead of preprocessor text replacement.

The order of registration doesn't matter to any users of `CcpRegisterCaptureMask`, primarily because that order depends on the order of calls to that function, not the implementation itself.

This is for an upcoming QoL improvement for capture mask users, where they should no longer have to rely on the implementation of using a bitmask for the capture mask.

Since the bitmask is only an implementation details, and because `CcpSetActiveCaptureMask` takes in the names, too.

This avoids double-checking whether we can access memory safely further down the test. If we only use EXPECT_* macros some tests may crash if the observed container is empty.

…I compatibility ABI is preserved with the overload as well, since both the `uint32_t` and `uint64_t` version have their distinct mangled names exported. So just dropping in the new library should be just fine. Of course, what it does mean is that downstream consumers of core will need to make some fixes to their code when they get _recompiled_ but that is not breaking _ABI_. That said, semantic versioning is also about the public API. So we are facing a decision here: 1. Accept that we need yet another major version tag for Core 2. Introduce terrible developer ergonomics where anyone who wants to construct the right version of `TelemetryZone` needs to pass in a really awkward sentinel value as first parameter. 3. Alternately, define a `TelemetryZoneWithCaptureMask` subclass that takes the `uint64_t` constructor without requiring the sentinel constant. 4. Last, but not least, change the way that capture mask registration works. Instead of "leaking" the bit mask implementation details through to the caller, we can return the index into the array of registered masks. That's still leaking an implementation details, but we can make that intent clear by calling it a "CaptureMaskHandle" or something similar. I'd argue that we should settle for option 4. Option 1 is annoying because just tagged a new major version for a subset of the telemetry improvements. Option 2 is terrible ergonomics on the call side. Option 3 complicates the API surface: When to use which one? OK, one is deprecated, that gives a hint, but the deprecation also implies that we _are_ going to break ABI and API again in the future. So we may as well just take the pain now. Option 4, on the other hand, does not require changes to function signatures, and therefore retains both ABI and API compatibility. It also means we no longer have to do a reverse-lookup of bitmask to color for a registered capture mask. So this commit is the first step in that direction of option 4.

…capture mask Users of the telemetry instrumentation should pass this handle on when creating a `TelemetryZone` if they want it to be captured as part of their registered mask. Implementation details: This currently represents the index into the array of registered capture masks, as that keeps the look up of name and color cheap.

CCP-Aporia · 2026-07-20T12:28:40Z

+// - An active CaptureMask defaults to "all" but can be set/narrowed before or
+//   during a Telemetry session is started using either:
+//   - a numerical bit mask value of the active CaptureMask
+//   - list of "component display names" ("all" is allowed)
+// - Setting active CaptureMask is available for both:
+//   - already registered components
+//   - "pending" (yet to be registered) components


This reads like it should be part of the general documentation for our telemetry integration, as it's not specific at all to the CcpCaptureMaskInfo structure.

Agreed. We should reserve comment blocks like this for classes and symbols we ourselves define, rather than explaining external concepts like what a capture mask is. Top-down explanations for how our systems operate should be included in general documentation as well.

CCP-Aporia · 2026-07-20T12:35:01Z

+
+CARBON_CORE_API uint64_t CcpRegisterCaptureMask( const std::string& name );
+CARBON_CORE_API uint64_t CcpRegisterCaptureMask( const std::string& name, CcpColor color );
+CARBON_CORE_API std::vector<CcpCaptureMaskInfo> CcpGetRegisteredCaptureMasks();


Why not return a const reference to the built-in capture mask array?

Because the array contains reserved entries as well as all the registered capture masks.

CCP-Aporia · 2026-07-21T16:20:36Z

 public:
 	TelemetryZone() = delete;
-	CARBON_CORE_API TelemetryZone( uint32_t ctx, const char* name, const char* filename, uint32_t lineno, CcpColor color = CcpColor::SteelBlue );
+	CARBON_CORE_API TelemetryZone( CcpCaptureMaskHandle handle, const char* name, const char* filename, uint32_t lineno, CcpColor obsolete = CcpColor::SteelBlue );


Because CcpCaptureMaskHandle is a uint32_t then this should be both ABI and API compatible. Need to verify. In worst case, this stays the non-specialised type for the time being.

There is only one capture mask from the user's perspective, so let's keep implementation details out of the function name.

There were two issues at play here: 1. The same iterator into the `FiberNameStore` may have been queued for erasure multiple times 2. Inserting a name into the `FiberNameStore` that has a pending erasure would Surprisingly, those issues primarily surfaced on macOS, and only rarely on Windows. The solution to the problem is to maintain a map instead of a queue. This way, inserting into `FiberNameStore` can cancel any pending erasure. Additionally, the map ensures that there is only ever one entry for `FiberNameStore` iterator that should be deleted.

ccp-serpent

There are clear improvements which can be made to the documentation, and the code could do with some refactoring. At a functional level, however, I don't see any problems with the code as it's written.

What I'd like to draw particular attention to is how the API makes mentions of the CaptureMask concept. As the code is written now, a capture mask can be either a struct, a list of strings, a list of structs, when it's in fact a uint64_t type.

Take the following functions for example:

std::vector<CcpCaptureMaskInfo> CcpGetRegisteredCaptureMasks()
std::vector<std::string> CcpGetActiveCaptureMask()

There's only ever one capture mask active, yet the API reads like we may have multiple capture masks registered and active within our telemetry integration.

Neither of these functions return a capture mask. They return a list of names, which correspond to a profiling category which may be represented by a capture mask. In that sense, they´re not behaving as their name suggests.

This goes even further, see below:
CcpCaptureMaskHandle CcpRegisterCaptureMask( const std::string& name, CcpColor color = CcpColor::Fuchsia )

This function is not registering a capture mask. It's registering a profiler category which the code is later able to associate with a bit and incorporate into a capture mask.

These examples read like there's something missing at a conceptual level from the code. Or perhaps the functions just need to be renamed. I don't want to state which is which, but I believe this could stand improving.

ccp-serpent · 2026-07-24T13:22:12Z

+// - An active CaptureMask defaults to "all" but can be set/narrowed before or
+//   during a Telemetry session is started using either:
+//   - a numerical bit mask value of the active CaptureMask
+//   - list of "component display names" ("all" is allowed)
+// - Setting active CaptureMask is available for both:
+//   - already registered components
+//   - "pending" (yet to be registered) components


Agreed. We should reserve comment blocks like this for classes and symbols we ourselves define, rather than explaining external concepts like what a capture mask is. Top-down explanations for how our systems operate should be included in general documentation as well.

ccp-serpent · 2026-07-24T13:23:00Z

+// ---------------------------------------------------------------------------
+// CaptureMasks:
+// - CaptureMask(s) are used to determine if a given Zone should be emitted to
+//   Telemetry tracking or not based on its origin, i.e. carbon component.


There is no hard association between capture masks and carbon components. A component may register multiple capture masks, for example, or even none. This reads a bit confused.

ccp-serpent · 2026-07-24T13:25:14Z

+//   - a numerical bit mask value of the active CaptureMask
+//   - list of "component display names" ("all" is allowed)
+// - Setting active CaptureMask is available for both:
+//   - already registered components


This is just a struct. What is this concept of components already registered or pending registration? This documentation may be relevant to whatever function performs the registration, or the container for pending components. But I don't think these are useful concepts to explain in the context of a CcpCaptureMaskInfo struct.

ccp-serpent · 2026-07-24T13:29:32Z

+// ---------------------------------------------------------------------------
+struct CcpCaptureMaskInfo
+{
+	std::string name;    // The lower-case display name of the CaptureMask (carbon-component)


Carbon components are not conceptually related to capture masks. I'm also not sure it's useful to document that the display name is in lower-case, unless the system requires display names to be lowercase, or the display name is converted to lowercase upon registration. Either case, that should probably be documented alongside the registration function for capture masks.

ccp-serpent · 2026-07-24T13:33:39Z

+struct CcpCaptureMaskInfo
+{
+	std::string name;    // The lower-case display name of the CaptureMask (carbon-component)
+	CcpColor color{CcpColor::White}; // The chosen/allocated color for the CaptureMask


The words "chosen" and "allocated" are not necessarily interchangeable. Does some external system allocate a display color to the mask info struct? I suggest documenting this with a simpler // Display color, further context can be added to code points utilizing the struct, if necessary.

ccp-serpent · 2026-07-24T15:12:28Z

+
+bool CcpSetCaptureMask( const std::vector<std::string>& maskNames )
+{
+	// Guard access to all CaptureMasks members


See https://github.com/carbonengine/core/pull/34/changes#r3646346444.

ccp-serpent · 2026-07-24T15:18:47Z

+	uint64_t newActiveCaptureMask = 0;
+	for( const auto& rawName : maskNames )
+	{
+		if( rawName.empty() )


I think the code would read better if the contents of this for loop was extracted into a separate function, i.e. bool CcpAddCaptureMask(const std::string& name)

ccp-serpent · 2026-07-24T15:31:27Z

+
+CARBON_CORE_API uint64_t CcpRegisterCaptureMask( const std::string& name );
+CARBON_CORE_API uint64_t CcpRegisterCaptureMask( const std::string& name, CcpColor color );
+CARBON_CORE_API std::vector<CcpCaptureMaskInfo> CcpGetRegisteredCaptureMasks();


Because the array contains reserved entries as well as all the registered capture masks.

Those two tests covered functionality that has been removed / collapsed into the same `TelemetryZone` constructor since then.

As per code-review feedback, the "mask" implementation details does no longer hold up across the API surface.

Reduces compile / link time when building without telemetry, and removes the need for the `#ifdef` in the test code. Take note that currently building `WITH_TELEMETRY=OFF` does not work in general for core, unrelated to this work.

Without it, there's a risk of accessing bad memory when determining the color, and odd behaviour when checking the bit mask.

As per code review feedback. It's a better way to track existence without relying on a sentinel value that could theoretically be valid.

As opposed to the generic `std::string`.

ccp-chargeback added 12 commits July 9, 2026 16:32

Add RegisterCaptureMask() functionality

cf74c37

Change contains: - Ability to call RegisterCaptureMask() for a given name and an optional Telemetry Zone display color - Automatically assign a color if one is not provided Part of: https://fenriscreations.atlassian.net/browse/PLAT-11474

Merge remote-tracking branch 'carbonengine/main' into capture_mask_su…

315486c

…pport

Fix errors after Color=>CcpColor name change

d45f9ee

Changed: - enum class Color => CcpColor - namespace CcpColor => ColorUtil

Remove static from ColorUtil inline helper functions

f44da09

Store registered CaptureMasks in a fixed size array

3a7688b

Will give us the option of a O(1) lookup for display color once we change telemetry to mark zones based on CaptureMask bit.

Add support for color to the TracyTestClient.

867b6d7

Make sure we can verify the color associated with a Zone from the TracyTestClient in tests.

Explicitly add includes for std types used in CcpTelemetry.h

31f7c93

Done because of build problems on v143/v145 vs v141 identified by ccptoebeans.

Add support for CaptureMasks in CcpTelemetryEnterZone/TelemetryZone

d9f7aea

Add CaptureMaksBit overloads for: - TelemetryZone constructor - CcpTelemetryEnterZone() Make sure tests reflect the new reality. Part of: https://fenriscreations.atlassian.net/browse/PLAT-11476

Use EXPECT_TRUE instead of ASSERT_TRUE in CcpTelemetry tests

fd800aa

Merge remote-tracking branch 'carbonengine/main' into capture_mask_su…

4a6ace5

…pport # Conflicts: # include/CcpTelemetry.h

ccp-chargeback requested review from CCP-Aporia and ccp-serpent and removed request for CCP-Aporia and ccp-serpent July 17, 2026 15:31

ccp-chargeback added 5 commits July 17, 2026 17:15

Simplify auto color selection for CaptureMasks

bbec690

Use deBruijn to find the index of a single bit number

8283b2e

Remove un-necessary std::atomic use on CaptureMasks

78306a4

Fix wrong function name in non-Telemetry code path

e5ed1dc

Rename: CcpGetCaptureMasks() To: CcpGetRegisteredCaptureMasks()

Remove double bookkeeping on registeredCaptureMasks vs their bits

4a58c70

ccp-chargeback requested review from CCP-Aporia and ccp-serpent July 18, 2026 13:50

Fix typo in test helper IsSCaptureMaskSingleBit()

942e009

CCP-Aporia added 5 commits July 20, 2026 16:16

Remove unused headers from CcpColorConstants.h

7142ed6

Use currently advised copyright statement

82609e4

Legal hasn't advised on an updated statement yet, so we stick with `CCP ehf.`.

Update CcpColorString to return a string_view of all possible nam…

59ac96c

…es for a given color value To avoid unnecessary memory copies of the underlying string data, and to avoid surprises in seeing `CcpColorToString( CcpColor::Cyan )` return `Aqua`.

Use constexpr size_t instead of #define

9eee0e4

For type-safe constants instead of preprocessor text replacement.

CCP-Aporia added 10 commits July 21, 2026 13:01

Remove slow test that covers an unimportant implementation details

253ae9f

The order of registration doesn't matter to any users of `CcpRegisterCaptureMask`, primarily because that order depends on the order of calls to that function, not the implementation itself.

Remove uint64_t overload for CcpSetActiveCaptureMask

ae7e72b

This is for an upcoming QoL improvement for capture mask users, where they should no longer have to rely on the implementation of using a bitmask for the capture mask.

Return list of active capture mask names instead of bitmask

8a812a9

Since the bitmask is only an implementation details, and because `CcpSetActiveCaptureMask` takes in the names, too.

Use ASSERT_* instead of EXPECT_* in a few places

a9ab8f3

This avoids double-checking whether we can access memory safely further down the test. If we only use EXPECT_* macros some tests may crash if the observed container is empty.

Add missing export annotation to CcpColorToString

1528331

Add missing uint32_t -> CcpCaptureMaskHandle type change

601d77f

Remove unnecessary comment

a57664a

Remove no longer existing function overload from non-telemetry path

5d3dab5

CCP-Aporia reviewed Jul 21, 2026

View reviewed changes

CCP-Aporia added 2 commits July 21, 2026 16:34

Rename CcpSetActiveCaptureMask to CcpSetCaptureMask

f6a294a

There is only one capture mask from the user's perspective, so let's keep implementation details out of the function name.

ccp-serpent requested changes Jul 24, 2026

View reviewed changes

CCP-Aporia added 7 commits July 27, 2026 12:20

Remove obsolete tests

1bb58df

Those two tests covered functionality that has been removed / collapsed into the same `TelemetryZone` constructor since then.

Reword the "capture mask" concept as "profiler zone"

6eb92bf

As per code-review feedback, the "mask" implementation details does no longer hold up across the API surface.

Skip telemetry tests if WITH_TELEMETRY=OFF

5632876

Reduces compile / link time when building without telemetry, and removes the need for the `#ifdef` in the test code. Take note that currently building `WITH_TELEMETRY=OFF` does not work in general for core, unrelated to this work.

Rename CcpGetActiveProfilerZone to CcpGetActiveProfilerZones

e9819c3

Add missing bounds check

5cd97fb

Without it, there's a risk of accessing bad memory when determining the color, and odd behaviour when checking the bit mask.

Use std::optional for tracking registered profiler zones

2de388f

As per code review feedback. It's a better way to track existence without relying on a sentinel value that could theoretically be valid.

Return CcpProfilerZone where applicable

188e049

As opposed to the generic `std::string`.

Uh oh!

Conversation

ccp-chargeback commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccpgames-carbon commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccp-serpent left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ccp-chargeback commented Jul 17, 2026 •

edited

Loading

ccpgames-carbon commented Jul 20, 2026 •

edited

Loading

ccp-serpent left a comment •

edited

Loading