From a8785570df453389ac88b6a8e8a550a897bedd93 Mon Sep 17 00:00:00 2001 From: Blaz Kristan Date: Sat, 6 Aug 2022 12:39:12 +0200 Subject: [PATCH] Memory allocation fixes. Whitespace. Cleanup. --- wled00/FX.h | 7 ++- wled00/FX_2Dfcn.cpp | 28 ++++----- wled00/FX_fcn.cpp | 107 ++++++++++++++++------------------- wled00/data/liveviewws2D.htm | 26 ++++----- wled00/wled.h | 2 +- 5 files changed, 80 insertions(+), 90 deletions(-) diff --git a/wled00/FX.h b/wled00/FX.h index d2b01c89..d3c7733c 100644 --- a/wled00/FX.h +++ b/wled00/FX.h @@ -449,7 +449,7 @@ typedef struct Segment { uint16_t _dataLen; static uint16_t _usedSegmentData; - // transition data, valid only if getOption(SEG_OPTION_TRANSITIONAL)==true, holds values during transition + // transition data, valid only if transitional==true, holds values during transition struct Transition { uint32_t _colorT[NUM_COLORS]; uint8_t _briT; // temporary brightness @@ -526,11 +526,11 @@ typedef struct Segment { Segment& operator= (Segment &&orig) noexcept; // move assignment #ifdef WLED_DEBUG - size_t getSize() { return sizeof(Segment) + (data?_dataLen:0) + (name?strlen(name):0) + (_t?sizeof(Transition):0) + (leds?sizeof(CRGB)*length():0); } + size_t getSize() { return sizeof(Segment) + (data?_dataLen:0) + (name?strlen(name):0) + (_t?sizeof(Transition):0) + (!Segment::_globalLeds && leds?sizeof(CRGB)*length():0); } #endif inline bool getOption(uint8_t n) { return ((options >> n) & 0x01); } - inline bool isSelected(void) { return getOption(0); } + inline bool isSelected(void) { return selected; } inline bool isActive(void) { return stop > start; } inline bool is2D(void) { return !(startY == 0 && stopY == 1); } inline uint16_t width(void) { return stop - start; } // segment width in physical pixels (length if 1D) @@ -737,6 +737,7 @@ class WS2812FX { // 96 bytes _modeData.clear(); _segments.clear(); customPalettes.clear(); + if (useLedsArray && Segment::_globalLeds) free(Segment::_globalLeds); } static WS2812FX* getInstance(void) { return instance; } diff --git a/wled00/FX_2Dfcn.cpp b/wled00/FX_2Dfcn.cpp index 32f6a755..df119bfc 100644 --- a/wled00/FX_2Dfcn.cpp +++ b/wled00/FX_2Dfcn.cpp @@ -153,7 +153,7 @@ void IRAM_ATTR Segment::setPixelColorXY(int x, int y, uint32_t col) if (leds) leds[XY(x,y)] = col; - uint8_t _bri_t = currentBri(getOption(SEG_OPTION_ON) ? opacity : 0); + uint8_t _bri_t = currentBri(on ? opacity : 0); if (_bri_t < 255) { byte r = scale8(R(col), _bri_t); byte g = scale8(G(col), _bri_t); @@ -162,9 +162,9 @@ void IRAM_ATTR Segment::setPixelColorXY(int x, int y, uint32_t col) col = RGBW32(r, g, b, w); } - if (getOption(SEG_OPTION_REVERSED) ) x = virtualWidth() - x - 1; - if (getOption(SEG_OPTION_REVERSED_Y)) y = virtualHeight() - y - 1; - if (getOption(SEG_OPTION_TRANSPOSED)) { uint16_t t = x; x = y; y = t; } // swap X & Y if segment transposed + if (reverse ) x = virtualWidth() - x - 1; + if (reverse_y) y = virtualHeight() - y - 1; + if (transpose) { uint16_t t = x; x = y; y = t; } // swap X & Y if segment transposed x *= groupLength(); // expand to physical pixels y *= groupLength(); // expand to physical pixels @@ -177,15 +177,15 @@ void IRAM_ATTR Segment::setPixelColorXY(int x, int y, uint32_t col) strip.setPixelColorXY(start + xX, startY + yY, col); - if (getOption(SEG_OPTION_MIRROR)) { //set the corresponding horizontally mirrored pixel - if (getOption(SEG_OPTION_TRANSPOSED)) strip.setPixelColorXY(start + xX, startY + height() - yY - 1, col); - else strip.setPixelColorXY(start + width() - xX - 1, startY + yY, col); + if (mirror) { //set the corresponding horizontally mirrored pixel + if (transpose) strip.setPixelColorXY(start + xX, startY + height() - yY - 1, col); + else strip.setPixelColorXY(start + width() - xX - 1, startY + yY, col); } - if (getOption(SEG_OPTION_MIRROR_Y)) { //set the corresponding vertically mirrored pixel - if (getOption(SEG_OPTION_TRANSPOSED)) strip.setPixelColorXY(start + width() - xX - 1, startY + yY, col); - else strip.setPixelColorXY(start + xX, startY + height() - yY - 1, col); + if (mirror_y) { //set the corresponding vertically mirrored pixel + if (transpose) strip.setPixelColorXY(start + width() - xX - 1, startY + yY, col); + else strip.setPixelColorXY(start + xX, startY + height() - yY - 1, col); } - if (getOption(SEG_OPTION_MIRROR_Y) && getOption(SEG_OPTION_MIRROR)) { //set the corresponding vertically AND horizontally mirrored pixel + if (mirror_y && mirror) { //set the corresponding vertically AND horizontally mirrored pixel strip.setPixelColorXY(width() - xX - 1, height() - yY - 1, col); } } @@ -240,9 +240,9 @@ void Segment::setPixelColorXY(float x, float y, uint32_t col, bool aa) uint32_t Segment::getPixelColorXY(uint16_t x, uint16_t y) { int i = XY(x,y); if (leds) return RGBW32(leds[i].r, leds[i].g, leds[i].b, 0); - if (getOption(SEG_OPTION_REVERSED) ) x = virtualWidth() - x - 1; - if (getOption(SEG_OPTION_REVERSED_Y)) y = virtualHeight() - y - 1; - if (getOption(SEG_OPTION_TRANSPOSED)) { uint16_t t = x; x = y; y = t; } // swap X & Y if segment transposed + if (reverse ) x = virtualWidth() - x - 1; + if (reverse_y) y = virtualHeight() - y - 1; + if (transpose) { uint16_t t = x; x = y; y = t; } // swap X & Y if segment transposed x *= groupLength(); // expand to physical pixels y *= groupLength(); // expand to physical pixels if (x >= width() || y >= height()) return 0; diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index 4b3f971a..a9b93bb2 100644 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -78,23 +78,22 @@ CRGB *Segment::_globalLeds = nullptr; // copy constructor Segment::Segment(const Segment &orig) { - DEBUG_PRINTLN(F("-- Segment duplicated --")); + DEBUG_PRINTLN(F("-- Copy segment constructor --")); memcpy(this, &orig, sizeof(Segment)); name = nullptr; data = nullptr; _dataLen = 0; _t = nullptr; + if (leds && !Segment::_globalLeds) leds = nullptr; if (orig.name) { name = new char[strlen(orig.name)+1]; if (name) strcpy(name, orig.name); } if (orig.data) { if (allocateData(orig._dataLen)) memcpy(data, orig.data, orig._dataLen); } if (orig._t) { _t = new Transition(orig._t->_dur, orig._t->_briT, orig._t->_cctT, orig._t->_colorT); } - if (orig.leds) { leds = (CRGB*)malloc(sizeof(CRGB)*length()); } - DEBUG_PRINTF(" Original data: %p (%d)\n", orig.data, (int)orig._dataLen); - DEBUG_PRINTF(" Constructed data: %p (%d)\n", data, (int)_dataLen); + if (orig.leds && !Segment::_globalLeds) { leds = (CRGB*)malloc(sizeof(CRGB)*length()); if (leds) memcpy(leds, orig.leds, sizeof(CRGB)*length()); } } // move constructor Segment::Segment(Segment &&orig) noexcept { - DEBUG_PRINTLN(F("-- Move constructor --")); + DEBUG_PRINTLN(F("-- Move segment constructor --")); memcpy(this, &orig, sizeof(Segment)); orig.name = nullptr; orig.data = nullptr; @@ -105,27 +104,26 @@ Segment::Segment(Segment &&orig) noexcept { // copy assignment Segment& Segment::operator= (const Segment &orig) { - DEBUG_PRINTLN(F("-- Segment copied --")); + DEBUG_PRINTLN(F("-- Copying segment --")); if (this != &orig) { - if (name) { - DEBUG_PRINTF(" Copy Deleting %s (%p)\n", name, name); - delete[] name; - } - if (_t) delete _t; - if (leds) free(leds); + // clean destination + if (name) delete[] name; + if (_t) delete _t; + if (leds && !Segment::_globalLeds) free(leds); deallocateData(); + // copy source memcpy(this, &orig, sizeof(Segment)); + // erase pointers to allocated data name = nullptr; data = nullptr; _dataLen = 0; _t = nullptr; - leds = nullptr; + if (!Segment::_globalLeds) leds = nullptr; + // copy source data if (orig.name) { name = new char[strlen(orig.name)+1]; if (name) strcpy(name, orig.name); } if (orig.data) { if (allocateData(orig._dataLen)) memcpy(data, orig.data, orig._dataLen); } - if (orig._t) { _t = new Transition(orig._t->_dur, orig._t->_briT, orig._t->_cctT, orig._t->_colorT); } - if (orig.leds) { leds = (CRGB*)malloc(sizeof(CRGB)*length()); if (leds) memcpy(leds, orig.leds, sizeof(CRGB)*length()); } - DEBUG_PRINTF(" Original data: %p (%d)\n", orig.data, (int)orig._dataLen); - DEBUG_PRINTF(" Copied data: %p (%d)\n", data, (int)_dataLen); + if (orig._t) { _t = new Transition(orig._t->_dur, orig._t->_briT, orig._t->_cctT, orig._t->_colorT); } + if (orig.leds && !Segment::_globalLeds) { leds = (CRGB*)malloc(sizeof(CRGB)*length()); if (leds) memcpy(leds, orig.leds, sizeof(CRGB)*length()); } } return *this; } @@ -134,13 +132,10 @@ Segment& Segment::operator= (const Segment &orig) { Segment& Segment::operator= (Segment &&orig) noexcept { DEBUG_PRINTLN(F("-- Moving segment --")); if (this != &orig) { - if (name) { - DEBUG_PRINTF(" Move Deleting %s (%p)\n", name, name); - delete[] name; // free old name - } + if (name) delete[] name; // free old name deallocateData(); // free old runtime data if (_t) delete _t; - if (leds) free(leds); + if (leds && !Segment::_globalLeds) free(leds); memcpy(this, &orig, sizeof(Segment)); orig.name = nullptr; orig.data = nullptr; @@ -166,19 +161,15 @@ bool Segment::allocateData(size_t len) { Segment::addUsedSegmentData(len); _dataLen = len; memset(data, 0, len); - DEBUG_PRINTF("-- Allocated data %p (%d)\n", data, (int)len); return true; } void Segment::deallocateData() { if (!data) return; - DEBUG_PRINTF("-- Deallocating data: %p (%d)\n", data, (int)_dataLen); free(data); - DEBUG_PRINTLN(F("-- Data freed.")); data = nullptr; Segment::addUsedSegmentData(-_dataLen); _dataLen = 0; - DEBUG_PRINTLN(F("-- Dealocated data.")); } /** @@ -189,8 +180,8 @@ void Segment::deallocateData() { * may free that data buffer. */ void Segment::resetIfRequired() { - if (reset) { // (getOption(SEG_OPTION_RESET)) - if (leds) { free(leds); leds = nullptr; DEBUG_PRINTLN(F("Freeing leds.")); } + if (reset) { + if (leds && !Segment::_globalLeds) { free(leds); leds = nullptr; } if (_t) _t->_dur = 0; next_time = 0; step = 0; call = 0; aux0 = 0; aux1 = 0; reset = false; // setOption(SEG_OPTION_RESET, false); @@ -198,6 +189,7 @@ void Segment::resetIfRequired() { } void Segment::setUpLeds() { + // deallocation happens in resetIfRequired() as it is called when segment changes or in destructor if (Segment::_globalLeds) #ifndef WLED_DISABLE_2D leds = &Segment::_globalLeds[start + startY*strip.matrixWidth]; // TODO: remove this hack @@ -209,17 +201,16 @@ void Segment::setUpLeds() { } void Segment::startTransition(uint16_t dur) { - if (transitional || _t) return; // already in transition + if (transitional || _t) return; // already in transition no need to store anything // starting a transition has to occur before change so we get current values 1st - uint8_t _briT = currentBri(getOption(SEG_OPTION_ON) ? opacity : 0); // comment out uint8_t if not using Transition struct - uint8_t _cctT = currentBri(cct, true); // comment out uint8_t if not using Transition struct + uint8_t _briT = currentBri(on ? opacity : 0); + uint8_t _cctT = currentBri(cct, true); CRGBPalette16 _palT; loadPalette(_palT, palette); - uint8_t _modeP = mode; // comment out uint8_t if not using Transition struct - uint32_t _colorT[NUM_COLORS]; // comment out if not using Transition struct + uint8_t _modeP = mode; + uint32_t _colorT[NUM_COLORS]; for (size_t i=0; i_briT = _briT; @@ -227,14 +218,11 @@ void Segment::startTransition(uint16_t dur) { _t->_palT = _palT; _t->_modeP = _modeP; for (size_t i=0; i_colorT[i] = _colorT[i]; - // comment out if using transition struct as it is done in constructor - //_dur = dur; - //_start = millis(); - - setOption(SEG_OPTION_TRANSITIONAL, true); + transitional = true; // setOption(SEG_OPTION_TRANSITIONAL, true); } -uint16_t Segment::progress() { //transition progression between 0-65535 +// transition progression between 0-65535 +uint16_t Segment::progress() { if (!transitional || !_t) return 0xFFFFU; uint32_t timeNow = millis(); if (timeNow - _t->_start > _t->_dur) return 0xFFFFU; @@ -391,7 +379,7 @@ void Segment::setOpacity(uint8_t o) { } void Segment::setOption(uint8_t n, bool val) { - bool prevOn = getOption(SEG_OPTION_ON); + bool prevOn = on; if (fadeTransition && n == SEG_OPTION_ON && val != prevOn) startTransition(strip.getTransition()); // start transition prior to change if (val) options |= 0x01 << n; else options &= ~(0x01 << n); @@ -400,15 +388,15 @@ void Segment::setOption(uint8_t n, bool val) { // 2D matrix uint16_t Segment::virtualWidth() { uint16_t groupLen = groupLength(); - uint16_t vWidth = ((getOption(SEG_OPTION_TRANSPOSED) ? height() : width()) + groupLen - 1) / groupLen; - if (getOption(SEG_OPTION_MIRROR)) vWidth = (vWidth + 1) /2; // divide by 2 if mirror, leave at least a single LED + uint16_t vWidth = ((transpose ? height() : width()) + groupLen - 1) / groupLen; + if (mirror) vWidth = (vWidth + 1) /2; // divide by 2 if mirror, leave at least a single LED return vWidth; } uint16_t Segment::virtualHeight() { uint16_t groupLen = groupLength(); - uint16_t vHeight = ((getOption(SEG_OPTION_TRANSPOSED) ? width() : height()) + groupLen - 1) / groupLen; - if (getOption(SEG_OPTION_MIRROR_Y)) vHeight = (vHeight + 1) /2; // divide by 2 if mirror, leave at least a single LED + uint16_t vHeight = ((transpose ? width() : height()) + groupLen - 1) / groupLen; + if (mirror_y) vHeight = (vHeight + 1) /2; // divide by 2 if mirror, leave at least a single LED return vHeight; } @@ -433,7 +421,7 @@ uint16_t Segment::virtualLength() { #endif uint16_t groupLen = groupLength(); uint16_t vLength = (length() + groupLen - 1) / groupLen; - if (getOption(SEG_OPTION_MIRROR)) vLength = (vLength + 1) /2; // divide by 2 if mirror, leave at least a single LED + if (mirror) vLength = (vLength + 1) /2; // divide by 2 if mirror, leave at least a single LED return vLength; } @@ -473,7 +461,7 @@ void IRAM_ATTR Segment::setPixelColor(int i, uint32_t col) if (leds) leds[i] = col; uint16_t len = length(); - uint8_t _bri_t = currentBri(getOption(SEG_OPTION_ON) ? opacity : 0); + uint8_t _bri_t = currentBri(on ? opacity : 0); if (_bri_t < 255) { byte r = scale8(R(col), _bri_t); byte g = scale8(G(col), _bri_t); @@ -484,8 +472,8 @@ void IRAM_ATTR Segment::setPixelColor(int i, uint32_t col) // expand pixel (taking into account start, grouping, spacing [and offset]) i = i * groupLength(); - if (getOption(SEG_OPTION_REVERSED)) { // is segment reversed? - if (getOption(SEG_OPTION_MIRROR)) { // is segment mirrored? + if (reverse) { // is segment reversed? + if (mirror) { // is segment mirrored? i = (len - 1) / 2 - i; //only need to index half the pixels } else { i = (len - 1) - i; @@ -495,9 +483,9 @@ void IRAM_ATTR Segment::setPixelColor(int i, uint32_t col) // set all the pixels in the group for (int j = 0; j < grouping; j++) { - uint16_t indexSet = i + ((getOption(SEG_OPTION_REVERSED)) ? -j : j); + uint16_t indexSet = i + ((reverse) ? -j : j); if (indexSet >= start && indexSet < stop) { - if (getOption(SEG_OPTION_MIRROR)) { //set the corresponding mirrored pixel + if (mirror) { //set the corresponding mirrored pixel uint16_t indexMir = stop - indexSet + start - 1; indexMir += offset; // offset/phase if (indexMir >= stop) indexMir -= len; // wrap @@ -564,7 +552,7 @@ uint32_t Segment::getPixelColor(uint16_t i) if (leds) return RGBW32(leds[i].r, leds[i].g, leds[i].b, 0); - if (getOption(SEG_OPTION_REVERSED)) i = virtualLength() - i - 1; + if (reverse) i = virtualLength() - i - 1; i *= groupLength(); i += start; /* offset/phase */ @@ -862,12 +850,12 @@ void WS2812FX::finalizeInit(void) } //initialize leds array. TBD: realloc if nr of leds change + if (Segment::_globalLeds) { + purgeSegments(true); + free(Segment::_globalLeds); + Segment::_globalLeds = nullptr; + } if (useLedsArray) { - if (Segment::_globalLeds) { - for (Segment seg : _segments) if (seg.leds) { free(seg.leds); seg.leds = nullptr; } - free(Segment::_globalLeds); - Segment::_globalLeds = nullptr; - } #if defined(ARDUINO_ARCH_ESP32) && defined(WLED_USE_PSRAM) if (psramFound()) Segment::_globalLeds = (CRGB*) ps_malloc(sizeof(CRGB) * _length); @@ -902,7 +890,7 @@ void WS2812FX::service() { doShow = true; uint16_t delay = FRAMETIME; - if (!seg.getOption(SEG_OPTION_FREEZE)) { //only run effect function if not frozen + if (!seg.freeze) { //only run effect function if not frozen _virtualSegmentLength = seg.virtualLength(); _colors_t[0] = seg.currentColor(0, seg.colors[0]); _colors_t[1] = seg.currentColor(1, seg.colors[1]); @@ -914,7 +902,7 @@ void WS2812FX::service() { // effect blending (execute previous effect) // actual code may be a bit more involved as effects have runtime data including allocated memory - //if (getOption(SEG_OPTION_TRANSITIONAL) && seg._modeP) (*_mode[seg._modeP])(progress()); + //if (seg.transitional && seg._modeP) (*_mode[seg._modeP])(progress()); delay = (*_mode[seg.currentMode(seg.mode)])(); if (seg.mode != FX_MODE_HALLOWEEN_EYES) seg.call++; if (seg.transitional && delay > FRAMETIME) delay = FRAMETIME; // foce faster updates during transition @@ -1406,6 +1394,7 @@ void WS2812FX::printSize() DEBUG_PRINTF("Modes: %d*%d=%uB\n", sizeof(mode_ptr), _mode.size(), (_mode.capacity()*sizeof(mode_ptr))); DEBUG_PRINTF("Data: %d*%d=%uB\n", sizeof(const char *), _modeData.size(), (_modeData.capacity()*sizeof(const char *))); DEBUG_PRINTF("Map: %d*%d=%uB\n", sizeof(uint16_t), (int)customMappingSize, customMappingSize*sizeof(uint16_t)); + if (useLedsArray) DEBUG_PRINTF("Buffer: %d*%d=%uB\n", sizeof(CRGB), (int)_length, _length*sizeof(CRGB)); } #endif diff --git a/wled00/data/liveviewws2D.htm b/wled00/data/liveviewws2D.htm index b8dbe337..2f2c518f 100644 --- a/wled00/data/liveviewws2D.htm +++ b/wled00/data/liveviewws2D.htm @@ -23,7 +23,7 @@ } setCanvas(); // Check for canvas support - var ctx = c.getContext('2d'); + var ctx = c.getContext('2d'); if (ctx) { // Access the rendering context // use parent WS or open new var ws; @@ -44,18 +44,18 @@ if (toString.call(e.data) === '[object ArrayBuffer]') { let leds = new Uint8Array(event.data); if (leds[0] != 76 || !ctx) return; //'L', set in ws.cpp - let mW = leds[2]; // matrix width - let mH = leds[3]; // matrix height - let pPL = Math.min(c.width / mW, c.height / mH); // pixels per LED (width of circle) - let lOf = Math.floor((c.width - pPL*mW)/2); //left offeset (to center matrix) - var i = 4; - for (y=0.5;y