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

A spookier version of Halloween Eyes #3501

Merged
merged 7 commits into from
Nov 12, 2023
Merged

Conversation

TripleWhy
Copy link
Contributor

I wanted to use Halloween Eyes for Halloween this year, but when I tried it, it was less spooky than I remembered. ^^
So I tried to write a spookier version. Maybe you like it, too.

Bonus: if (seg.mode != FX_MODE_HALLOWEEN_EYES) in FX_fcn.cpp isn't required anymore.

@blazoncek
Copy link
Collaborator

Thank you. Can you provide a video of the modified effect in action?

@TripleWhy
Copy link
Contributor Author

spookyEyes.webm

The old effect did the following: Display two eyes in a random location, fade them out.

The video isn't great, but mine does this:

  • rapidly fade in two eyes in a random location
  • randomly blink (or not) for very short durations
  • turn off

I also increased the maximum configurable time between eyes showing up and the maximum configurable duration that eyes show up for. For the video, I chose a low time between eyes. In a real setup I'd have a rather large time so that eyes pop up when you don't expect them.

Copy link
Collaborator

@blazoncek blazoncek left a comment

Choose a reason for hiding this comment

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

I see you like to type a lot. 😁
IMO too many static_casts that do not add to code readability, but that is my personal preference.

I would also condense the code wherever possible and add a few more comments.

constexpr uint32_t minimumOnTimeBegin = 1024u;
constexpr uint32_t minimumOnTimeEnd = 1024u;
const uint32_t fadeInAnimationState = elapsedTime * static_cast<uint32_t>(256*8) / duration;
const uint32_t backgroundColor = SEGCOLOR(1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's not gonna work good with overlay. You will need to improve that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A few questions about that:

  • what are overlays?
  • did it work with the original halloween eyes?
  • why doesn't it work?
  • what is the correct approach?

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • Overlays: segments overlapping.
  • in fact it did not as the principle was the same as in your code, use getPixelColor() instead if overlay is enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm. I don't think I get it. I tried looking at some usages of getPixelColor, but it's not really wide spread among other effects.
I tried to experiment with an effect that is simple and uses getPixelColor: Dissolve. But it doesn't even seem to work correctly when using it as a simple effect (without overlays), when the background color is anything other than black. Not even dark gray works.

Long story short: I'm willing to put some more work into this, but I'd need more input. Can you maybe point me to some documentation or to an effect I can study that demonstrates how things are done correctly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately there's no such documentation.
Basically getPixelColor() retrieves the color pixel is set with. When segments overlap it will retrieve pixel color set in underlying effect or if it was set by some other part of current effect. If the pixel was not yet modified it will get a color from previous frame. It works similarly as getting leds[i] when using FastLED.
You need to enable double buffering in LED settings to retrieve unmodified color otherwise the color may be slightly modified. This is due to brightness being applied prior to outputting color to LEDs.

In case of Halloween Eyes, if they are used as overlay over underlying segment you need to fade eyes into underlying color instead of SEGCOLOR(1) otherwise the underlying segment will have "incorrect" pixel when eyes fade out.

wled00/FX.cpp Show resolved Hide resolved
wled00/FX_fcn.cpp Show resolved Hide resolved
@TripleWhy
Copy link
Contributor Author

I see you like to type a lot. 😁 IMO too many static_casts that do not add to code readability, but that is my personal preference.

Yeah you are right, but they are also safer :)
What would you like better? C-style casts? using a regular enum instead of enum class? That would allow for more implicit casting.

@blazoncek
Copy link
Collaborator

Being old-ish, I prefer simplicity of C and first iterations of C++. But I like to learn new tricks nevertheless.
Keep the code as is, except if you can tune it (to be smaller and/or faster).

@blazoncek blazoncek changed the base branch from main to 0_15 November 12, 2023 11:49
@blazoncek blazoncek merged commit 552ae83 into wled-dev:0_15 Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants