Skip to content

Commit

Permalink
a new Feature Flag API
Browse files Browse the repository at this point in the history
Feature flags are intended to be used when a new feature is added to 
filament and have generally two purposes:

1) during feature development, the feature can be implemented but
   disabled which can help developing large features. This way the
   feature can be tested by stakeholders while it's being developed
   without impacting other clients.

2) once a feature is ready, its flag can be enabled by default, but 
   in case the feature breaks something or has unintended consequence,
   clients have the option to turn it off.

Feature flags are intended to have a relatively short life span, i.e.
once a feature is stable, the flag fill be removed.

There two types of feature flags. Constant feature flags can only be
set during Engine initialization via Engine::Builder. Non-constant
feature flags can be set at any time. 

Feature flags SHOULD NOT be used as configuration or settings.


Feature flags are designed with a few ideas in mind:
- they are very cheap to check inside the engine
- non-constant flags can easily be toggled using ImGUI
  • Loading branch information
pixelflinger committed Nov 5, 2024
1 parent b64df46 commit b8551e5
Show file tree
Hide file tree
Showing 6 changed files with 309 additions and 13 deletions.
41 changes: 41 additions & 0 deletions android/filament-android/src/main/cpp/Engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,37 @@ Java_com_google_android_filament_Engine_nGetActiveFeatureLevel(JNIEnv *, jclass,
return (jint)engine->getActiveFeatureLevel();
}

extern "C"
JNIEXPORT jboolean JNICALL
Java_com_google_android_filament_Engine_nHasFeatureFlag(JNIEnv *env, jclass clazz,
jlong nativeEngine, jstring name_) {
Engine* engine = (Engine*) nativeEngine;
const char *name = env->GetStringUTFChars(name_, 0);
std::optional<bool> result = engine->getFeatureFlag(name);
env->ReleaseStringUTFChars(name_, name);
return result.has_value();
}
extern "C"
JNIEXPORT jboolean JNICALL
Java_com_google_android_filament_Engine_nSetFeatureFlag(JNIEnv *env, jclass clazz,
jlong nativeEngine, jstring name_, jboolean value) {
Engine* engine = (Engine*) nativeEngine;
const char *name = env->GetStringUTFChars(name_, 0);
jboolean result = engine->setFeatureFlag(name, (bool)value);
env->ReleaseStringUTFChars(name_, name);
return result;
}
extern "C"
JNIEXPORT jboolean JNICALL
Java_com_google_android_filament_Engine_nGetFeatureFlag(JNIEnv *env, jclass clazz,
jlong nativeEngine, jstring name_) {
Engine* engine = (Engine*) nativeEngine;
const char *name = env->GetStringUTFChars(name_, 0);
std::optional<bool> result = engine->getFeatureFlag(name);
env->ReleaseStringUTFChars(name_, name);
return result.value_or(false); // we should never fail here
}

extern "C" JNIEXPORT jlong JNICALL Java_com_google_android_filament_Engine_nCreateBuilder(JNIEnv*,
jclass) {
Engine::Builder* builder = new Engine::Builder{};
Expand Down Expand Up @@ -565,6 +596,16 @@ extern "C" JNIEXPORT void JNICALL Java_com_google_android_filament_Engine_nSetBu
builder->paused((bool) paused);
}

extern "C"
JNIEXPORT void JNICALL
Java_com_google_android_filament_Engine_nSetBuilderFeature(JNIEnv *env, jclass clazz,
jlong nativeBuilder, jstring name_, jboolean value) {
Engine::Builder* builder = (Engine::Builder*) nativeBuilder;
const char *name = env->GetStringUTFChars(name_, 0);
builder->feature(name, (bool)value);
env->ReleaseStringUTFChars(name_, name);
}

