Passing backgroundLayer in a function declaration / why does swapBuffers default to copy=true?

marcmerlin · December 31, 2018, 1:36pm

I’m struggling to integrate SmartMatrix with Adafruit GFX because my object needs to receive backgroundLayer as an argument, and it seems to be a complex template thing (not a fan of C++ templates, they make life such a pain, or maybe I’m just not smart enough, or both ).

I can’t send it as an argument without a type (with FastLED, it was simple, it was simply
SmartMatrix_GFX::SmartMatrix_GFX(CRGB *leds, uint8_t w, uint8_t h):

Anyway, I found:
SmartMatrix3<pwm_depth, width, height, panel_type, option_flags> matrix_name
If I read this right, depth, size, type and options are all template arguments which means they need to be constant values at compile time. In turn, it seems impossible to pass backgroundlayer because I can’t define it in a library which does not know those arguments which can’t be defined at calling time but must be known at compile time, so I can’t define the type in the function signature.

Am I correct, is my effort more or less doomed?

For more context, this is what I’m trying to do

github.com

marcmerlin/FastLED_NeoMatrix/blob/master/FastLED_NeoMatrix.cpp#L57


#endif
#endif


#ifndef _swap_uint16_t
#define _swap_uint16_t(a, b) { uint16_t t = a; a = b; b = t; }
#endif






// Constructor for single matrix:
FastLED_NeoMatrix::FastLED_NeoMatrix(CRGB *leds, uint8_t w, uint8_t h, 
   uint8_t matrixType): 
 Adafruit_GFX(w, h),
 type(matrixType), matrixWidth(w), matrixHeight(h), tilesX(0), tilesY(0), 
 remapFn(NULL) { 
   _leds = leds;
   // WARNING: Serial.print seems to crash in the constructor, 
   // but works in begin()
   numpix = matrixWidth * matrixHeight;
 }

(as written for FastLED)

Also, I’m not super clear about how the base layer, SMARTMATRIX_ALLOCATE_BUFFERS(matrix …
is used and whether I need it if I only want/need a single dumb layer. Is it required because you write to backgroundlayer and then swapbuffers copies the contents to matrix which is used to run the actual display?

Thanks.

marcmerlin · December 31, 2018, 3:17pm

Actually, assuming this isn’t really possible, I’m now thinking I’m going to have an easier path leaving all my code to render into a fastled buffer (i.e. an array of CRBG) because some of that code I’d like to port runs stuff like leds[YX(x,y)] += 3, and copy the result into pixelmatrix pixel by pixel at the time show() is run.

Update: this is what I ended up doing:

github.com

marcmerlin/SmartMatrix_GFX/blob/master/SmartMatrix_GFX.cpp#L58


#endif


#ifndef _swap_uint16_t
#define _swap_uint16_t(a, b) { uint16_t t = a; a = b; b = t; }
#endif




#include "FastLED.h"


//SmartMatrix_GFX::SmartMatrix_GFX(RGB888 *leds, uint8_t w, uint8_t h): 
SmartMatrix_GFX::SmartMatrix_GFX(CRGB *leds, uint8_t w, uint8_t h, void (* showptr)()): 
Adafruit_GFX(w, h),
matrixWidth(w), matrixHeight(h), type(0), tilesX(0), tilesY(0), remapFn(NULL){ 
  _leds = leds;
  _show = showptr;
  // WARNING: Serial.print seems to crash in the constructor, 
  // but works in begin()
  numpix = matrixWidth * matrixHeight;
}


void SmartMatrix_GFX::begin() {

Can I get rid of background layer, backgroundLayer.swapBuffers(), and somehow directly copy into the real buffer used for output?
Or I guess I can just call backgroundLayer.swapBuffers(false) and it will be zero copy, so just as good.

I guess I’m just not too clear on why swapbuffers actually defaults to copying (which isn’t a swap). Given that there are 2 buffers and they’re being swapped anyway (i…e you’re not writing on the swapped one that is used for current display), why is copying better and default?
That said, it’s not really slower or faster with copy true or false, backgroundLayer.swapBuffers() is a slow 76fps (4096 pixels) either way for me.

marcmerlin · January 2, 2019, 7:09am

I just re-ran the same test on teensy 3.6, and it was a lot faster: 228 fps
So, there is definitely an issue with swapBuffers speed on ESP32

marcmerlin · January 3, 2019, 3:10pm

@Louis, with more reading, I now found
rgb24 *buffer = backgroundLayer.backBuffer();
in https://github.com/pixelmatix/SmartMatrix/blob/master/MIGRATION.md

However, reading the rgb24 definition in MatrixCommon.h, it seems to be missing many of the operators/methods defined in FastLED’s CRGB:

github.com

FastLED/FastLED/blob/master/pixeltypes.h#L90


HUE_ORANGE = 32,
HUE_YELLOW = 64,
HUE_GREEN = 96,
HUE_AQUA = 128,
HUE_BLUE = 160,
HUE_PURPLE = 192,
HUE_PINK = 224
} HSVHue;


/// Representation of an RGB pixel (Red, Green, Blue)
struct CRGB {
	union {
		struct {
        union {
            uint8_t r;
            uint8_t red;
        };
        union {
            uint8_t g;
            uint8_t green;
        };

I think that as a result, rgb24 is not quite compatible (as in a dropin replacement) for CRGB for FastLED code that addresses an array of CRGBs.
For instance, you point out
no known conversion for argument 1 from 'CHSV' to 'const rgb24&'
Would you consider extending rgb24 and making it compatible with FastLED’s CRGB (including all its methods).
I think if you do, it would make SmartMatrix more of a drop in replacement for FastLED code, and in the case of SmartMatrix::GFX I think I could drop the intermediate array of CRGB and later copy to backBuffer, and point to backBuffer directly.

In the meantime, though, it seems that instead of looping through my array of CRGB to copy it to backgroundLayer, I should be able to do a direct memcpy between the 2 arrays, which will be a bit faster than my loop.

Thoughts? Corrections?

Louis · January 3, 2019, 8:54pm

Can you use a cast of the rgb24 to CRGB to accomplish the same thing?

(That’s not a rhetorical question, I’m not sure of the answer and don’t know if it will work)

marcmerlin · January 4, 2019, 7:11am

I think cast will work for some operations since the underlying data types are compatible I believe, but it won’t work for FastLED operations that use all the extra methods defined in FastLED’s CRGB.

Look around https://github.com/FastLED/FastLED/blob/master/pixeltypes.h#L138
I haven’t checked how much code is compatible or not as a result.
If I have extra time (might not, between 2 trips and just found out I now have to write a talk for linux.conf.au before flying to NZ in 4 days), I’ll try to see if FastLED code would just work with your backend without the virtual framebuffer I currently use, but I may not get around to it.

Louis · January 4, 2019, 5:10pm

I’m very unlikely to include all the CRGB operations inside SmartMatrix Library, as SmartMatrix Library was dependent on FastLED in the past and this caused some problems.

I’m sorry I don’t have time to dive into this issue and try to help you get your integration working right now

marcmerlin · January 4, 2019, 11:03pm

Not to worry, I worked around it by creating a fastled framebuffer. It’s not efficient memory-wise, but it works.
If you can just give me a hint on
backgroundLayer.swapBuffers(false)
vs
backgroundLayer.swapBuffers(true)
Why does it default to true and why would someone want true given that you’re already not writing in the buffer that’s currently being displayed.

Also, I’m only getting 76fps on swapbuffer on ESP32 (it’s faster on teensy). Any idea why? Is it faster for you?

Louis · January 6, 2019, 11:50pm

copy = true copies what’s was the draw buffer to the new draw buffer after the swap. If you drew a shape to the buffer, then swapped buffers to display the shape on the screen, you might expect the draw buffer to also contain that shape. copy = true takes care of that, and its default is true because of that expectation. It’s faster to do the swap without copying (and waiting for the swap to happen so you can start copying), so copy = false if should be more efficient, and is just fine for something like playing GIFs or a FastLED pattern that fills the screen each time with new pixels.

Take a look at the README for setCalcRefreshRateDivider() and also note that there’s a getCalcRefreshRateDivider() method. The default is 2. If your sketch is hogging the CPU and the SmartMatrix calculation background task can’t keep up, it may automatically increase the refresh rate divider further

marcmerlin · January 7, 2019, 12:43am

Aaah, I understand what copy does not, thank you.
Because I copy the entire virtualfb into your FB every time anyway, copy is useless for my case, but I see why it would be useful by default otherwise, thanks for explaining.

For the FPS, I did read the README warning, but I changed my speed to 240Hz to lessen recording issues, so I was expecting faster even with the divider. I’ll do more tests when I get a chance to see what’s going on.

I also misunderstood that you were using one core for display and one core for normal code, but given that you are using DMA and not big banging anyway, it’s probably not necessary.

Thanks for the replies.
Marc

Louis · January 7, 2019, 12:58am

There’s a lot of work done in turning the buffer into the bits to go out to the panel, so taking advantage of the multiple cores would be nice, but I also don’t know the side effects of using the other core. Would be nice to explore that someday maybe.

marcmerlin · January 20, 2019, 10:00am

@Louis took me a while to get back to it (giving my first talk at linux.conf.au on led matrices tomorrow), but as we guessed, doing a memcpy between fastled CRGB and backbuffer, works fine.

With this change, SmartMatrix::GFX should be almost as fast as native SmartMatrix since there is a single copy and I can run swapbuffers(false) to avoid the other copy.
diff: https://github.com/marcmerlin/FastLED_NeoMatrix_SmartMatrix_LEDMatrix_GFX_Demos/commit/edcaf23c3960895cc8d81307335cf1da4cfd5c0a

The only downside left of using my API is the double memory use, but since I only use a single layer in SmartMatrix, it’s not so bad.

Anyway, I’ve solved all the issues I had in this thread, so let’s call it closed. Thanks for your help again.