From 7071a5870f18f2e527a7f7c5694f86812697d2d7 Mon Sep 17 00:00:00 2001 From: TripleWhy Date: Fri, 28 Feb 2025 22:05:03 +0100 Subject: [PATCH 1/8] Append segments by moving them --- wled00/FX.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wled00/FX.h b/wled00/FX.h index 3544e1fa7f..70318f9764 100644 --- a/wled00/FX.h +++ b/wled00/FX.h @@ -883,7 +883,7 @@ class WS2812FX { // 96 bytes inline void trigger() { _triggered = true; } // Forces the next frame to be computed on all active segments. inline void setShowCallback(show_callback cb) { _callback = cb; } inline void setTransition(uint16_t t) { _transitionDur = t; } // sets transition time (in ms) - inline void appendSegment(const Segment &seg = Segment()) { if (_segments.size() < getMaxSegments()) _segments.push_back(seg); } + inline void appendSegment(Segment &&seg = Segment()) { if (_segments.size() < getMaxSegments()) _segments.push_back(std::move(seg)); } inline void suspend() { _suspend = true; } // will suspend (and canacel) strip.service() execution inline void resume() { _suspend = false; } // will resume strip.service() execution From 90a49c5a072c668a8c855beb765e029d79821e34 Mon Sep 17 00:00:00 2001 From: TripleWhy Date: Fri, 28 Feb 2025 22:27:22 +0100 Subject: [PATCH 2/8] Move segment json-specific segment diffing to json.cpp - Avoid calling Segment copy constructor and destructor. This is fine as long as we only read primitive values. - Only diff stuff we are actually interested in. - Only diff stuff if it has an effect. --- wled00/FX.h | 1 - wled00/FX_fcn.cpp | 29 +---------------------------- wled00/json.cpp | 33 +++++++++++++++++++++++++++++++-- 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/wled00/FX.h b/wled00/FX.h index 70318f9764..c359d5d1c3 100644 --- a/wled00/FX.h +++ b/wled00/FX.h @@ -626,7 +626,6 @@ typedef struct Segment { Segment &setMode(uint8_t fx, bool loadDefaults = false); Segment &setPalette(uint8_t pal); Segment &setName(const char* name); - uint8_t differs(const Segment& b) const; void refreshLightCapabilities(); // runtime data functions diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index 7bd875ad0d..f1df8b52f8 100644 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -1084,33 +1084,6 @@ uint32_t IRAM_ATTR_YN Segment::getPixelColor(int i) const return strip.getPixelColor(i); } -uint8_t Segment::differs(const Segment& b) const { - uint8_t d = 0; - if (start != b.start) d |= SEG_DIFFERS_BOUNDS; - if (stop != b.stop) d |= SEG_DIFFERS_BOUNDS; - if (offset != b.offset) d |= SEG_DIFFERS_GSO; - if (grouping != b.grouping) d |= SEG_DIFFERS_GSO; - if (spacing != b.spacing) d |= SEG_DIFFERS_GSO; - if (opacity != b.opacity) d |= SEG_DIFFERS_BRI; - if (mode != b.mode) d |= SEG_DIFFERS_FX; - if (speed != b.speed) d |= SEG_DIFFERS_FX; - if (intensity != b.intensity) d |= SEG_DIFFERS_FX; - if (palette != b.palette) d |= SEG_DIFFERS_FX; - if (custom1 != b.custom1) d |= SEG_DIFFERS_FX; - if (custom2 != b.custom2) d |= SEG_DIFFERS_FX; - if (custom3 != b.custom3) d |= SEG_DIFFERS_FX; - if (startY != b.startY) d |= SEG_DIFFERS_BOUNDS; - if (stopY != b.stopY) d |= SEG_DIFFERS_BOUNDS; - - //bit pattern: (msb first) - // set:2, sound:2, mapping:3, transposed, mirrorY, reverseY, [reset,] paused, mirrored, on, reverse, [selected] - if ((options & 0b1111111111011110U) != (b.options & 0b1111111111011110U)) d |= SEG_DIFFERS_OPT; - if ((options & 0x0001U) != (b.options & 0x0001U)) d |= SEG_DIFFERS_SEL; - for (unsigned i = 0; i < NUM_COLORS; i++) if (colors[i] != b.colors[i]) d |= SEG_DIFFERS_COL; - - return d; -} - void Segment::refreshLightCapabilities() { unsigned capabilities = 0; unsigned segStartIdx = 0xFFFFU; @@ -1790,7 +1763,7 @@ void WS2812FX::resetSegments() { #else segment seg = Segment(0, _length); #endif - _segments.push_back(seg); + _segments.push_back(std::move(seg)); _segments.shrink_to_fit(); // just in case ... _mainSegment = 0; } diff --git a/wled00/json.cpp b/wled00/json.cpp index 200307464e..d17b0167b8 100644 --- a/wled00/json.cpp +++ b/wled00/json.cpp @@ -15,6 +15,34 @@ * JSON API (De)serialization */ +namespace { +bool differs(const Segment& segment, const std::array& backup) { + const Segment& segmentBackup = *reinterpret_cast(backup.data()); + if (segment.start != segmentBackup.start) return true; + if (segment.stop != segmentBackup.stop) return true; + if (segment.offset != segmentBackup.offset) return true; + if (segment.grouping != segmentBackup.grouping) return true; + if (segment.spacing != segmentBackup.spacing) return true; + if (segment.opacity != segmentBackup.opacity) return true; + if (segment.mode != segmentBackup.mode) return true; + if (segment.speed != segmentBackup.speed) return true; + if (segment.intensity != segmentBackup.intensity) return true; + if (segment.palette != segmentBackup.palette) return true; + if (segment.custom1 != segmentBackup.custom1) return true; + if (segment.custom2 != segmentBackup.custom2) return true; + if (segment.custom3 != segmentBackup.custom3) return true; + if (segment.startY != segmentBackup.startY) return true; + if (segment.stopY != segmentBackup.stopY) return true; + + //bit pattern: (msb first) + // set:2, sound:2, mapping:3, transposed, mirrorY, reverseY, [reset,] paused, mirrored, on, reverse, [selected] + if ((segment.options & 0b1111111111011110U) != (segmentBackup.options & 0b1111111111011110U)) return true; + for (unsigned i = 0; i < NUM_COLORS; i++) if (segment.colors[i] != segmentBackup.colors[i]) return true; + + return false; +} +} + bool deserializeSegment(JsonObject elem, byte it, byte presetId) { byte id = elem["id"] | it; @@ -34,7 +62,8 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId) //DEBUG_PRINTLN(F("-- JSON deserialize segment.")); Segment& seg = strip.getSegment(id); //DEBUG_PRINTF_P(PSTR("-- Original segment: %p (%p)\n"), &seg, seg.data); - const Segment prev = seg; //make a backup so we can tell if something changed (calling copy constructor) + alignas(Segment) std::array segmentBackup; //make a backup so we can tell if something changed (voiding copy constructor) + std::memcpy(segmentBackup.data(), &seg, sizeof(Segment)); //DEBUG_PRINTF_P(PSTR("-- Duplicate segment: %p (%p)\n"), &prev, prev.data); int start = elem["start"] | seg.start; @@ -293,7 +322,7 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId) strip.trigger(); // force segment update } // send UDP/WS if segment options changed (except selection; will also deselect current preset) - if (seg.differs(prev) & 0x7F) stateChanged = true; + stateChanged = stateChanged || differs(seg, segmentBackup); return true; } From 5f26983f8c41f02fb5e859660ac4395a797f8303 Mon Sep 17 00:00:00 2001 From: TripleWhy Date: Sat, 1 Mar 2025 10:37:51 +0100 Subject: [PATCH 3/8] applyValuesToSelectedSegs(): Avoid full Segment copy --- wled00/led.cpp | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/wled00/led.cpp b/wled00/led.cpp index 9731da655a..7c2f50feaf 100644 --- a/wled00/led.cpp +++ b/wled00/led.cpp @@ -28,21 +28,27 @@ void setValuesFromSegment(uint8_t s) // problem: if the first selected segment already has the value to be set, other selected segments are not updated void applyValuesToSelectedSegs() { - // copy of first selected segment to tell if value was updated - unsigned firstSel = strip.getFirstSelectedSegId(); - Segment selsegPrev = strip.getSegment(firstSel); - for (unsigned i = 0; i < strip.getSegmentsNum(); i++) { - Segment& seg = strip.getSegment(i); - if (i != firstSel && (!seg.isActive() || !seg.isSelected())) continue; - - if (effectSpeed != selsegPrev.speed) {seg.speed = effectSpeed; stateChanged = true;} - if (effectIntensity != selsegPrev.intensity) {seg.intensity = effectIntensity; stateChanged = true;} - if (effectPalette != selsegPrev.palette) {seg.setPalette(effectPalette);} - if (effectCurrent != selsegPrev.mode) {seg.setMode(effectCurrent);} - uint32_t col0 = RGBW32( col[0], col[1], col[2], col[3]); - uint32_t col1 = RGBW32(colSec[0], colSec[1], colSec[2], colSec[3]); - if (col0 != selsegPrev.colors[0]) {seg.setColor(0, col0);} - if (col1 != selsegPrev.colors[1]) {seg.setColor(1, col1);} + // copy of first selected segment data to tell if value was updated + const uint8_t firstSel = strip.getFirstSelectedSegId(); + const Segment& firstSelectedSegment = strip.getSegment(firstSel); + const uint8_t previousSpeed = firstSelectedSegment.speed; + const uint8_t previousIntensity = firstSelectedSegment.intensity; + const uint32_t previousColor0 = firstSelectedSegment.colors[0]; + const uint8_t previousPalette = firstSelectedSegment.palette; + const uint8_t previousMode = firstSelectedSegment.mode; + const uint32_t previousColor1 = firstSelectedSegment.colors[1]; + + const uint32_t col0 = RGBW32( col[0], col[1], col[2], col[3]); + const uint32_t col1 = RGBW32(colSec[0], colSec[1], colSec[2], colSec[3]); + for (Segment& seg : strip._segments) { + if (&seg != &firstSelectedSegment && (!seg.isActive() || !seg.isSelected())) continue; + + if (effectSpeed != previousSpeed) {seg.speed = effectSpeed; stateChanged = true;} + if (effectIntensity != previousIntensity) {seg.intensity = effectIntensity; stateChanged = true;} + if (effectPalette != previousPalette) {seg.setPalette(effectPalette);} + if (effectCurrent != previousMode) {seg.setMode(effectCurrent);} + if (col0 != previousColor0) {seg.setColor(0, col0);} + if (col1 != previousColor1) {seg.setColor(1, col1);} } } From a0fc6bfb1add926bb9e017b997a03b3d88fb4fdb Mon Sep 17 00:00:00 2001 From: TripleWhy Date: Sat, 1 Mar 2025 11:18:06 +0100 Subject: [PATCH 4/8] Fix for ancient compilers --- wled00/json.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wled00/json.cpp b/wled00/json.cpp index d17b0167b8..164afb8e63 100644 --- a/wled00/json.cpp +++ b/wled00/json.cpp @@ -16,7 +16,7 @@ */ namespace { -bool differs(const Segment& segment, const std::array& backup) { +bool differs(const Segment& segment, const std::array& backup) { const Segment& segmentBackup = *reinterpret_cast(backup.data()); if (segment.start != segmentBackup.start) return true; if (segment.stop != segmentBackup.stop) return true; @@ -62,8 +62,8 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId) //DEBUG_PRINTLN(F("-- JSON deserialize segment.")); Segment& seg = strip.getSegment(id); //DEBUG_PRINTF_P(PSTR("-- Original segment: %p (%p)\n"), &seg, seg.data); - alignas(Segment) std::array segmentBackup; //make a backup so we can tell if something changed (voiding copy constructor) - std::memcpy(segmentBackup.data(), &seg, sizeof(Segment)); + alignas(Segment) std::array segmentBackup; //make a backup so we can tell if something changed (voiding copy constructor) + memcpy(segmentBackup.data(), &seg, sizeof(Segment)); //DEBUG_PRINTF_P(PSTR("-- Duplicate segment: %p (%p)\n"), &prev, prev.data); int start = elem["start"] | seg.start; From 8efc2dd3b90fcab5a671c0d4f88e2306fdcf0d8c Mon Sep 17 00:00:00 2001 From: TripleWhy Date: Sat, 1 Mar 2025 12:45:53 +0100 Subject: [PATCH 5/8] Replace some _segments.push_back calls with _segments.emplace_back calls --- wled00/FX_fcn.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index f1df8b52f8..b7d0a41ed2 100644 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -1759,11 +1759,14 @@ Segment& WS2812FX::getSegment(unsigned id) { void WS2812FX::resetSegments() { _segments.clear(); // destructs all Segment as part of clearing #ifndef WLED_DISABLE_2D - segment seg = isMatrix ? Segment(0, Segment::maxWidth, 0, Segment::maxHeight) : Segment(0, _length); + if (isMatrix) { + _segments.emplace_back(0, Segment::maxWidth, 0, Segment::maxHeight); + } else { + _segments.emplace_back(0, _length); + } #else - segment seg = Segment(0, _length); + _segments.emplace_back(0, _length); #endif - _segments.push_back(std::move(seg)); _segments.shrink_to_fit(); // just in case ... _mainSegment = 0; } @@ -1811,12 +1814,12 @@ void WS2812FX::makeAutoSegments(bool forceReset) { // there is always at least one segment (but we need to differentiate between 1D and 2D) #ifndef WLED_DISABLE_2D if (isMatrix) - _segments.push_back(Segment(0, Segment::maxWidth, 0, Segment::maxHeight)); + _segments.emplace_back(0, Segment::maxWidth, 0, Segment::maxHeight); else #endif - _segments.push_back(Segment(segStarts[0], segStops[0])); + _segments.emplace_back(segStarts[0], segStops[0]); for (size_t i = 1; i < s; i++) { - _segments.push_back(Segment(segStarts[i], segStops[i])); + _segments.emplace_back(segStarts[i], segStops[i]); } DEBUG_PRINTF_P(PSTR("%d auto segments created.\n"), _segments.size()); From 9dadc0628cabcdf4f02549184254d04db05775a8 Mon Sep 17 00:00:00 2001 From: TripleWhy Date: Sun, 2 Mar 2025 09:30:38 +0100 Subject: [PATCH 6/8] Replace appendSegment with emplaceSegment --- wled00/FX.h | 3 ++- wled00/json.cpp | 2 +- wled00/udp.cpp | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/wled00/FX.h b/wled00/FX.h index c359d5d1c3..afa6057231 100644 --- a/wled00/FX.h +++ b/wled00/FX.h @@ -882,7 +882,8 @@ class WS2812FX { // 96 bytes inline void trigger() { _triggered = true; } // Forces the next frame to be computed on all active segments. inline void setShowCallback(show_callback cb) { _callback = cb; } inline void setTransition(uint16_t t) { _transitionDur = t; } // sets transition time (in ms) - inline void appendSegment(Segment &&seg = Segment()) { if (_segments.size() < getMaxSegments()) _segments.push_back(std::move(seg)); } + template + inline void emplaceSegment(Args&&... args) { if (_segments.size() < getMaxSegments()) _segments.emplace_back(std::forward(args)...); } inline void suspend() { _suspend = true; } // will suspend (and canacel) strip.service() execution inline void resume() { _suspend = false; } // will resume strip.service() execution diff --git a/wled00/json.cpp b/wled00/json.cpp index 164afb8e63..333d338e46 100644 --- a/wled00/json.cpp +++ b/wled00/json.cpp @@ -54,7 +54,7 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId) // append segment if (id >= strip.getSegmentsNum()) { if (stop <= 0) return false; // ignore empty/inactive segments - strip.appendSegment(Segment(0, strip.getLengthTotal())); + strip.emplaceSegment(0, strip.getLengthTotal()); id = strip.getSegmentsNum()-1; // segments are added at the end of list newSeg = true; } diff --git a/wled00/udp.cpp b/wled00/udp.cpp index 4395b285d0..3dada3a9ef 100644 --- a/wled00/udp.cpp +++ b/wled00/udp.cpp @@ -272,7 +272,7 @@ static void parseNotifyPacket(const uint8_t *udpIn) { DEBUG_PRINTF_P(PSTR("UDP segment received: %u\n"), id); if (id > strip.getSegmentsNum()) break; else if (id == strip.getSegmentsNum()) { - if (receiveSegmentBounds && id < strip.getMaxSegments()) strip.appendSegment(); + if (receiveSegmentBounds && id < strip.getMaxSegments()) strip.emplaceSegment(); else break; } DEBUG_PRINTF_P(PSTR("UDP segment check: %u\n"), id); From 7785114f986f2c46b299f051448770ffd7d7b575 Mon Sep 17 00:00:00 2001 From: TripleWhy Date: Mon, 3 Mar 2025 13:41:46 +0100 Subject: [PATCH 7/8] Adjust applyValuesToSelectedSegs() according to blazoncek's suggestion https://github.com/wled/WLED/pull/4578#discussion_r1977156345 --- wled00/led.cpp | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/wled00/led.cpp b/wled00/led.cpp index 7c2f50feaf..93adece8f7 100644 --- a/wled00/led.cpp +++ b/wled00/led.cpp @@ -25,30 +25,18 @@ void setValuesFromSegment(uint8_t s) // applies global legacy values (col, colSec, effectCurrent...) -// problem: if the first selected segment already has the value to be set, other selected segments are not updated -void applyValuesToSelectedSegs() -{ - // copy of first selected segment data to tell if value was updated - const uint8_t firstSel = strip.getFirstSelectedSegId(); - const Segment& firstSelectedSegment = strip.getSegment(firstSel); - const uint8_t previousSpeed = firstSelectedSegment.speed; - const uint8_t previousIntensity = firstSelectedSegment.intensity; - const uint32_t previousColor0 = firstSelectedSegment.colors[0]; - const uint8_t previousPalette = firstSelectedSegment.palette; - const uint8_t previousMode = firstSelectedSegment.mode; - const uint32_t previousColor1 = firstSelectedSegment.colors[1]; - +void applyValuesToSelectedSegs() { const uint32_t col0 = RGBW32( col[0], col[1], col[2], col[3]); const uint32_t col1 = RGBW32(colSec[0], colSec[1], colSec[2], colSec[3]); for (Segment& seg : strip._segments) { - if (&seg != &firstSelectedSegment && (!seg.isActive() || !seg.isSelected())) continue; - - if (effectSpeed != previousSpeed) {seg.speed = effectSpeed; stateChanged = true;} - if (effectIntensity != previousIntensity) {seg.intensity = effectIntensity; stateChanged = true;} - if (effectPalette != previousPalette) {seg.setPalette(effectPalette);} - if (effectCurrent != previousMode) {seg.setMode(effectCurrent);} - if (col0 != previousColor0) {seg.setColor(0, col0);} - if (col1 != previousColor1) {seg.setColor(1, col1);} + if (!(seg.isActive() && seg.isSelected())) continue; + stateChanged = stateChanged || (effectSpeed != seg.speed) || (effectIntensity != seg.intensity); + seg.speed = effectSpeed; + seg.intensity = effectIntensity; + seg.setPalette(effectPalette); + seg.setMode(effectCurrent); + seg.setColor(0, col0); + seg.setColor(1, col1); } } From 46bee5ad7bdeab03100d2068fe4afc15cf60ebf8 Mon Sep 17 00:00:00 2001 From: TripleWhy Date: Fri, 6 Jun 2025 14:11:44 +0200 Subject: [PATCH 8/8] Minor corrections - Name the SegmentCopy struct. Some things don't work with typedefed anonymous types. - Remove unused values from SegmentCopy. --- wled00/json.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/wled00/json.cpp b/wled00/json.cpp index 31b0e3afb7..67630bda6e 100644 --- a/wled00/json.cpp +++ b/wled00/json.cpp @@ -15,7 +15,7 @@ * JSON API (De)serialization */ namespace { - typedef struct { + struct SegmentCopy { uint32_t colors[NUM_COLORS]; uint16_t start; uint16_t stop; @@ -33,10 +33,7 @@ namespace { uint8_t custom1; uint8_t custom2; uint8_t custom3; - bool check1; - bool check2; - bool check3; - } SegmentCopy; + }; bool differs(const Segment& segment, const SegmentCopy& segmentBackup) { if (segment.start != segmentBackup.start) return true; @@ -101,10 +98,7 @@ static bool deserializeSegment(JsonObject elem, byte it, byte presetId) seg.intensity, seg.custom1, seg.custom2, - seg.custom3, - seg.check1, - seg.check2, - seg.check3 + seg.custom3 }; int start = elem["start"] | seg.start;