Skip to content
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

FX improvements and cleanup #4145

Open
wants to merge 16 commits into
base: 0_15
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions wled00/FX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2204,7 +2204,7 @@ uint16_t mode_colorwaves() {

return FRAMETIME;
}
static const char _data_FX_MODE_COLORWAVES[] PROGMEM = "Colorwaves@!,Hue;!;!";
static const char _data_FX_MODE_COLORWAVES[] PROGMEM = "Colorwaves@!,Hue;!;!;;pal=26";


// colored stripes pulsing at a defined Beats-Per-Minute (BPM)
Expand Down Expand Up @@ -2251,7 +2251,7 @@ uint16_t mode_noise16_1() {

return FRAMETIME;
}
static const char _data_FX_MODE_NOISE16_1[] PROGMEM = "Noise 1@!;!;!";
static const char _data_FX_MODE_NOISE16_1[] PROGMEM = "Noise 1@!;!;!;;pal=20";


uint16_t mode_noise16_2() {
Expand All @@ -2269,7 +2269,7 @@ uint16_t mode_noise16_2() {

return FRAMETIME;
}
static const char _data_FX_MODE_NOISE16_2[] PROGMEM = "Noise 2@!;!;!";
static const char _data_FX_MODE_NOISE16_2[] PROGMEM = "Noise 2@!;!;!;;pal=43";


uint16_t mode_noise16_3() {
Expand All @@ -2290,7 +2290,7 @@ uint16_t mode_noise16_3() {

return FRAMETIME;
}
static const char _data_FX_MODE_NOISE16_3[] PROGMEM = "Noise 3@!;!;!";
static const char _data_FX_MODE_NOISE16_3[] PROGMEM = "Noise 3@!;!;!;;pal=35";


//https://github.com/aykevl/ledstrip-spark/blob/master/ledstrip.ino
Expand All @@ -2302,7 +2302,7 @@ uint16_t mode_noise16_4() {
}
return FRAMETIME;
}
static const char _data_FX_MODE_NOISE16_4[] PROGMEM = "Noise 4@!;!;!";
static const char _data_FX_MODE_NOISE16_4[] PROGMEM = "Noise 4@!;!;!;;pal=26";


//based on https://gist.github.com/kriegsman/5408ecd397744ba0393e
Expand Down Expand Up @@ -2477,7 +2477,7 @@ uint16_t mode_railway() {
SEGENV.step += FRAMETIME;
return FRAMETIME;
}
static const char _data_FX_MODE_RAILWAY[] PROGMEM = "Railway@!,Smoothness;1,2;!";
static const char _data_FX_MODE_RAILWAY[] PROGMEM = "Railway@!,Smoothness;1,2;!;;pal=3";


//Water ripple
Expand Down Expand Up @@ -3234,7 +3234,7 @@ uint16_t mode_glitter()
glitter_base(SEGMENT.intensity, SEGCOLOR(2) ? SEGCOLOR(2) : ULTRAWHITE);
return FRAMETIME;
}
static const char _data_FX_MODE_GLITTER[] PROGMEM = "Glitter@!,!,,,,,Overlay;,,Glitter color;!;;pal=0,m12=0"; //pixels
static const char _data_FX_MODE_GLITTER[] PROGMEM = "Glitter@!,!,,,,,Overlay;,,Glitter color;!;;pal=11,m12=0"; //pixels


//Solid colour background with glitter (can be replaced by Glitter)
Expand Down Expand Up @@ -4144,7 +4144,7 @@ uint16_t mode_sunrise() {

return FRAMETIME;
}
static const char _data_FX_MODE_SUNRISE[] PROGMEM = "Sunrise@Time [min],Width;;!;;sx=60";
static const char _data_FX_MODE_SUNRISE[] PROGMEM = "Sunrise@Time [min],Width;;!;;pal=35,sx=60";


