Proposal: Sprites::getWidth and Sprites::getHeight

I was helping someone on the forum when it suddenly occurred to me that perhaps Sprites ought to provide a getWidth and getHeight function for retrieving the width and height of Sprites-format images.

Key thoughts:

The other functions in Sprites rely on images being in this format, so it seems the most sensible place to keep such functions.
Providing designated functions for doing this would allow inexperienced users to read the width and height of images without needing to know about pgm_read_byte - the functions would abstract away the details.

Proposed implementations:

uint8_t Sprites::getWidth(const uint8_t * sprite)
{
	return pgm_read_byte(&sprite[0]);
}

uint8_t Sprites::getHeight(const uint8_t * sprite)
{
	return pgm_read_byte(&sprite[1]);
}

(const uint8_t sprite[] may be preferred to const uint8_t * sprite, depending on which you think is clearer, but to the compiler they're the same thing anyway.)

These two functions could also potentially be used within Sprites::draw to slightly improve clarity compared to the current code:

Arduboy2/src/Sprites.cpp

Lines 47 to 49 in 2374247

    
           uint8_t width = pgm_read_byte(bitmap); 
        
           uint8_t height = pgm_read_byte(++bitmap); 
        
           bitmap++;

Versus the potential:

uint8_t width = getWidth(bitmap); 
uint8_t height = getHeight(bitmap); 
bitmap += 2; // or bitmap = &bitmap[2];

Though this is probably more beneficial for porters than for maintainers.

Looks like a good idea. I'll add this for the next release.

@Pharap - I presume the change to sprites.cpp L47-49, will always inline those get functions. i.e. there is no compiled size penalty(?).

@ace-dent That depends entirely on how strict your definition of 'always' is.

Using the Arduino IDE's default settings targetting an Arduino Leonardo (or Arduboy), I would presume it's highly likely to be inlined in most cases given that pgm_read_byte is likely to produce fewer instructions than a function call would. At best the whole thing will be converted to a single lpm (i.e. a single machine code instruction), at worst the compiler might not be able to inline the function.

I can't guarantee it always will be inlined any more than anyone else can because many different factors can affect what optimisations the compiler chooses to make.

If you're worried about it, run some tests and see what results you get.
E.g.

Try with the [[gnu::always_inline]] attribute applied
Try with the [[gnu::noinline]] attribute applied
Try with no attribute applied
Try each of the above tests with 1, 2, 3 and 4 separate calls to the function
- Note: 3 calls is around the point the compiler usually stops inlining for larger functions, hence 2 calls can often produce more code than 3 calls if the function body is larger than the cost of a function call

	uint8_t width = pgm_read_byte(bitmap);
	uint8_t height = pgm_read_byte(++bitmap);
	bitmap++;