Feature flags live where they are read

docs/STRUCTURE.md

@@ -189,7 +189,7 @@ nearly everywhere. Nothing in it may include `rxe/`.
 | `files` | `core/file_system`, `core/paths` |
 | `tasks` | `core/job_system` |
 | `memory` | `core/memory/*`, `core/shared.h` |
-| `features` | `core/feature_registry`. `features.def` goes away: each module declares its own flags in its `features.h`, Chromium's `BASE_DECLARE_FEATURE` shape, so adding a flag no longer edits a shared file. |
+| `features` | `core/feature_registry`. `features.def` is gone (5b): a module declares each flag as a `base::Feature` in the file that reads it, so adding a flag edits no shared file. `InitFeatures` applies `RX_FEATURES` over the one process-wide chain. |
 | `time` | `core/frame_timer` |
 | `system` | `core/platform`, `core/app_identity` |
 | `math` | `core/math.h`, `core/scalar.h` |
@@ -371,7 +371,7 @@ What has to exist once per process, and how the shared build keeps it so:
 | `render/util` | grab-bag | done (4c): `gpu/rhi`, `asset/exr_write`, `ui/imgui` |
 | `runtime/engine_context.h` | god object passed to every demo | phase 6, with the rest of `runtime/` |
 | `fly_camera` | duplicated in `runtime/` and `apps/editor/` | done (2b): `scene::FlyCamera`, fed a resolved `FlyCameraInput` |
-| `core/features.def` | one file every feature edits | phase 5: per-module flags self-register, which needs the one-per-process registry |
+| `core/features.def` | one file every feature edits | done (5b): flags live where they are read |
 | `CMakeLists.txt` | 1381 lines, registers every test centrally | done (4b): each module registers its own |
 | `anim/locomotion` vs `engine/locomotion` | two modules called locomotion | done (2b): `anim::ProceduralGait` |
 
@@ -424,7 +424,8 @@ and after.
    - 5a (done): the whole tree links as shared objects (missing exports
      added), one option registry per process, per-DSO volk filled through
      `GetVulkanHandles`, and the `linux-dev` preset.
-   - 5b: per-module feature flags on the single registry.
+   - 5b (done): per-module feature flags on the single registry;
+     `features.def` and its three never-read render flags are gone.
 6. **Plugins and apps.** Add `RX_PLUGIN`, `PluginRegistry` and `rx_add_app`.
    `runtime/` becomes `apps/shell/`, the tools move into `apps/`, and
    `EngineContext` goes away.

foundation/features/feature_registry.cc

@@ -2,48 +2,29 @@
 
 #include <stdlib.h>
 