/*
Expand Down Expand Up @@ -7573,7 +7573,7 @@ uint16_t mode_2Dsoap() {

return FRAMETIME;
}
static const char _data_FX_MODE_2DSOAP[] PROGMEM = "Soap@!,Smoothness;;!;2";
static const char _data_FX_MODE_2DSOAP[] PROGMEM = "Soap@!,Smoothness;;!;2;pal=11";


//Idea from https://www.youtube.com/watch?v=HsA-6KIbgto&ab_channel=GreatScott%21
Expand Down
3 changes: 2 additions & 1 deletion wled00/FX.h
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ typedef struct Segment {
uint8_t _reserved : 4;
};
};
uint8_t _default_palette; // palette number that gets assigned to pal0
uint16_t _dataLen;
static uint16_t _usedSegmentData;

Expand Down Expand Up @@ -544,7 +545,7 @@ typedef struct Segment {
void setOpacity(uint8_t o);
void setOption(uint8_t n, bool val);
void setMode(uint8_t fx, bool loadDefaults = false);
void setPalette(uint8_t pal);
void setPalette(uint8_t pal, bool setdefault = false);
uint8_t differs(Segment& b) const;
void refreshLightCapabilities();

Expand Down
24 changes: 8 additions & 16 deletions wled00/FX_fcn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,24 +213,12 @@ CRGBPalette16 &Segment::loadPalette(CRGBPalette16 &targetPalette, uint8_t pal) {
if (pal < 245 && pal > GRADIENT_PALETTE_COUNT+13) pal = 0;
if (pal > 245 && (strip.customPalettes.size() == 0 || 255U-pal > strip.customPalettes.size()-1)) pal = 0; // TODO remove strip dependency by moving customPalettes out of strip
//default palette. Differs depending on effect
if (pal == 0) switch (mode) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unfortunate consequence of this removal is that when used from existing preset, there will be no "default" palette to choose from. So, unless user chooses the effect in UI (with defined "default" palette from metadata) it will not have "default" palette defined. Once user selects it from UI the palette will be defined.

This is then inconsistent behaviour.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default palette gets set when FX is loaded.
I don't fully understand in what scenario the behaviour would differ (with the latest bugfix). Can you check latest commit changes and give me a description of a test case I can try?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take, for example, effect "Fire 2012". Its default platte is "Fire" (35) even if user selects palette "Default" (0).

If I have an existing preset with Fire 2012 it will most likely have ”pal”:0 as its selected palette. But after removal of this code this will mean "Party" palette until user manually selects Fire 2012 from UI which will populate its default palette to 35.

Steps to reproduce:

  • use 0.14 to create preset of Fire 2012 with default values (i.e. palette chosen is Default)
  • update to this PR
  • restart ESP
  • select preset with Fire 2012 effect
  • it will display with Party colors instead of Fire
  • select another preset or effect
  • now select Fire 2012 in Effects tab, palette will change to Fire
  • select another preset or effect
  • select preset with Fire 2012
  • it will display with Fire palette.

Copy link
Collaborator Author

@DedeHai DedeHai Sep 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it thx, I will try to fix this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, fixed it. can confirm your test scenario now works.

case FX_MODE_FIRE_2012 : pal = 35; break; // heat palette
case FX_MODE_COLORWAVES : pal = 26; break; // landscape 33
case FX_MODE_FILLNOISE8 : pal = 9; break; // ocean colors
case FX_MODE_NOISE16_1 : pal = 20; break; // Drywet
case FX_MODE_NOISE16_2 : pal = 43; break; // Blue cyan yellow
case FX_MODE_NOISE16_3 : pal = 35; break; // heat palette
case FX_MODE_NOISE16_4 : pal = 26; break; // landscape 33
case FX_MODE_GLITTER : pal = 11; break; // rainbow colors
case FX_MODE_SUNRISE : pal = 35; break; // heat palette
case FX_MODE_RAILWAY : pal = 3; break; // prim + sec
case FX_MODE_2DSOAP : pal = 11; break; // rainbow colors
}
if (pal == 0) pal = _default_palette; //load default palette set in FX _data, party colors as default
switch (pal) {
case 0: //default palette. Exceptions for specific effects above
targetPalette = PartyColors_p; break;
case 1: //randomly generated palette
targetPalette = _randomPalette; //random palette is generated at intervals in handleRandomPalette()
targetPalette = _randomPalette; //random palette is generated at intervals in handleRandomPalette()
break;
case 2: {//primary color only
CRGB prim = gamma32(colors[0]);
Expand Down Expand Up @@ -593,21 +581,25 @@ void Segment::setMode(uint8_t fx, bool loadDefaults) {
sOpt = extractModeDefaults(fx, "mi"); if (sOpt >= 0) mirror = (bool)sOpt; // NOTE: setting this option is a risky business
sOpt = extractModeDefaults(fx, "rY"); if (sOpt >= 0) reverse_y = (bool)sOpt;
sOpt = extractModeDefaults(fx, "mY"); if (sOpt >= 0) mirror_y = (bool)sOpt; // NOTE: setting this option is a risky business
sOpt = extractModeDefaults(fx, "pal"); if (sOpt >= 0) setPalette(sOpt); //else setPalette(0);
sOpt = extractModeDefaults(fx, "pal"); if (sOpt >= 0) setPalette(sOpt, true); //else setPalette(0);
}
markForReset();
stateChanged = true; // send UDP/WS broadcast
}
}

void Segment::setPalette(uint8_t pal) {
void Segment::setPalette(uint8_t pal, bool setdefault) {
if (pal < 245 && pal > GRADIENT_PALETTE_COUNT+13) pal = 0; // built in palettes
if (pal > 245 && (strip.customPalettes.size() == 0 || 255U-pal > strip.customPalettes.size()-1)) pal = 0; // custom palettes
if (pal != palette) {
if (strip.paletteFade) startTransition(strip.getTransition());
palette = pal;
stateChanged = true; // send UDP/WS broadcast
}
if(setdefault) {
if(pal == 0) pal = 6; //partycolors
_default_palette = pal;
}
}

// 2D matrix
Expand Down