From 8eb8b3dc0fed6d394b5ff418d316052664fbde77 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 16:27:09 -1000 Subject: [PATCH] [light] Promote bit-pattern clamp to LightColorValues setters and add layout asserts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move `float_out_of_unit_range()` / add `clamp_unit_float()` to light_color_values.h so the nine `set_*(float)` setters can use the unsigned bit-pattern clamp instead of `std::clamp(x, 0.0f, 1.0f)`. `std::clamp` expands to two soft-float `__ltsf2`/`__gtsf2` calls per invocation on ESP8266 — replacing it with a single unsigned compare saves code across every caller of these setters (StrobeLightEffect, the 11-arg LightColorValues constructor, external components). Split the cold-path helper: `log_value_out_of_range_()` now only logs, and each caller applies the clamp strategy appropriate to its range (`clamp_unit_float` for the 8-field loop, `std::clamp` for color temperature's runtime-variable range). Add layout/format assertions: - std::is_standard_layout_v on LightCall and LightColorValues so the offsetof arithmetic in the clamp loop is well-defined. - sizeof(float) == 4 and is_iec559 so the bit-pattern trick is valid. A direct __builtin_bit_cast check would be cleaner but is not available on the ESP8266 xtensa toolchain. Text-section delta vs. prior commit (isolated light build): ESP32-IDF: .flash.text 137760 -> 136988 (-772 B) ESP8266: .irom0.text 268440 -> 267912 (-528 B) --- esphome/components/light/light_call.cpp | 59 +++++++++--------- esphome/components/light/light_color_values.h | 62 ++++++++++++++++--- 2 files changed, 83 insertions(+), 38 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 045a4f2935..bc0b12aa77 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -1,4 +1,5 @@ #include +#include #include "light_call.h" #include "light_state.h" @@ -10,17 +11,19 @@ namespace esphome::light { static const char *const TAG = "light"; -// Cold-path helper: called only when the caller has already determined the -// value is out of range. Keeping the range check at the caller avoids the -// call-site spill/reload and prologue on the hot path (in-range). The -// `param_name_progmem` argument points into the FIELD_NAMES table in flash; -// `progmem_read_ptr` is a plain `*addr` inline on non-ESP8266 platforms. -static void log_out_of_range_and_clamp_(const char *name, float &value, const LogString *const *param_name_progmem, - float min, float max) { +// Cold-path logger: called only after the caller has determined `value` is +// out of range. Does not clamp — the caller handles that with the strategy +// appropriate to its range (bit-pattern clamp_unit_float for [0,1] on the +// hot path, std::clamp for arbitrary ranges like color_temperature). Keeping +// the range check at the caller avoids the call-site spill/reload and +// prologue when the value is in range. The `param_name_progmem` argument +// points into the FIELD_NAMES table in flash; `progmem_read_ptr` is a plain +// `*addr` inline on non-ESP8266 platforms. +static void log_value_out_of_range_(const char *name, float value, const LogString *const *param_name_progmem, + float min, float max) { const auto *param_name = reinterpret_cast(progmem_read_ptr(reinterpret_cast(param_name_progmem))); ESP_LOGW(TAG, "'%s': %s value %.2f is out of range [%.1f - %.1f]", name, LOG_STR_ARG(param_name), value, min, max); - value = clamp(value, min, max); } #if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_WARN @@ -291,6 +294,15 @@ LightColorValues LightCall::validate_() { // offset is exactly 12 bytes lower (enforced by the static_asserts below). // Iterating via bit-position arithmetic lets us collapse eight inlined // clamp/copy blocks into a single loop. + // offsetof is only well-defined on standard-layout types (C++17 relaxed it + // slightly, but GCC still warns on non-standard-layout). Verify here rather + // than relying on diagnostics: a future change that adds a virtual base, a + // non-public data member mixed with public ones, or a derived-class data + // member would break the layout contract below. + static_assert(std::is_standard_layout_v, "LightCall must be standard-layout for offsetof arithmetic"); + static_assert(std::is_standard_layout_v, + "LightColorValues must be standard-layout for offsetof arithmetic"); + constexpr size_t SRC_BASE = offsetof(LightCall, brightness_); constexpr size_t SRC_TO_DST_DELTA = SRC_BASE - offsetof(LightColorValues, brightness_); @@ -335,15 +347,6 @@ LightColorValues LightCall::validate_() { // clear-lowest-bit: HA can drive high-frequency automations through // perform(), so the hot path runs in O(popcount) instead of always // scanning all eight slots. - // - // The range check is done on the IEEE 754 bit pattern as an unsigned int, - // not on the float itself. Values in [0.0f, 1.0f] have bits in - // [0x00000000, 0x3F800000]; anything greater (as unsigned) is out of range: - // values > 1.0f have a larger bit pattern, and negative values have the - // sign bit (0x80000000) set which makes their unsigned interpretation - // enormous. One unsigned compare replaces two soft-float __ltsf2/__gtsf2 - // calls on ESP8266 and is essentially free on targets with an FPU too. - constexpr uint32_t ONE_F_BITS = 0x3F800000u; // bit pattern of 1.0f float *const src_fields = &this->brightness_; float *const dst_fields = &v.brightness_; unsigned active = this->flags_ & CLAMP_FLAGS_MASK; @@ -351,26 +354,24 @@ LightColorValues LightCall::validate_() { unsigned bit = __builtin_ctz(active); active &= active - 1; // clear lowest set bit float &value = src_fields[bit]; - // Union type-pun (GCC/Clang extension): bit_cast/memcpy don't optimize to - // a no-op on xtensa-gcc, same reasoning as api/proto.h float_to_raw(). - union { - float f; - uint32_t u; - } pun; - pun.f = value; - if (pun.u > ONE_F_BITS) - log_out_of_range_and_clamp_(name, value, &FIELD_NAMES[bit], 0.0f, 1.0f); + if (float_out_of_unit_range(value)) { + log_value_out_of_range_(name, value, &FIELD_NAMES[bit], 0.0f, 1.0f); + value = clamp_unit_float(value); + } dst_fields[bit] = value; } // color_temperature uses a dynamic range from the light's traits and is - // handled separately. + // handled separately. No bit-pattern shortcut here because the range is + // runtime-variable. if (this->has_color_temperature()) { static const LogString *const CT_NAME PROGMEM = LOG_STR("Color temperature"); const float ct_min = traits.get_min_mireds(); const float ct_max = traits.get_max_mireds(); - if (this->color_temperature_ < ct_min || this->color_temperature_ > ct_max) - log_out_of_range_and_clamp_(name, this->color_temperature_, &CT_NAME, ct_min, ct_max); + if (this->color_temperature_ < ct_min || this->color_temperature_ > ct_max) { + log_value_out_of_range_(name, this->color_temperature_, &CT_NAME, ct_min, ct_max); + this->color_temperature_ = clamp(this->color_temperature_, ct_min, ct_max); + } v.color_temperature_ = this->color_temperature_; } diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index c520f4dc25..74af29fe59 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -3,11 +3,55 @@ #include "esphome/core/helpers.h" #include "color_mode.h" #include +#include +#include namespace esphome::light { inline static uint8_t to_uint8_scale(float x) { return static_cast(roundf(x * 255.0f)); } +// IEEE 754 bit pattern of 1.0f. Floats in [0.0f, 1.0f] have unsigned bit +// pattern <= this value; negatives have the sign bit set (→ huge unsigned), +// values > 1.0f have a larger exponent, and NaN/Infinity also exceed this. +// Verify the platform actually provides IEEE 754 single-precision floats so +// the bit-pattern tricks below are well-defined. +static constexpr uint32_t ONE_F_BITS = 0x3F800000u; +// sizeof check + is_iec559 together pin the format to IEEE 754 single-precision, +// which fixes the bit pattern of 1.0f as 0x3F800000. A direct bit-cast check +// would be cleaner but __builtin_bit_cast is not available on the older xtensa +// toolchain used for ESP8266. +static_assert(sizeof(float) == sizeof(uint32_t), "float must be 32-bit for bit-pattern range checks"); +static_assert(std::numeric_limits::is_iec559, "IEEE 754 single-precision float required"); + +// Returns true iff `x` is outside [0.0f, 1.0f] via a single unsigned compare on +// its IEEE 754 bit pattern. Uses a union type-pun (GCC/Clang extension) because +// memcpy/bit_cast don't optimize to a no-op on xtensa-gcc (same reasoning as +// api/proto.h's float_to_raw). Replaces two soft-float __ltsf2/__gtsf2 calls +// with one `bltu` on ESP8266 and is free on FPU targets. +inline bool float_out_of_unit_range(float x) { + union { + float f; + uint32_t u; + } pun; + pun.f = x; + return pun.u > ONE_F_BITS; +} + +// Clamps `x` to [0.0f, 1.0f] with no floating-point compares. In-range values +// return via a single branch; out-of-range pick 0.0f for negatives (sign bit +// set) and 1.0f otherwise (> 1.0f, NaN, Infinity). Cheaper than std::clamp on +// ESP8266, which expands to two soft-float calls per invocation. +inline float clamp_unit_float(float x) { + union { + float f; + uint32_t u; + } pun; + pun.f = x; + if (pun.u <= ONE_F_BITS) + return x; + return (pun.u & 0x80000000u) ? 0.0f : 1.0f; +} + /** This class represents the color state for a light object. * * The representation of the color state is dependent on the active color mode. A color mode consists of multiple @@ -220,39 +264,39 @@ class LightColorValues { /// Get the binary true/false state of these light color values. bool is_on() const { return this->get_state() != 0.0f; } /// Set the state of these light color values. In range from 0.0 (off) to 1.0 (on) - void set_state(float state) { this->state_ = clamp(state, 0.0f, 1.0f); } + void set_state(float state) { this->state_ = clamp_unit_float(state); } /// Set the state of these light color values as a binary true/false. void set_state(bool state) { this->state_ = state ? 1.0f : 0.0f; } /// Get the brightness property of these light color values. In range 0.0 to 1.0 float get_brightness() const { return this->brightness_; } /// Set the brightness property of these light color values. In range 0.0 to 1.0 - void set_brightness(float brightness) { this->brightness_ = clamp(brightness, 0.0f, 1.0f); } + void set_brightness(float brightness) { this->brightness_ = clamp_unit_float(brightness); } /// Get the color brightness property of these light color values. In range 0.0 to 1.0 float get_color_brightness() const { return this->color_brightness_; } /// Set the color brightness property of these light color values. In range 0.0 to 1.0 - void set_color_brightness(float brightness) { this->color_brightness_ = clamp(brightness, 0.0f, 1.0f); } + void set_color_brightness(float brightness) { this->color_brightness_ = clamp_unit_float(brightness); } /// Get the red property of these light color values. In range 0.0 to 1.0 float get_red() const { return this->red_; } /// Set the red property of these light color values. In range 0.0 to 1.0 - void set_red(float red) { this->red_ = clamp(red, 0.0f, 1.0f); } + void set_red(float red) { this->red_ = clamp_unit_float(red); } /// Get the green property of these light color values. In range 0.0 to 1.0 float get_green() const { return this->green_; } /// Set the green property of these light color values. In range 0.0 to 1.0 - void set_green(float green) { this->green_ = clamp(green, 0.0f, 1.0f); } + void set_green(float green) { this->green_ = clamp_unit_float(green); } /// Get the blue property of these light color values. In range 0.0 to 1.0 float get_blue() const { return this->blue_; } /// Set the blue property of these light color values. In range 0.0 to 1.0 - void set_blue(float blue) { this->blue_ = clamp(blue, 0.0f, 1.0f); } + void set_blue(float blue) { this->blue_ = clamp_unit_float(blue); } /// Get the white property of these light color values. In range 0.0 to 1.0 float get_white() const { return white_; } /// Set the white property of these light color values. In range 0.0 to 1.0 - void set_white(float white) { this->white_ = clamp(white, 0.0f, 1.0f); } + void set_white(float white) { this->white_ = clamp_unit_float(white); } /// Get the color temperature property of these light color values in mired. float get_color_temperature() const { return this->color_temperature_; } @@ -277,12 +321,12 @@ class LightColorValues { /// Get the cold white property of these light color values. In range 0.0 to 1.0. float get_cold_white() const { return this->cold_white_; } /// Set the cold white property of these light color values. In range 0.0 to 1.0. - void set_cold_white(float cold_white) { this->cold_white_ = clamp(cold_white, 0.0f, 1.0f); } + void set_cold_white(float cold_white) { this->cold_white_ = clamp_unit_float(cold_white); } /// Get the warm white property of these light color values. In range 0.0 to 1.0. float get_warm_white() const { return this->warm_white_; } /// Set the warm white property of these light color values. In range 0.0 to 1.0. - void set_warm_white(float warm_white) { this->warm_white_ = clamp(warm_white, 0.0f, 1.0f); } + void set_warm_white(float warm_white) { this->warm_white_ = clamp_unit_float(warm_white); } friend class LightCall;