'drawBitmap' Frames


#1

Can someone remind me how drawBitmap expects frames to be laid out?

I keep trying to do:

constexpr std::uint8_t imageName[]
{
	// Width, Height
	8, 8,
	
	// Frame 0 data...
	0x00, 0x00, 0x00, 0x00,
	
	// Frame 1 data...
	0x00, 0x00, 0x00, 0x00,
};

I vaguely remember there being at least a 3rd ‘parameter’ at the start, but I can’t remember what.

(I also remember having a conversation about how said parameter probably isn’t actually necessary, but I can’t remember the details.)


#2

Using 2d array or adding a offset to array, Need to add width and height for every frame


#3

I know I can just keep an array of pointers to image,
but I’m specifically asking about one particular overload of Pokitto::Display::drawBitmap.

Mainly because it would be easier to tweak my image generator tool to output the correct frame format than it would be to make it create arrays of pointers to images.

Besides which, it would be nice to have this documented,
the doxygen documentation makes no mention of it.


#4

i think this will explain every thing

void Display::drawBitmap(int16_t x, int16_t y, const uint8_t* bitmap, uint8_t frame)
{
    int16_t w = *bitmap;
	int16_t h = *(bitmap + 1);
	uint8_t framew = *(bitmap+2);
    bitmap = bitmap + 3; //add an offset to the pointer to start after the width and height

#5

That explains everything but what framew actually means.
Is it just a duplicate of the frame width?


#6

frame size in bytes if you are using 16 colors it is width*height/2.


#7

Doc said its:

ONLY 4-bit mode for time being


#8

Ok, thank you, that makes sense.
So more specifically it’s ((frameWidth * frameHeight) / (8 / bitsPerPixel)).

And as I remembered, it’s completely unnecessary because the code could easily calculate that.

There’s definitely code for 1bpp and 2bpp modes:

Unless that code doesn’t actually do what it says on the tin,
in which case it really has no business being there and should probably be removed.


#9

It seems that this doesn’t work…

firmware.bin (40.3 KB)

I’m guessing frame drawing is broken?


Directions for use:

There is a cursor, but it’s a bit hard to spot because the rectangle drawing function doesn’t work correctly.

The arrow keys move the cursor.
A and B increment and decrement the frame index.

It should make each tile cycle through the various frames of the image,
but instead it seems to be scrolling a single 16 pixel row each time.
And even then the tiles don’t seem to be drawing correctly.

Here are the tiles for reference:

GreyRoomTiles


#10

i don’t know what’s the problem, but you can use drawBitmapData function

Display::drawBitmapData(drawX, drawY, Images::tiles[0], Images::tiles[1], Images::tiles+3+(index*Images::tiles[2]));

#11

Whats the use of this, why not use a 2D array?


#12

there is a problem with the drawRect function, it draw one pixel more then it should.

here what i have change to make it work

Display::drawBitmapData(drawX, drawY, Images::tiles[0], Images::tiles[1],Images::tiles+3+index*Images::tiles[2]);
            
Display::setColor(0);
if((this->selectorX == x) && (this->selectorY == y))
     Display::drawRect(drawX, drawY, tileWidth-1, tileHeight-1);

2


#13

A 2D array wouldn’t allow the width and height of the frames to be ‘baked’ into the image data so they’d have to be passed to drawBitmap which limits the code’s flexibility slightly.

(Though admittedly it would possibly work out slightly cheaper than the array of pointers approach depending on the circumstances.)


Yeah. I’m sure somone (I think it was @sbmrgd?) was supposed to have made a PR to fix that at some point, but maybe I’m misremembering?

Maybe it got talked about but never happened?
Or maybe it’s fixed in PokittoLib and not PokittoIO?

The .gif is a bit overkill, I would have trusted the code in your first comment. :P
(Though I prefer &Images::tiles[3 + (index * imageWidth)], and I’d wrap it in a function to be safe.)

It somewhat baffles me why the function isn’t implemented like this.
I could imagine that there might be faster ways, but working is better than not working.
I wonder if this works for multiple modes/bpps…


I’m tempted to suggest changing the function to use this as a solution, but I don’t currently have the time to dedicate to properly testing it for other screen modes.

Plus it would mean getting rid of that third ‘parameter’, which I’d really like to do, but that sort of change warrants some discussion about the possible implications of such a change.

If the function’s always been broken then it’s no big deal,
but if the function does actually work somehow and there’s a game using it somewhere then that game’s going to break.


#14

Why not use the regular draw function and separate out width and height and data.
BaseDrawBitmap(x,y, width, height, data)
Then your frame funtion can pull these apart and use an pointer offset, would even add vertical scrolling if wanted.
Seems the documentations out of date and the library hacked together.


#15

That’s precisely what the Display::drawBitmapData function @bl_ackrain mentioned is.

The documentation has always been a bit of a problem.
Writing documentation is a lot of effort, it’s really boring and its rare for people to get recognition for it because people tend to take good documentation for granted, so it’s fallen to the wayside.

There have been a few discussions of making a reboot of the library but nobody’s got round to it.

I actually wrote a prototype myself a while back but I didn’t get it to the point where it was actually working.
I was mainly focusing on the API, how it should look and what it should include.


#16

Sorry dint really understand the code, so would be simpler to wrap that in a function and reduce redundant code?
Is there any bounty on the API or documentation rewrite?


#17

Which part? I’ll have a go at explaining if you want.

Yep, which is what I have done.

I used @bl_ackrain’s idea as a base and then rewrote it to do away with that pesky third parameter.

Et voila:

#pragma once

#include <cstdint>
#include <cstddef>

#if !defined(DISABLEAVRMIN)
#define DISABLEAVRMIN
#endif

#include <Pokitto.h>

namespace DisplayUtils
{
	inline void drawBitmap(std::int16_t x, std::int16_t y, const std::uint8_t * bitmap, std::uint8_t frameIndex)
	{
		using Pokitto::Display;

		constexpr std::size_t widthIndex = 0;
		constexpr std::size_t heightIndex = 1;
		constexpr std::size_t dataIndex = 2;
		constexpr std::size_t bitsPerByte = 8;

		const std::size_t width = bitmap[widthIndex];
		const std::size_t height = bitmap[heightIndex];
		const std::size_t frameSize = ((width * height) / (bitsPerByte / Display::m_colordepth));

		Display::drawBitmapData(x, y, width, height, &bitmap[dataIndex + (frameIndex * frameSize)]);
	}
}

Which accepts data in the format I mentioned at the start of the thread:

constexpr std::uint8_t imageName[]
{
	// Width, Height
	8, 8,
	
	// Frame 0 data...
	0x00, 0x00, 0x00, 0x00,
	
	// Frame 1 data...
	0x00, 0x00, 0x00, 0x00,
};

(Just like the Arduboy’s format. But you know, not monochrome.)

What sort of ‘bounty’?

As far as I’m aware there isn’t even a badge for it.


#18

To be more precise I opened an issue for fillRectangle


#19

Please add this as documentation comments in the file. you might forget or someone 12 months later will be confused and would have to go digging thru this forum, is exhausting…

Unsure, my pokitto’s speakers is not working and the usb plug fell out. Any compensation would be nice…


#20

Ah, thank you, I knew it was something along those lines.

If the issue’s still open maybe I (or someone else) ought to dive in and fix it some time?


Part of the reason I’m reluctant to make changes to the library is because there’s no consensus about style and I have a habit of drastically redecorating code when I edit it and people don’t always take too kindly to it.

For example I favour tabs and allman style over spaces and K&R style,
and I have a laundry list of style rules that I try to adhere to.

Such as
  • Always leave a space around binary operators
    • It makes them more obvious and more readable
  • Always use explicit round brackets on subexpressions
    • To make the order of evaluation (partially) unambigous
  • Declare a variable at the latest possible moment
    • Means there’s less variables to think about at once
    • Often results in smaller code for various reasons
  • Try to keep variables immutable where possible
    • Makes the code easier to follow
    • Sometimes produces smaller code
  • Never use C-style casting, always use C++-style casting
    • C-style casting is more likely to introduce bugs
    • C-style casting is harder to search for
    • C++-style casting makes dangerous operations more obvious

Et cetera et cetera…


Which file?

I do try to comment my code anyway.

Or at least the code I publish and intend for people to read and learn from.
For example in Physix.

Report that to @jonne immediately.
That’s definitely not supposed to happen.