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

Game of Life Memory Leak Fix + Other Changes #133

Merged
merged 22 commits into from
Jun 22, 2024

Conversation

Brandon502
Copy link

Fixed memory leak using new. Removed gameoflife struct and reverted back to using pointers to segenv.data (similar to starLeds version).

Added Blur slider. Blurs dead cells slowly to bgColor depending on slider instead of immediately dying. Doesn't effect game logic.

Removed confusing pauseFrames, uses .step to pause now.

Keeps track of last palette used and immediately recolors cells if new palette/color selected. Can be improved.

Changes gameSpeed slider to be more consistent. Speed = updates/second now in increments of 4.

Removed gameoflife struct.
Blurs dead cells instead of immediately turning off depending on slider value. Blur is disabled when using overlay.
Uses .step now to pause.
Recolors all live cells on palette/color change. Before you needed a game restart or mutation over time.
Every 4 ticks of speed adds 1 update per second. 1 - 64.
@Brandon502
Copy link
Author

Please look over palette change code and game speed.
I couldn't figure out a way to store the actual previous palette used, so it cheats by just using the first color of the palette. This works most of the time. Only doesn't switch immediately if two palettes share same 0 index color (rainbow/rainbow stripe).
Game update speed is capped to 64 updates/second which may be too fast depending on setup. Not sure if this can cause issues so may need adjusting.

@softhack007
Copy link
Collaborator

softhack007 commented Jun 3, 2024

Hi, thanks for this update.
I'll look into the details like game speed in the next days, and then we can merge it into the code for the new release of WledMM (it's still the same "upcoming" release)

I couldn't figure out a way to store the actual previous palette used, so it cheats by just using the first color of the palette

You could use uint8_t currentPal = SEGMENT.palette; to get the current palette ID.
Downside: sometimes a palette is dynamically adjusted, while the ID stays the same. For example: during effect blending, or when using one of the audioreactive palettes. So this would also not make the code 100% bullet proof.

But your "cheat" is OK for me, too. It doesn't need to be a perfect solution.

@Brandon502
Copy link
Author

uint8_t currentPal = SEGMENT.palette;

Some how overlooked this. Didn't think about palettes that change overtime, keeping the first color would work better for those. Each method seems to have a slight drawback, implementing both would be overkill. I'll have to test out those special palettes.

I have a few more changes I've made in StarLeds that I need to convert to WLED and push. Finally figured out a way to make blending look better when a game starts repeating.

Blur now fades while game is paused.
New blur mode when blur slider is >220
Added variables to make code easier to follow instead of check1 custom1 etc. (not needed)
Removed % with random8 calls.
Misc changes
@Brandon502
Copy link
Author

Brandon502 commented Jun 4, 2024

I'll post videos when I get a chance later. Code is a bit more complicated, especially the new redraw loop. Added new blend option when blur slider >220. There is an unintended effect when you have two segments and overlay off. Looks really good on high game speeds, but breaks (flashes) on lower ones. Tried a few ways to get it working on lower speeds but no luck.

Default parameters don't seem to be working. May just be using progmem incorrectly. Edit: It works, just had WLED setting enabled that ignores default.

futureCells is technically not needed to be stored in memory. It is only needed when game updates occur.

@softhack007 I ended going with your suggestion with storing palette index. It looks nicer with palettes that cycle. Only downside is if using single/gradient color wheel options they don't instantly switch. Still works overtime though, so not a huge deal.

@Brandon502
Copy link
Author

Brandon502 commented Jun 5, 2024

Short video showing the new options.

  • Top left: Blur >220. The background keeps a faded color of the last alive cell on that led. Kind of a history/turf war of the alive colors.
  • Top right: High blur value with multiple palette changes to show instant switching and blur fading as the game ends.
  • Bottom left: Unintended effect when you turn off overlay when you have another effect in the background. Alive cells take the background color and you can apply a different blur color. Video shows white then switching to Atlantica.
  • Bottom right: Side effect when game speed is too low, tons of flashing since there are effects stacked. May be able to figure out how to prevent it so there would be a second overlay type without a toggle.

Edit: Fixed flicker issue. Can now swap between overlay types.

GoLBlurOverlay.mp4

