pixelmatix / SmartMatrix

SmartMatrix Library for Teensy 3, Teensy 4, and ESP32
http://docs.pixelmatix.com/SmartMatrix
611 stars 161 forks source link

FastLED_Functions example has seam in noise pattern on 32x64 panel #81

Open embedded-creations opened 5 years ago

embedded-creations commented 5 years ago

Should FastLED_Functions example calculate noise based on height/width in a different way?

There seems to be a seam at 32 pixels wide when driving a 32x64 panel

sutaburosu commented 3 years ago

Perhaps it would be useful to describe the issue in hand. The crux of the problem is this line. char noise[][] has the same dimensions as the matrix layout. That line indexes into noise[][] with both [x][y] and [y][x]. Clearly, this will never work for non-square layouts.

I can't fathom whether or not FastLED's noise function is periodic. I was hoping some modulus operation might be an efficient way to deal with rectangular layouts. Instead, I chose to simply grow noise[][] to be large enough for either axis.

Things I've learned since opening this PR:

I see at least 3 possible futures. I'm not experienced enough in C/C++/SmartMatrix to have a worthwhile opinion on style, so it falls to you to choose between them, or something else entirely. Personally, I spray #defines everywhere. I'm aware that this is anathema to others. Regardless of your preference, I'm happy to press the appropriate buttons to get this PR into a mergeable state.

  1. fix the two problematic sketches with const uint, in the style of the current PR.
  2. change all three affected sketches to use const uint.
  3. fix the two problematic sketches with a #define MAX_DIMENSION_OVERALL, to match the style of FastLED+APA102. Initially, I had a similar name, but then I remembered INT_MAX et al, and decided that MAX shouldn't be in the name, hence kMatrixLongestSide.
embedded-creations commented 3 years ago

Thanks for your PR and nice comments about the project!

My preference is 2, I'd be happy with 1 if you don't want to go through the extra effort.