extern "C" JNIEXPORT jlong JNICALL
Java_com_google_android_filament_Engine_nBuilderBuild(JNIEnv*, jclass, jlong nativeBuilder) {
Engine::Builder* builder = (Engine::Builder*) nativeBuilder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,17 @@ public Builder paused(boolean paused) {
return this;
}

/**
* Set a feature flag value. This is the only way to set constant feature flags.
* @param name feature name
* @param value true to enable, false to disable
* @return A reference to this Builder for chaining calls.
*/
public Builder feature(@NonNull String name, boolean value) {
nSetBuilderFeature(mNativeBuilder, name, value);
return this;
}

/**
* Creates an instance of Engine
*
Expand Down Expand Up @@ -393,6 +404,7 @@ public static class Config {
/**
* Set to `true` to forcibly disable parallel shader compilation in the backend.
* Currently only honored by the GL backend.
* @Deprecated use "backend.disable_parallel_shader_compile" feature flag instead
*/
public boolean disableParallelShaderCompile = false;

Expand Down Expand Up @@ -433,6 +445,7 @@ public static class Config {

/**
* Disable backend handles use-after-free checks.
* @Deprecated use "backend.disable_handle_use_after_free_check" feature flag instead
*/
public boolean disableHandleUseAfterFreeCheck = false;

Expand Down Expand Up @@ -469,6 +482,7 @@ public enum ShaderLanguage {
* Assert the native window associated to a SwapChain is valid when calling makeCurrent().
* This is only supported for:
* - PlatformEGLAndroid
* @Deprecated use "backend.opengl.assert_native_window_is_valid" feature flag instead
*/
public boolean assertNativeWindowIsValid = false;
}
Expand Down Expand Up @@ -693,11 +707,11 @@ public Config getConfig() {

/**
* Returns the maximum number of stereoscopic eyes supported by Filament. The actual number of
* eyes rendered is set at Engine creation time with the {@link
* Engine#Config#stereoscopicEyeCount} setting.
* eyes rendered is set at Engine creation time with the {@link Config#stereoscopicEyeCount}
* setting.
*
* @return the max number of stereoscopic eyes supported
* @see Engine#Config#stereoscopicEyeCount
* @see Config#stereoscopicEyeCount
*/
public long getMaxStereoscopicEyes() {
return nGetMaxStereoscopicEyes(getNativeObject());
Expand Down Expand Up @@ -894,7 +908,8 @@ public boolean isValidMaterial(@NonNull Material object) {

/**
* Returns whether the object is valid.
* @param object Object to check for validity
* @param ma Material
* @param mi MaterialInstance to check for validity
* @return returns true if the specified object is valid.
*/
public boolean isValidMaterialInstance(@NonNull Material ma, MaterialInstance mi) {
Expand Down Expand Up @@ -1316,6 +1331,39 @@ public void unprotected() {
*/
public static native long getSteadyClockTimeNano();


/**
* Checks if a feature flag exists
* @param name name of the feature flag to check
* @return true if it exists false otherwise
*/
public boolean hasFeatureFlag(@NonNull String name) {
return nHasFeatureFlag(mNativeObject, name);
}

/**
* Set the value of a non-constant feature flag.
* @param name name of the feature flag to set
* @param value value to set
* @return true if the value was set, false if the feature flag is constant or doesn't exist.
*/
public boolean setFeatureFlag(@NonNull String name, boolean value) {
return nSetFeatureFlag(mNativeObject, name, value);
}

/**
* Retrieves the value of any feature flag.
* @param name name of the feature flag
* @return the value of the flag if it exists
* @exception IllegalArgumentException is thrown if the feature flag doesn't exist
*/
public boolean getFeatureFlag(@NonNull String name) {
if (!hasFeatureFlag(name)) {
throw new IllegalArgumentException("The feature flag \"" + name + "\" doesn't exist");
}
return nGetFeatureFlag(mNativeObject, name);
}

@UsedByReflection("TextureHelper.java")
public long getNativeObject() {
if (mNativeObject == 0) {
Expand Down Expand Up @@ -1405,6 +1453,9 @@ private static void assertDestroy(boolean success) {
private static native int nGetSupportedFeatureLevel(long nativeEngine);
private static native int nSetActiveFeatureLevel(long nativeEngine, int ordinal);
private static native int nGetActiveFeatureLevel(long nativeEngine);
private static native boolean nHasFeatureFlag(long nativeEngine, String name);
private static native boolean nSetFeatureFlag(long nativeEngine, String name, boolean value);
private static native boolean nGetFeatureFlag(long nativeEngine, String name);

private static native long nCreateBuilder();
private static native void nDestroyBuilder(long nativeBuilder);
Expand All @@ -1420,5 +1471,6 @@ private static native void nSetBuilderConfig(long nativeBuilder, long commandBuf
private static native void nSetBuilderFeatureLevel(long nativeBuilder, int ordinal);
private static native void nSetBuilderSharedContext(long nativeBuilder, long sharedContext);
private static native void nSetBuilderPaused(long nativeBuilder, boolean paused);
private static native void nSetBuilderFeature(long nativeBuilder, String name, boolean value);
private static native long nBuilderBuild(long nativeBuilder);
}
74 changes: 74 additions & 0 deletions filament/include/filament/Engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@

#include <utils/compiler.h>
#include <utils/Invocable.h>
#include <utils/Slice.h>

#include <initializer_list>
#include <optional>

#include <stdint.h>
#include <stddef.h>
Expand Down Expand Up @@ -309,6 +313,7 @@ class UTILS_PUBLIC Engine {
/**
* Set to `true` to forcibly disable parallel shader compilation in the backend.
* Currently only honored by the GL and Metal backends.
* @deprecated use "backend.disable_parallel_shader_compile" feature flag instead
*/
bool disableParallelShaderCompile = false;

Expand Down Expand Up @@ -349,6 +354,7 @@ class UTILS_PUBLIC Engine {

/*
* Disable backend handles use-after-free checks.
* @deprecated use "backend.disable_handle_use_after_free_check" feature flag instead
*/
bool disableHandleUseAfterFreeCheck = false;

Expand Down Expand Up @@ -385,10 +391,32 @@ class UTILS_PUBLIC Engine {
* Assert the native window associated to a SwapChain is valid when calling makeCurrent().
* This is only supported for:
* - PlatformEGLAndroid
* @deprecated use "backend.opengl.assert_native_window_is_valid" feature flag instead
*/
bool assertNativeWindowIsValid = false;
};


/**
* Feature flags can be enabled or disabled when the Engine is built. Some Feature flags can
* also be toggled at any time. Feature flags should alawys use their default value unless
* the feature enabled by the flag is faulty. Feature flags provide a last resort way to
* disable problematic features.
* Feature flags are intended to have a short life-time and are regularly removed as features
* mature.
*/
struct FeatureFlag {
char const* UTILS_NONNULL name; //!< name of the feature flag
char const* UTILS_NONNULL description; //!< short description
bool const* UTILS_NONNULL value; //!< pointer to the value of the flag
bool constant; //!< whether the flag is constant after construction
};

/**
* Returns the list of available feature flags
*/
utils::Slice<const FeatureFlag> getFeatureFlags() const noexcept;

#if UTILS_HAS_THREADING
using CreateCallback = void(void* UTILS_NULLABLE user, void* UTILS_NONNULL token);
#endif
Expand Down Expand Up @@ -461,6 +489,21 @@ class UTILS_PUBLIC Engine {
*/
Builder& paused(bool paused) noexcept;

/**
* Set a feature flag value. This is the only way to set constant feature flags.
* @param name feature name
* @param value true to enable, false to disable
* @return A reference to this Builder for chaining calls.
*/
Builder& feature(char const* UTILS_NONNULL name, bool value) noexcept;

/**
* Enables a list of features.
* @param list list of feature names to enable.
* @return A reference to this Builder for chaining calls.
*/
Builder& features(std::initializer_list<char const *> list) noexcept;

#if UTILS_HAS_THREADING
/**
* Creates the filament Engine asynchronously.
Expand Down Expand Up @@ -1078,6 +1121,37 @@ class UTILS_PUBLIC Engine {

DebugRegistry& getDebugRegistry() noexcept;

/**
* Check if a feature flag exists
* @param name name of the feature flag to check
* @return true if the feature flag exists, false otherwise
*/
inline bool hasFeatureFlag(char const* UTILS_NONNULL name) noexcept {
return getFeatureFlag(name).has_value();
}

/**
* Set the value of a non-constant feature flag.
* @param name name of the feature flag to set
* @param value value to set
* @return true if the value was set, false if the feature flag is constant or doesn't exist.
*/
bool setFeatureFlag(char const* UTILS_NONNULL name, bool value) noexcept;

/**
* Retrieves the value of any feature flag.
* @param name name of the feature flag
* @return the value of the flag if it exists
*/
std::optional<bool> getFeatureFlag(char const* UTILS_NONNULL name) const noexcept;

/**
* Returns a pointer to a non-constant feature flag value.
* @param name name of the feature flag
* @return a pointer to the feature flag value, or nullptr if the feature flag is constant or doesn't exist
*/
bool* UTILS_NULLABLE getFeatureFlagPtr(char const* UTILS_NONNULL name) const noexcept;

protected:
//! \privatesection
Engine() noexcept = default;
Expand Down
17 changes: 17 additions & 0 deletions filament/src/Engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

#include <utils/compiler.h>
#include <utils/Panic.h>
#include <utils/Slice.h>

#include <chrono>

Expand Down Expand Up @@ -442,6 +443,22 @@ uint64_t Engine::getSteadyClockTimeNano() noexcept {
return std::chrono::steady_clock::now().time_since_epoch().count();
}

utils::Slice<const Engine::FeatureFlag> Engine::getFeatureFlags() const noexcept {
return downcast(this)->getFeatureFlags();
}

bool Engine::setFeatureFlag(char const* name, bool value) noexcept {
return downcast(this)->setFeatureFlag(name, value);
}

std::optional<bool> Engine::getFeatureFlag(char const* name) const noexcept {
return downcast(this)->getFeatureFlag(name);
}

bool* Engine::getFeatureFlagPtr(char const* UTILS_NONNULL name) const noexcept {
return downcast(this)->getFeatureFlagPtr(name);
}

#if defined(__EMSCRIPTEN__)
void Engine::resetBackendState() noexcept {
downcast(this)->resetBackendState();
Expand Down
Loading

0 comments on commit b8551e5

Please sign in to comment.