-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Speed improvements for discussion #4138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 15 commits
c3f472f
9341768
feac45f
992d11b
b07658b
09428dc
ec938f2
d45b4ad
2afff05
6a37f25
0e5bd4e
f3137eb
686866c
6962905
a88436c
17d59d3
0a54002
33cf82a
906f8fc
bef1ac2
c44b9f8
b404458
a76a895
7c0fe12
202901b
c842994
9114867
ffbc8c5
336da25
8e78fb4
0ae7329
ee380c5
ba3a61f
a15c391
ca06214
eb5ad23
be64930
210191b
ef1e24c
5c2bac4
0a05611
cae9845
7b9b3f1
3323d2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -712,11 +712,16 @@ void IRAM_ATTR_YN Segment::setPixelColor(int i, uint32_t col) | |
{ | ||
if (!isActive()) return; // not active | ||
#ifndef WLED_DISABLE_2D | ||
int vStrip = i>>16; // hack to allow running on virtual strips (2D segment columns/rows) | ||
int vStrip; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a big fan of leaving locals uninitialized... are you sure that vStrip is never used without first assigning a value? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good call. |
||
#endif | ||
i &= 0xFFFF; | ||
|
||
if (i >= virtualLength() || i<0) return; // if pixel would fall out of segment just exit | ||
if (i >= virtualLength() || i<0) // pixel would fall out of segment, check if this is a virtual strip NOTE: this is almost always false if not virtual strip, saves the calculation on 'standard' call | ||
{ | ||
#ifndef WLED_DISABLE_2D | ||
vStrip = i>>16; // hack to allow running on virtual strips (2D segment columns/rows) | ||
#endif | ||
i &= 0xFFFF; //truncate vstrip index | ||
if (i >= virtualLength() || i<0) return; // if pixel would still fall out of segment just exit | ||
} | ||
|
||
#ifndef WLED_DISABLE_2D | ||
if (is2D()) { | ||
|
@@ -730,7 +735,7 @@ void IRAM_ATTR_YN Segment::setPixelColor(int i, uint32_t col) | |
case M12_pBar: | ||
// expand 1D effect vertically or have it play on virtual strips | ||
if (vStrip>0) setPixelColorXY(vStrip - 1, vH - i - 1, col); | ||
else for (int x = 0; x < vW; x++) setPixelColorXY(x, vH - i - 1, col); | ||
else for (int x = 0; x < vW; x++) setPixelColorXY(x, vH - i - 1, col); | ||
break; | ||
case M12_pArc: | ||
// expand in circular fashion from center | ||
|
@@ -791,7 +796,7 @@ void IRAM_ATTR_YN Segment::setPixelColor(int i, uint32_t col) | |
// Odd rays start further from center if prevRay started at center. | ||
static int prevRay = INT_MIN; // previous ray number | ||
if ((i % 2 == 1) && (i - 1 == prevRay || i + 1 == prevRay)) { | ||
int jump = min(vW/3, vH/3); // can add 2 if using medium pinwheel | ||
int jump = min(vW/3, vH/3); // can add 2 if using medium pinwheel | ||
posx += inc_x * jump; | ||
posy += inc_y * jump; | ||
} | ||
|
@@ -907,8 +912,7 @@ uint32_t IRAM_ATTR_YN Segment::getPixelColor(int i) const | |
if (!isActive()) return 0; // not active | ||
#ifndef WLED_DISABLE_2D | ||
int vStrip = i>>16; | ||
#endif | ||
i &= 0xFFFF; | ||
#endif | ||
|
||
#ifndef WLED_DISABLE_2D | ||
if (is2D()) { | ||
|
@@ -919,7 +923,7 @@ uint32_t IRAM_ATTR_YN Segment::getPixelColor(int i) const | |
return getPixelColorXY(i % vW, i / vW); | ||
break; | ||
case M12_pBar: | ||
if (vStrip>0) return getPixelColorXY(vStrip - 1, vH - i -1); | ||
if (vStrip>0) { i &= 0xFFFF; return getPixelColorXY(vStrip - 1, vH - i -1); } | ||
blazoncek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
else return getPixelColorXY(0, vH - i -1); | ||
break; | ||
case M12_pArc: | ||
|
@@ -1141,8 +1145,8 @@ void Segment::blur(uint8_t blur_amount, bool smear) { | |
uint32_t part = color_fade(cur, seep); | ||
curnew = color_fade(cur, keep); | ||
if (i > 0) { | ||
if (carryover) curnew = color_add(curnew, carryover, true); | ||
uint32_t prev = color_add(lastnew, part, true); | ||
if (carryover) curnew = color_add(curnew, carryover); | ||
uint32_t prev = color_add(lastnew, part); | ||
// optimization: only set pixel if color has changed | ||
if (last != prev) setPixelColor(i - 1, prev); | ||
} else // first pixel | ||
|
@@ -1165,7 +1169,7 @@ uint32_t Segment::color_wheel(uint8_t pos) const { | |
pos = 255 - pos; | ||
if (pos < 85) { | ||
return RGBW32((255 - pos * 3), 0, (pos * 3), w); | ||
} else if(pos < 170) { | ||
} else if (pos < 170) { | ||
pos -= 85; | ||
return RGBW32(0, (pos * 3), (255 - pos * 3), w); | ||
} else { | ||
|
@@ -1184,18 +1188,21 @@ uint32_t Segment::color_wheel(uint8_t pos) const { | |
* @returns Single color from palette | ||
*/ | ||
uint32_t Segment::color_from_palette(uint16_t i, bool mapping, bool wrap, uint8_t mcol, uint8_t pbri) const { | ||
uint32_t color = gamma32(currentColor(mcol)); | ||
|
||
uint32_t color = currentColor(mcol); | ||
// default palette or no RGB support on segment | ||
if ((palette == 0 && mcol < NUM_COLORS) || !_isRGB) return (pbri == 255) ? color : color_fade(color, pbri, true); | ||
if ((palette == 0 && mcol < NUM_COLORS) || !_isRGB) { | ||
color = gamma32(color); | ||
return (pbri == 255) ? color : color_fade(color, pbri, true); | ||
} | ||
|
||
unsigned paletteIndex = i; | ||
if (mapping && virtualLength() > 1) paletteIndex = (i*255)/(virtualLength() -1); | ||
// paletteBlend: 0 - wrap when moving, 1 - always wrap, 2 - never wrap, 3 - none (undefined) | ||
if (!wrap && strip.paletteBlend != 3) paletteIndex = scale8(paletteIndex, 240); //cut off blend at palette "end" | ||
CRGB fastled_col = ColorFromPalette(_currentPalette, paletteIndex, pbri, (strip.paletteBlend == 3)? NOBLEND:LINEARBLEND); // NOTE: paletteBlend should be global | ||
|
||
return RGBW32(fastled_col.r, fastled_col.g, fastled_col.b, W(color)); | ||
return RGBW32(fastled_col.r, fastled_col.g, fastled_col.b, gamma8(W(color))); | ||
softhack007 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
|
||
|
@@ -1877,7 +1884,7 @@ bool WS2812FX::deserializeMap(uint8_t n) { | |
return (customMappingSize > 0); | ||
} | ||
|
||
uint16_t IRAM_ATTR WS2812FX::getMappedPixelIndex(uint16_t index) const { | ||
__attribute__ ((always_inline)) inline uint16_t IRAM_ATTR WS2812FX::getMappedPixelIndex(uint16_t index) const { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non obligatory: I would prefer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I honestly have no Idea what the precompiler instructions mean in detail, I copied this from what they use in fastled... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I've recently learned these are compiler attributes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 not sure if always_inline plays well with IRAM_ATTR .... the first tells the compiler to always inline the function, the latter says "put the function into IRAM" which means that a real function is needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that is what I was wondering too, this is just a suggestion, i.e. to inline this for speed but how to tell the compiler to inline it to the functions that are in ram... not sure how it will do it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To my understanding:
If you want to optimize function calls, its sometime useful to add |
||
// convert logical address to physical | ||
if (index < customMappingSize | ||
&& (realtimeMode == REALTIME_MODE_INACTIVE || realtimeRespectLedMaps)) index = customMappingTable[index]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I overlooked something in the previous discussions - I guess that setting the last parameter to true was done on purpose ... are you sure it does the same thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change is to be approved. It does not do 100% the same thing but it does it faster. I tested this with fire2012 at full blur and the 'overflow' happens only once (i.e. for one pixel) every few frames. There may be situations where this change actually is visible but I could not find one (except the ones I updated with 'smear' functionality in the other PR, smearing looks better without saturation, but this is a new feature so no backwards compatibility required)