Flicker fixed by always redrawing dead cells if overlay unchecked. Can swap between overlay types by checking/unchecking overlayBG.
Fixed overlooked data allocation after switching gliderLength to uint16.
@Brandon502
Copy link
Author

Brandon502 commented Jun 5, 2024

Stress tested on a 64x32 matrix with 4 pins. Showing 30 FPS with max game speed. 40+ when updating is set to 20/sec.

32x64GoL.mp4
32x64GoLOverlay.mp4

Rainbow Background Overlay a bit higher FPS around 33. Opposite "overlay" 24 FPS.

Copy link
Collaborator

@softhack007 softhack007 left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the update.
i've tested your new code quickly, and it works for me.

Below some recommendations that could slightly improve speed.

wled00/FX.cpp Outdated Show resolved Hide resolved
wled00/FX.cpp Outdated Show resolved Hide resolved
wled00/FX.cpp Outdated Show resolved Hide resolved
wled00/FX.cpp Outdated
if (!SEGMENT.check2) SEGMENT.setPixelColorXY(x,y, !SEGMENT.check1?backgroundColor : RGBW32(backgroundColor.r, backgroundColor.g, backgroundColor.b, 0));
setBitValue(futureCells, cIndex, false);
// Blur/turn off dying cells
if (!overlayBG) SEGMENT.setPixelColorXY(x,y, blend(SEGMENT.getPixelColorXY(x,y), bgColor, bgBlendMode ? bgBlur : blur));
Copy link
Collaborator

Choose a reason for hiding this comment

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

optimization: try to use color_blend() instead of blend()

wled00/FX.cpp Outdated Show resolved Hide resolved
@Brandon502
Copy link
Author

Is this a known issue regarding overlays "breaking"?

Normal overlay:

SoapNormal.mp4

Glitched overlay:

SoapGlitched.mp4

Seems like the segments start sharing colors instead of keeping them separate. It allows for some neat effects. Just makes debugging annoying when I don't want it to happen.

GoLGlitchedOverlay.mp4

wled00/FX.cpp Outdated Show resolved Hide resolved
wled00/FX.cpp Outdated Show resolved Hide resolved
@softhack007
Copy link
Collaborator

softhack007 commented Jun 19, 2024

Is this a known issue regarding overlays "breaking"?

Normally not - only two exceptions that I know about

  • "Use global LED buffer" enabled - this makes all pixels shared (plus known bug ghost pixels with "Use global LED buffer" #58)
  • effects run with 1D->2D mapping (pinwheel, bar, etc.) In this case the segment buffer cannot be used by getPixelColor(), so all pixels are read back from the "shared" hardware driver

Remove redundant check
@Brandon502
Copy link
Author

Brandon502 commented Jun 19, 2024

@softhack007 I think I made all your recommended changes. Although I am not sure when to use uint32_t wled_color = uint32_t(fastled_color) & 0x00FFFFFF conversion. Currently just set all CRGB to uint32. Everything seems to work, using color_blend didn't have a noticeable affect on FPS on my end, ~+1 FPS.

There is one last thing I'd like to add to ease transition between games. It doesn't seem to want to cooperate with all options yet though.

Starting alive cells are randomly colored in and bg fades on game start.
Merged generation == 1 checks into standard checks.
@Brandon502
Copy link
Author

Finally got alive cells to spawn in randomly on game start working on everything, but overlayBG. Had a small flicker on start so decided to remove it. Looks a lot better with transition between games. The redraw loop is even more complicated now unfortunately.

Copy link
Collaborator

@softhack007 softhack007 left a comment

Choose a reason for hiding this comment

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

Thanks, looks good 👍

Just tell once you're happy with the code, and i'll merge it into mdev.

@Brandon502
Copy link
Author

Thanks, looks good 👍

Just tell once you're happy with the code, and i'll merge it into mdev.

Sounds good, I'll play around with it some more for a day or two to make sure everything is working normally and no bugs pop up. I'll let you know soon.

@Brandon502
Copy link
Author

@softhack007 Haven't run into any bugs on my setup so far. Feel free to merge whenever.

@softhack007 softhack007 merged commit 6dd8f6e into MoonModules:mdev Jun 22, 2024
34 checks passed
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