+#include "base/strings/string_ref.h"
 #include "foundation/logging/log.h"
 
 namespace rx {
 namespace {
 
-// The one and only table. Each entry is direct-initialized in place, so
-// base::Feature's deleted copy/move is never needed. Non-const because the env
-// override flips `enabled`. This object is referenced by InitFeatures() which
-// runs from engine startup, so the linker keeps it: no static-init stripping.
-base::Feature g_features[] = {
-#define RX_FEATURE(id, name, enabled) {name, enabled},
-#include "base/strings/string_ref.h"
-#include "foundation/features/features.def"
-#undef RX_FEATURE
-};
-
-constexpr unsigned kCount = static_cast<unsigned>(FeatureId::kCount);
-static_assert(sizeof(g_features) / sizeof(g_features[0]) == kCount,
-              "features.def and FeatureId disagree on the flag count");
-
-// Flip the flag whose name matches `name`. Returns false if there is no such
-// flag.
+// Flip the flag whose name matches `name`. Returns false if no flag in this
+// binary carries it.
 bool ApplyOne(base::StringRef name, bool enabled) {
-  for (auto& f : g_features) {
-    if (name == f.name) {
-      f.enabled = enabled;
-      return true;
+  bool found = false;
+  base::Feature::VisitAll([&](const base::Feature* feature) {
+    if (name == feature->name) {
+      // The chain hands out const pointers; flags are mutable globals, which is
+      // the one thing this function exists to write.
+      const_cast<base::Feature*>(feature)->enabled = enabled;
+      found = true;
     }
-  }
-  return false;
+  });
+  return found;
 }
 
 }  // namespace
 
-bool FeatureEnabled(FeatureId id) {
-  return g_features[static_cast<unsigned>(id)].enabled;
-}
-
-base::Span<base::Feature> Features() {
-  return base::Span<base::Feature>(g_features);
-}
-
 void InitFeatures() {
   const char* spec = ::getenv("RX_FEATURES");
   if (!spec || !*spec) return;

foundation/features/feature_registry.h

@@ -1,8 +1,6 @@
 #ifndef RX_FOUNDATION_FEATURES_FEATURE_REGISTRY_H_
 #define RX_FOUNDATION_FEATURES_FEATURE_REGISTRY_H_
 
-#include <base/containers/span.h>
-
 #include "foundation/build_config/export.h"
 // base/feature.h's constructor parameters shadow its members, so the header is
 // not -Wshadow clean. Silence just this include rather than spraying the
@@ -21,42 +19,24 @@
 #pragma warning(pop)
 #endif
 
-// A linker-proof feature-flag registry.
+// Feature flags. A module declares each flag at namespace scope in the file that
+// reads it, and reads it as a bool:
 //
-// base::Feature self-registers through InitChain, but that registration is a
-// static initializer: when a flag lives in a static library and nothing else
-// references its object file, the linker drops the object (and the
-// registration with it). This registry sidesteps that entirely. Every flag is
-// declared in core/features.def and lives in one table in one translation
-// unit; InitFeatures() is called from engine startup, so that object is always
-// pulled into the link and the table is always complete. We never walk the
-// InitChain.
-
-namespace rx {
-
-// Stable handle for every flag in core/features.def.
-enum class FeatureId : unsigned {
-#define RX_FEATURE(id, name, enabled) k##id,
-#include "foundation/features/features.def"
-#undef RX_FEATURE
-  kCount
-};
-
-// True when the flag is on (default from the manifest, after env overrides).
-// A single array read, no list walk.
+//   base::Feature ClothFeature{"physics.cloth", /*enabled=*/true};
+//   ...
+//   if (!ClothFeature) return {};
 //
-// The g_features table backing these is an anonymous-namespace global in
-// feature_registry.cc, reached only through these three functions, so under
-// RX_SHARED it stays a single instance inside the core DSO (verified: no other
-// translation unit can name the array).
-RX_FOUNDATION_EXPORT bool FeatureEnabled(FeatureId id);
+// base::Feature registers itself on one process-wide chain (equilibrium makes
+// the chain root unique across shared objects), so there is no central list to
+// edit. A flag lives exactly as long as the code that reads it: if the linker
+// drops an unreferenced object, it drops the flag with the only reader.
 
-// The whole table, for listing in the debug UI. size() == kCount.
-RX_FOUNDATION_EXPORT base::Span<base::Feature> Features();
+namespace rx {
 
-// Apply RX_FEATURES overrides once, early in engine startup. Tokens are comma
-// or whitespace separated: "name" / "+name" enable, "-name" / "name=0" disable.
-// Unknown names are warned about and skipped.
+// Apply RX_FEATURES overrides once, early in startup and before any flag is
+// read. Tokens are comma or whitespace separated: "name" / "+name" enable,
+// "-name" / "name=0" disable. A name no flag in this binary carries is warned
+// about and skipped.
 RX_FOUNDATION_EXPORT void InitFeatures();
 
 }  // namespace rx

rxe/physics/physics_world.cc

@@ -70,6 +70,9 @@ static_assert(JPH_VERSION_MAJOR > 5 || (JPH_VERSION_MAJOR == 5 && JPH_VERSION_MI
 namespace rx::physics {
 namespace {
 
+// Cloth simulation as a whole: off, AddCloth refuses and nothing steps.
+base::Feature ClothFeature{"physics.cloth", /*enabled=*/true};
+
 namespace layers {
 constexpr JPH::ObjectLayer kStatic = 0;
 constexpr JPH::ObjectLayer kDynamic = 1;
@@ -2568,7 +2571,7 @@ void PhysicsWorld::RemoveStrandGroom(StrandGroomId id) {
 }
 
 ClothId PhysicsWorld::CreateCloth(const ClothDesc& desc, const Mat4& transform) {
-  if (!impl_ || !FeatureEnabled(FeatureId::kCloth) || !desc.positions || !desc.indices ||
+  if (!impl_ || !ClothFeature || !desc.positions || !desc.indices ||
       desc.vertex_count < 3 || desc.index_count < 3 || (desc.pin_count > 0 && !desc.pins) ||
       !IsFinite(transform)) {
     return 0;