Skip to content

Add CaptureMask support#34

Draft
ccp-chargeback wants to merge 34 commits into
carbonengine:mainfrom
ccp-chargeback:capture_mask_support
Draft

Add CaptureMask support#34
ccp-chargeback wants to merge 34 commits into
carbonengine:mainfrom
ccp-chargeback:capture_mask_support

Conversation

@ccp-chargeback

@ccp-chargeback ccp-chargeback commented Jul 17, 2026

Copy link
Copy Markdown
Member

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
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
@ccp-chargeback
ccp-chargeback requested review from CCP-Aporia and ccp-serpent and removed request for CCP-Aporia and ccp-serpent July 17, 2026 15:31
@ccpgames-carbon

ccpgames-carbon commented Jul 20, 2026

Copy link
Copy Markdown

CLA assistant check
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.
Comment thread CcpTelemetry.cpp Outdated
Comment thread CcpTelemetry.cpp Outdated
Comment thread CcpTelemetry.cpp Outdated
Comment thread CcpTelemetry.cpp Outdated
Comment thread include/CcpTelemetry.h
Comment on lines +50 to +56
// - 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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread include/CcpTelemetry.h

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();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread include/CcpTelemetry.h Outdated
[[deprecated( "Use `CcpTelemetryEnterZone( void* key, uint64_t captureMaskBit, ... )` instead" )]]
CARBON_CORE_API void CcpTelemetryEnterZone( void* key, const char* name, const char* filename, uint32_t lineno );

CARBON_CORE_API void CcpTelemetryEnterZone( void* key, uint64_t captureMaskBit, const char* name, const char* filename, uint32_t lineno );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EnterZone / LeaveZone APIs are no longer good since we adopted Tracy, because they introduce a breaking point: An EnterZone is required to have a matching LeaveZone, otherwise the instrumentation breaks, but nobody is going to notice that problem until they run with the visualiser (or similar tool) attached to the process. Therefore, it is preferable to always use the TelemetryZone RAII wrapper instead.
Long story short, please don't add an overload for EnterZone ... but we might want to annotate all 3 of these methods as deprecated instead, pointing users of CcpTelemetry.h at TelemetryZone instead.

Comment thread CcpTelemetry.cpp
Comment thread include/CcpTelemetry.h
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 );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants