Adafruit-GFX-Library icon indicating copy to clipboard operation
Adafruit-GFX-Library copied to clipboard

Create a class that doesn't subclass Print

Open joshgoebel opened this issue 10 years ago • 15 comments

Could we please get a class to use that doesn't subclass Print.

class Adafruit_GFXWithoutPrint
{
// all the graphics stuff
}

class Adafruit_GFX: public Print, public Adafruit_GFXWithoutPrint 
{
// printing stuff
}

Obviously the names could change. Just subclassing Print gets you a whole dependency chain that most people don't realize:

  • write() - it's virtual, so it always gets linked via the vtable, even if you never print a single thing
  • drawChar() - called from write
  • font - used by drawChar
  • drawRect - used by drawChar (for x2+ size fonts)
  • drawFastHLine - used by drawRect
  • drawFastVLine - used by drawRect
  • drawLine - used by drawFast*Line
  • swap - used by drawLine
  • drawPixel - used by drawLine

A program that is only moving sprites around the screen might not ever be calling ANY of those methods (assuming a hardware optimized drawImage) - yet they are now linked in and easily taking up 2kb or more. Even if using the built in drawImage for most of your graphics the only thing from above you really need is drawPixel, but instead you get everything plus the kitchen sink.

I don't think most people understand or expect this behavior - even those that already understand the compiler doesn't link in what your program never calls. I had to get help from someone who knew a lot more C++ than me to find out that the virtual function table of the subclass is what is holding a pointer to write() and that's why the compiler can't remove it like it can for most functions that aren't used.

Thoughts?

joshgoebel avatar Jun 19 '15 01:06 joshgoebel

This is one of the issues we're having with ambitious programs on the Arduboy (https://www.kickstarter.com/projects/903888394/arduboy-card-sized-gaming). You run out of flash quick (code, sprites, images, etc)... so we may end up providing a graphics mode that doesn't pull in Print at all. I even think we should consider just subclassing AdafruitGFX for all the great work you guys have done - but obviously we can't do that if one of the requirements for us is to drop the Print requirement.

joshgoebel avatar Jun 19 '15 02:06 joshgoebel

I'm fine doing this work and making a PR, I just want to know the work is blessed so I'm not wasting my time.

joshgoebel avatar Jun 19 '15 02:06 joshgoebel

Moved my unrelated comments into #51.

joshgoebel avatar Jun 19 '15 03:06 joshgoebel

Yeah this is a real pain point with C++ & Arduino's use of inheritance. We have a similar issue with the CC3000 library as it subclasses Client so it's easy to use with Ethernet library code, but as a result it pulls in a lot of code to call all the subclass constructors, destructors, etc.

One thought I had but haven't had time to look at is making libraries like this and the CC3k library let you set a #define in the header which turns off inheritance. For example flip a #define in the Adafruit_GFX.h file and have an #ifdef block around the class declaration choose whether to define the class with Print as an parent, or without any parents. That seems like a good compromise as it lets people who have the space use the nice print functions, but still gives an easy way to turn off that behavior and save the space (at the expense of all the print functionality).

Doing something with multiple inheritance like you mention might work too, but I worry that will add to the problem since the extra parent class has to have its constructors called, etc. With macro #ifdef block to turn off inheriting from Print it won't add any overhead to enable or disable the functionality.

Does adding some #ifdef logic to the class declaration to choose if Print is a parent sound like it would help your issue with saving space? If so feel free to send a pull--we can definitely integrate it. Thanks!

tdicola avatar Jun 19 '15 19:06 tdicola

but I worry that will add to the problem since the extra parent class has to have its constructors called, etc.

Don't understand this concern. C++ makes this pretty easy unless you're just worried about the overhead of a constructor chaining another? I'm not sure there is any overhead if you just make two classes and then compose them together (inherit from both). Then you'd just duplicate the constructor for the two main parent classes.

Does adding some #ifdef logic to the class declaration to choose if Print is a parent sound like it would help your issue with saving space?

I'm struggling with this approach. It requires everyone vendor your library and hack their own changes onto it for every single app they build. Vs installing it as a library and then using it in a sketch. If I edit the defines then one project compiles and another breaks - and now I'm back to manually copying this library into every project that uses it - and changing it. Doing that makes it harder to stream in improvements and bug fixes over time - losing a lot of the benefit of having a library in the first place.

joshgoebel avatar Jun 19 '15 19:06 joshgoebel

Yeah I gave it a quick test and there's a tiny bit of overhead. If you want to try, check out this branch https://github.com/adafruit/Adafruit-GFX-Library/tree/print_refactor where I added a Adafruit_GFX_Core class that just has the drawing functions, and then the normal Adafruit_GFX class that uses multiple inheritance with the core and print. Compiling a SSD1306 OLED example before the change (for an Uno, using Arduino 1.6.4):

Sketch uses 20,206 bytes (62%) of program storage space.

And after the change to use multiple inheritance:

Sketch uses 20,222 bytes (62%) of program storage space.

So a small increase of ~15 bytes. That's not too crazy IMHO, but if you're looking for the most space savings possible it's a trade-off of flexibility (no need to change #defines) vs. a little extra space. Let me talk to folks more but I'm open to either approach since the overhead is so small for most users.

Overall though you do want to be careful of the target and priorities for the GFX library. It's meant to be general and easy to use and maintain which sometimes comes at the cost of performance and code size. If you're optimizing for the absolute fastest speed, lowest code size, etc. at some point you'll hit a wall where you need to give up flexibility, etc. to see more improvements.

tdicola avatar Jun 19 '15 20:06 tdicola

Overall though you do want to be careful of the target and priorities for the GFX library. It's meant to be general and easy to use and maintain which sometimes comes at the cost of performance and code size. If you're optimizing for the absolute fastest speed, lowest code size, etc. at some point you'll hit a wall where you need to give up flexibility, etc. to see more improvements.

Sure. I've been talking to the TeamARG people and they don't use printing or fonts any any of their games.. it's all sprites and images... and that actually doesn't seem all too unreasonable for a lot of those types of games... if the cost was 50 bytes or 150 bytes that might be one thing but linking in 2-3k of things that aren't needed just because of a dependency on Print crosses over the line for me. We're already down 4k from boot-loader and another 4-5kb from the silly USB stuff. We need all the help we can get over here. :-)

So a small increase of ~15 bytes.

I'll take a look and see if that can be fixed. Did you look to see where the increase was coming from?

joshgoebel avatar Jun 19 '15 21:06 joshgoebel

Can you provide a link to the sketch you're using for testing? It's probably just the chain of constructors though now that I think about it. I think you could avoid that if you just duplicated the init in two parents class from an abstract child class that had no constructor of it's own. (that's a bit weirder, but might get the bytes back - since someone is going to be using one class or the other not both)

Might also be some way to force the call to be inlined, but I'm not sure.

joshgoebel avatar Jun 19 '15 21:06 joshgoebel

Yeah IIRC I was using the stock ssd1306_128x32_i2c example. The overhead should be from the extra parent class and the need to call its constructor. I don't think you'll really find a way around this though since the compiler needs to call the constructor to ensure the base class state is initialized.

If the extra 15 bytes or so are an issue then #ifdef'ing out the Print class is the way to go. Yep it's extra work for people who need both versions, but like I said before it comes down to a trade-off of size vs. flexibility.

tdicola avatar Jun 19 '15 22:06 tdicola

Frankly, if you're only using the sprite functions, and if this is purpose-built for gaming, I'd ditch GFX altogether and gut the SSD1306 library for its screen init and refresh code, then do all the sprite-masking code directly into the screen buffer. Nearly did this for the Flying Toaster Pendant tutorial, which was already bogging down with just a few sprites.

GFX is designed mainly to get new display hardware up and running with minimal effort...subclasses at the very least just need initialization code and a pixel-drawing function, then all other primitives can build on that. Easy, but not particularly efficient...for example, clipping is performed on every pixel. Doing those operations on higher-level primitives and using byte-wide operations for sprite drawing is likely better than an order of magnitude faster, and the total code a fraction of the GFX+SSD1306 combo (especially without all that font data).

This would also address your other question about packaging multiple libraries, since it'd be replaced with a single thing.

PaintYourDragon avatar Jun 19 '15 22:06 PaintYourDragon

@PaintYourDragon Yeah, it's just crappy how for the most part the linker can remove unnecessary functions but the it all falls apart for virtual. There is no reason a nice graphics library couldn't shouldn't include primitives... but if all I use is the fast drawSprite, that's all I expect to get linked... but this virtual mess defies that expectation. Now I have to start making conscious decisions without relying on the linker... I consider myself pretty sharp and this drives me nuts. I hate to think of how a newb would try to understand this. Just when you teach them if it's not used it isn't include - EXCEPT for all the virtual nonsense.

It would be nice if we could just offer a single graphics lib with lots of functionality and the compiler removed what wasn't needed when linking - and instead it looks like we have to have multiple sets of classes just because of vtables.

joshgoebel avatar Jun 19 '15 22:06 joshgoebel

A lot of our graphics primitives are just copies from GFX, so I was trying to find some way to easily reuse that code (without having to copy it and then maintain it all ourselves).

joshgoebel avatar Jun 19 '15 22:06 joshgoebel

How about using a more modern technique than class inheritance? Correctly designed templates are very powerful at removing phantom vtable memory use; while more advanced to design and code, the user of them by users is often just as easy.

Just grab this book and read (https://www.amazon.com/Modern-Design-Generic-Programming-Patterns/dp/0201704315)

Makuna avatar Apr 03 '17 06:04 Makuna

I think it is about time to resurrect this issue. I would also like to see a better design not using virtual functions. This adds about 50 bytes of ram usage per class for the vtable. adding up to ~150 bytes vtables which is close to 13% of arduino uno ram just wasted. As stated by others this also adds dead waste on other places and slows down execution due to the vtable lookup.

DEvil0000 avatar Aug 29 '22 18:08 DEvil0000

I found a okay solution for me by putting vtables in programm memory instead of ram with help of this gcc plugin. This of course is no optimal solution but a improvement on the ram front.

DEvil0000 avatar Sep 02 '22 13:09 DEvil0000