Static queued segment bounds

(saves 180 bytes of RAM)
Fixed segment index not increasing on inactive segments

wled00/FX.h

@@ -395,8 +395,8 @@ typedef struct Segment {
     uint16_t        _dataLen;
     static uint16_t _usedSegmentData;
 
-    uint16_t _qStart, _qStop, _qStartY, _qStopY;
-    bool     _queuedChanges;
+    static uint16_t _qStart, _qStop, _qStartY, _qStopY;
+    static uint8_t _queuedChangesSegId;
 
     // transition data, valid only if transitional==true, holds values during transition (72 bytes)
     struct Transition {
@@ -466,7 +466,6 @@ typedef struct Segment {
       data(nullptr),
       _capabilities(0),
       _dataLen(0),
-      _queuedChanges(false),
       _t(nullptr)
     {
       //refreshLightCapabilities();
@@ -515,7 +514,7 @@ typedef struct Segment {
     static uint16_t getUsedSegmentData(void)    { return _usedSegmentData; }
     static void     addUsedSegmentData(int len) { _usedSegmentData += len; }
 
-    void    setUp(uint16_t i1, uint16_t i2, uint8_t grp=1, uint8_t spc=0, uint16_t ofs=UINT16_MAX, uint16_t i1Y=0, uint16_t i2Y=1, bool immediate=false);
+    void    setUp(uint16_t i1, uint16_t i2, uint8_t grp=1, uint8_t spc=0, uint16_t ofs=UINT16_MAX, uint16_t i1Y=0, uint16_t i2Y=1, uint8_t segId = 255);
     bool    setColor(uint8_t slot, uint32_t c); //returns true if changed
     void    setCCT(uint16_t k);
     void    setOpacity(uint8_t o);

wled00/FX_fcn.cpp

@@ -76,6 +76,9 @@
 uint16_t Segment::_usedSegmentData = 0U; // amount of RAM all segments use for their data[]
 uint16_t Segment::maxWidth = DEFAULT_LED_COUNT;
 uint16_t Segment::maxHeight = 1;
+uint8_t  Segment::_queuedChangesSegId = 255U;
+uint16_t Segment::_qStart  = 0, Segment::_qStop  = 0;
+uint16_t Segment::_qStartY = 0, Segment::_qStopY = 0;
 
 // copy constructor
 Segment::Segment(const Segment &orig) {
@@ -174,12 +177,15 @@ void Segment::deallocateData() {
   * may free that data buffer.
   */
 void Segment::resetIfRequired() {
-  if (reset) {
-    deallocateData();
-    next_time = 0; step = 0; call = 0; aux0 = 0; aux1 = 0;
-    if (_queuedChanges) setUp(_qStart, _qStop, grouping, spacing, offset, _qStartY, _qStopY, true);
-    reset = false; // setOption(SEG_OPTION_RESET, false);
+  if (!reset) return;
+  
+  deallocateData();
+  next_time = 0; step = 0; call = 0; aux0 = 0; aux1 = 0;
+  if (_queuedChangesSegId == strip.getCurrSegmentId()) { // apply queued changes
+    setUp(_qStart, _qStop, grouping, spacing, offset, _qStartY, _qStopY);
+    _queuedChangesSegId = 255;
   }
+  reset = false;
 }
 
 CRGBPalette16 &Segment::loadPalette(CRGBPalette16 &targetPalette, uint8_t pal) {
@@ -354,8 +360,10 @@ void Segment::handleTransition() {
   }
 }
 
-void Segment::setUp(uint16_t i1, uint16_t i2, uint8_t grp, uint8_t spc, uint16_t ofs, uint16_t i1Y, uint16_t i2Y, bool immediate) {
-  _queuedChanges = false; // cancel anything queued
+// segId is given when called from network callback, changes are queued if that segment is currently in its effect function
+void Segment::setUp(uint16_t i1, uint16_t i2, uint8_t grp, uint8_t spc, uint16_t ofs, uint16_t i1Y, uint16_t i2Y, uint8_t segId) {
+  if (_queuedChangesSegId == segId) _queuedChangesSegId = 255; // cancel queued change if already queued for this segment
+
   // return if neither bounds nor grouping have changed
   bool boundsUnchanged = (start == i1 && stop == i2);
   #ifndef WLED_DISABLE_2D
@@ -376,36 +384,39 @@ void Segment::setUp(uint16_t i1, uint16_t i2, uint8_t grp, uint8_t spc, uint16_t
   if (ofs < UINT16_MAX) offset = ofs;
 
   markForReset();
-  if (!boundsUnchanged) {
-    if (immediate) {
-      if (i2 <= i1) { //disable segment
-        stop = 0;
-        return;
-      }
-      if (i1 < Segment::maxWidth || (i1 >= Segment::maxWidth*Segment::maxHeight && i1 < strip.getLengthTotal())) start = i1; // Segment::maxWidth equals strip.getLengthTotal() for 1D
-      stop = i2 > Segment::maxWidth*Segment::maxHeight ? MIN(i2,strip.getLengthTotal()) : (i2 > Segment::maxWidth ? Segment::maxWidth : MAX(1,i2));
-      startY = 0;
-      stopY  = 1;
-      #ifndef WLED_DISABLE_2D
-      if (Segment::maxHeight>1) { // 2D
-        if (i1Y < Segment::maxHeight) startY = i1Y;
-        stopY = i2Y > Segment::maxHeight ? Segment::maxHeight : MAX(1,i2Y);
-      }
-      #endif
-      // safety check
-      if (start >= stop || startY >= stopY) {
-        stop = 0;
-        return;
-      }
-      if (!boundsUnchanged) refreshLightCapabilities();
-    } else {
-      _qStart  = i1;
-      _qStop   = i2;
-      _qStartY = i1Y;
-      _qStopY  = i2Y;
-      _queuedChanges = true;
-    }
+  if (boundsUnchanged) return; // TODO test if it is save to change grp/spc/ofs without queueing
+  // queuing a change for a second segment will lead to the loss of the first change if not yet applied
+  // however this is not a problem as the queued change is applied immediately after the effect function in that segment returns
+  if (segId < MAX_NUM_SEGMENTS && segId == strip.getCurrSegmentId() && strip.isServicing()) { // queue change to prevent concurrent access
+    _qStart  = i1;
+    _qStop   = i2;
+    _qStartY = i1Y;
+    _qStopY  = i2Y;
+    _queuedChangesSegId = segId;
+    return; // queued changes are applied immediately after effect function returns
   }
+
+  // apply change immediately
+  if (i2 <= i1) { //disable segment
+    stop = 0;
+    return;
+  }
+  if (i1 < Segment::maxWidth || (i1 >= Segment::maxWidth*Segment::maxHeight && i1 < strip.getLengthTotal())) start = i1; // Segment::maxWidth equals strip.getLengthTotal() for 1D
+  stop = i2 > Segment::maxWidth*Segment::maxHeight ? MIN(i2,strip.getLengthTotal()) : (i2 > Segment::maxWidth ? Segment::maxWidth : MAX(1,i2));
+  startY = 0;
+  stopY  = 1;
+  #ifndef WLED_DISABLE_2D
+  if (Segment::maxHeight>1) { // 2D
+    if (i1Y < Segment::maxHeight) startY = i1Y;
+    stopY = i2Y > Segment::maxHeight ? Segment::maxHeight : MAX(1,i2Y);
+  }
+  #endif
+  // safety check
+  if (start >= stop || startY >= stopY) {
+    stop = 0;
+    return;
+  }
+  refreshLightCapabilities();
 }
 
 
@@ -1087,7 +1098,7 @@ void WS2812FX::service() {
     // reset the segment runtime data if needed
     seg.resetIfRequired();
 
-    if (!seg.isActive()) continue;
+    if (!seg.isActive()) { _segment_index++; continue; }
 
     // last condition ensures all solid segments are updated at the same time
     if(nowUp > seg.next_time || _triggered || (doShow && seg.mode == FX_MODE_STATIC))
@@ -1115,6 +1126,7 @@ void WS2812FX::service() {
 
       seg.next_time = nowUp + delay;
     }
+    seg.resetIfRequired(); // another reset chance, mainly to apply new segment bounds if queued
     _segment_index++;
   }
   _virtualSegmentLength = 0;

wled00/json.cpp

@@ -112,8 +112,8 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId)
   if (stop > start && of > len -1) of = len -1;
 
   // update segment (delete if necessary)
-  // we must not change segment dimensions during drawing of effects as that may produce undesired behaviour (crash)
-  seg.setUp(start, stop, grp, spc, of, startY, stopY, !strip.isServicing());
+  // we must not change segment dimensions during drawing of effects in that segment as concurrent access may cause a crash
+  seg.setUp(start, stop, grp, spc, of, startY, stopY, id);
 
   if (seg.reset && seg.stop == 0) return true; // segment was deleted & is marked for reset, no need to change anything else
 
@@ -1065,7 +1065,10 @@ void serveJson(AsyncWebServerRequest* request)
 
   DEBUG_PRINTF("JSON buffer size: %u for request: %d\n", lDoc.memoryUsage(), subJson);
 
-  size_t len = response->setLength();
+  #ifdef WLED_DEBUG
+  size_t len =
+  #endif
+  response->setLength();
   DEBUG_PRINT(F("JSON content length: ")); DEBUG_PRINTLN(len);
 
   request->send(response);