[WIP] Arduboy2 Implementation Overhaul

sbmrgd · September 11, 2018, 6:02pm

Just tried both of them. They run indeed very smooth!

But… they look so tiny

Pharap · September 11, 2018, 6:36pm

I’m going to ramble on a bit about the Sprite::drawExternalMask bug and the process I went through to solve it.

I’m going to put it in a details block because I don’t know how many people are actually interested in the ramblings of a madman, and frankly it’s probably a bit boring, but maybe someone will find it useful or interesting.

I guess Arduboy2 porting isn’t as shiny as 3D rendering and emulation…

(Warning: these are really long.)

The Long And Tedious Process

My first port of call was to narrow down the cause of the bug.
I don’t have a hardware debugger yet, so I had to do things the old fashioned way - inserting print statements into the code to print information onto the screen.

This can be very tedious.
First I narrowed it down to gameplay(),
then I narrowed it down to drawObjects()
then I narrowed it down to drawObject(object)
and from there I narrowed it down to Sprites::drawExternalMask.

I tried substituting Spritess:drawOverwrite and SpritesB::drawExternalMask… same result.
I then thought “ah, maybe I broke it during clean up”, so I substituted the original Arduboy2 Sprites code (minus the assembly).
Same problem, which means that for some reason code that works perfectly well on the Arduboy doesn’t work perfectly well on the Pokitto.

Next I tried commenting out drawObject to see if everything else worked.
I discovered drawBackground() had the same issue, coming from the exact same functions.
So I commented out drawBackground() and suddenly the game worked - all the logic was there and working perfectly without that rendering code.

I realised that other parts of the code were still using Sprites::drawExternalMask just fine, so I decided to log the parameters being fed into the drawing function in drawObject (since that was the first breaking point).

At first I thought it was specific sprites causing the problem so I disabled specific object types.
I’d got to the third disabled object when I suddenly spotted a pattern.
Every time it froze, the y input was negative.
So I inserted an if(y > -1) above the drawing call, and sure enough everything started working.

So at this point I’d established that:

The code works fine on Arduboy
There’s something causing it to behave differently on the Pokitto
- That something is related to negative y values

So I moved into the drawing functions and started logging everywhere to find the breaking point, as standard.

I manage to pinpoint the break to one exact line:

uint8_t data = Arduboy2Base::sBuffer[ofs + WIDTH];

And I discovered that actually ofs was surprisingly high (in about the 65000 range).
I know from experience that’s usually a tell-tale sign of negative values being converted to unsigned values.
So I trace the code to ofs's defnition.

uint16_t ofs = ((sRow * WIDTH) + x + xOffset);

I find that sRow is defined:

const int8_t yOffset = (y % 8);
const int8_t tempSRow = (y / 8);
int8_t sRow = ((y < 0) && (yOffset > 0)) ? (tempSRow - 1) : tempSRow;

So evidently this is somehow related.

I start analysing the uint16_t ofs = ((sRow * WIDTH) + x + xOffset); expression and logging its components.
I find that sRow = -1, WIDTH = 128, x = 63 and xOffset = 0, which means (sRow * WIDTH) = -128 and x + xOffset = 63, so obviously -128 + 63 = -65 and of course -65 converted to a uint16_t is 65471.

So then I come back down to uint8_t data = Arduboy2Base::sBuffer[ofs + WIDTH];.
I think “ok, so 65471 + 128 = 65599, and obviously -65 + 128 = 63, which cancels out the -128 from earlier (which was from sRow * WIDTH while sRow = -1)…”
“But…” I ask myself, “this works on Arduboy, so what would cause it to not be working now?”

And then it hit me.
I remembered a basic law of C++ and suddenly everything made sense…

Note

(There was a point where I went down the wrong path and started looking into endianness, but after establishing both devices are little endian I scrapped that idea. A small 15-20 minute red herring.)

The Solution To The Problem

So eventually I had isolated the problem and had a moment of inspiration.

I realised two important bits of information and put 2 and 2 together and arrived at a wonderful 4.

The first thing I realised was the solution to the thing that was bugging me - why would it work on Pokitto and not Arduboy?
The answer has to be something that’s different between the Arduboy and Pokitto, and sure enough it is.

The answer is int.
On the Arduboy an int is 16 bits wide.
On the Pokitto an int is 32 bits` wide.

But if ofs is a uint16_t, then where is this int?
And then I remembered one of the fundamental rules of C++.
Something that happens all the time right under everyone’s noses but it often goes unnoticed.

The answer is ‘integer promotion’.
From cppreference’s section on Numeric Promotions:

unsigned char or unsigned short can be converted to int if it can hold its entire value range, and unsigned int otherwise;
arithmetic operators do not accept types smaller than int as arguments
And on the Pokitto, uint16_t is a type alias for unsigned short, hence ofs + WIDTH (i.e. uint16_t + <integer literal>) causes ofs to be promoted to int.

On the Arduboy, with or without promotion the arithmetic is 16-bit arithmetic, thus overflow occurs, the overflow bits are discarded and the addition correctly cancels out the earlier addition of a negative.

On the Pokitto, the promotion causes 32-bit arithmetic to be used, so instead of the overflow bit being discarded, the overflow bit is kept and the result is larger than the size of the display buffer, thus resulting in a buffer overrun.
It is that buffer overrun that caused the Pokitto to halt.

With this realisation I instantly form a solution to mimic the original behaviour:

uint8_t data = Arduboy2Base::sBuffer[(ofs + WIDTH) & 0xFFFF];

Mask off the higher bits that are dropped in the Arduboy’s original calculations.

To think that the addition of one single mask operation can be the difference between perfectly running code and a processor halting bug.

So what can we learn from this…

The Moral Of This Story

There are many things to take away from this.

Type safety is important. Always know the possible ranges of types.
int is at least 16 bits. It could be 16 bits, it could be 32 bits, it could be 64 bits.
If you want to write portable code, keep that in mind.

Be aware of integer promotion.
It can sneak up on you if you aren’t careful.
Sometimes people think their code means one thing,
but integer promotion changes its meaning and result.

Be careful of mixing signed and unsigned values.
Try not to mix them, but if you must mix them, make sure to double check the behaviour.

Always try to make type conversion explicit.
Hiding type conversion makes code harder to undestand.
If conversions aren’t explicit then sometimes you have to really look to understand what’s going on.

And lastly and most importantly:
Avoid integer overflow like the plague.
If your solution to a problem relies on overflow or underflow then you are probably tackling the problem incorrectly.
Don’t wait for overflow to happen - prevent it!

Pharap · September 11, 2018, 6:43pm

Firstly, compared to the actual Arduboy screen they’re not that much smaller.
An Arduboy screen is about ~2.9x1.9cm.
The Arduboy emulation on Pokitto results in a ~2.3x1.2cm image (so about 6mm either side).

Secondly, they can’t exactly fit the whole screen, unless someone’s prepare to make a scaling algorithm that gives them a width of 1.5 pixels (by mixing black and white to get grey).

Thirdly, I have a magic trick up my sleve for later.
I don’t want to ruin the suprise because it’s really going to be the cherry on top.

At most I’ve told 3-4 people, 2 of whom are actually partly involved in the development.

Jonne (listens to my ramblings and answers the odd question),
Vampirics (told him for a specific reason)
SpaceBruce (I know him in person so I often tell him what I’m working on),
and Filmote (I might not have told Filmote, I can’t remember).

Absolutely nobody else knows.

Pharap · September 11, 2018, 7:11pm

I’ve now properly commited the fix:

And marked this point in history with another release,
which includes the Minesweeper.bin.

Pharap · September 13, 2018, 7:20pm

I couldn’t get the insignificant screen manipulation functions that barely 1% of Arduboy games use working today because of the effort involved in adapting my current code to incorporate the requred changes…

So instead of some boring old screen functions, I guess I’ll have to implement EEPROM so you people can save your highscores and stuff…

So… fully functional LoveRush and Minesweeper anyone? :P

LoveRush.bin (56.9 KB)

Minesweeper.bin (53.7 KB)

jonne · September 13, 2018, 7:36pm

Just a minute, pardner!

I use a lot of time to come up with a 100% safe EEPROM handler and it seems you intend to write directly to the EEPROM.

Why? Have I failed to explain how it works and/or what are the benefits?

giphy

EDIT: this is all toungue-in-cheek ofcourse

EDIT2: I will, ofcourse, help make the safe implementation. But it is essential we don’t go back down to that level where settings get wiped out or something because everyone writes wherever they please in the EEPROM

EDIT3:

It should work something like this.

The Arduboy program reserves a block of EEPROM using the Pokitto::Cookie mechanism.
That block is filled with an array of bytes (for example 256 bytes)
EEPROM write bytes and read bytes are actually wrappers that manipulate individual bytes in that array
everything happens inside a Pokitto::Cookie, allocation tables do not get broken and there is no possibility of overwriting EEPROM saves from other programs, including other Arduboy programs that use EEPROM

p.s.

The Arduboy2 port is a fantastic contribution. Sorry for the hard comment on the EEPROM write/read bits, afterall its only a part of the problem. But I am dead serious. We will never get the EEPROM situation under control if there is no system to prevent accidental overlaps between programs. AFAIK this is a recurring problem on the Arduboy

Pharap · September 13, 2018, 9:16pm

I haven’t forgotten about the ‘cookie’ system. All in good time.
Before I go about implementing the ‘cookie’ system I need to inspect the ‘cookie’ API and decide the best way to apply it.

There’s some other bits and pieces I want to get working first.
Despite my little sidetrack with the timers earlier today
(which was primarily for testing the water of how accurate I can get the screen),
I am intending to focus on implementing the minimum amount of features required to get games running first, and then refine them afterwards.

As far as I’m aware, very few people worry about it.
Most people just accept it and remember to back up their saves.

There is actually a way to communicate with the Arduboy’s bootloader over serial to get a copy of the EEPROM for backup purposes but I’ve never tried it myself.

@Mr.Blinky wrote a quick and easy Python script for it, included among his Arduboy utilities:

There’s a video of it in action here.

I haven’t used it myself because I haven’t needed it.

Also, if you have a dig around my Minesweeper game you’ll find this little gem:

github.com

Pharap/Minesweeper/blob/master/Minesweeper/SaveSystem.h

#pragma once

//
// Copyright (C) 2018 Pharap (@Pharap)
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

#include <stddef.h>
#include <stdint.h>

This file has been truncated. show original

Fully checksummed to detect data corruption,
and both forward and backwards compatible
(provided that new versions of a game only ever add new save fields and don’t remove old ones).

(I was intending to try to get people to adopt it for Arduboy,
but my to do heap is ever-growing.)

And in case it looks like I have no idea what I’m doing,
I assure everyone that I have a plan.

I have a ‘to do’ list (categorised rather than ordered):

To Do

Sprites.cpp

Fix drawPlusMask

ArduboyCore

Implement setCPUSpeed8MHz or mark it as intentionally empty
Implement boolOLED or mark it as intentionally empty
Implement idle or mark it as intentionally empty
Implement bootPowerSaving or mark it as intentionally empty
Implement boot or mark it as intentionally empty
Implement displayOff *
Implement displayOn *
Implement paint8Pixels
Implement sendLCDCommand or mark it as intentionally empty
Implement invert *
Implement allPixelsOn *
Implement flipVertical *
Implement flipHorizontal *
Implement setRGBled variants
Implement freeRGBled
Implement digitalWriteRGB variants
Implement exitToBootloader
Implement mainNoUSB
Implement TXLED1
Implement TXLED0

wiring.cpp

Implement init or mark it as intentionally empty

stdlib.cpp

Implement itoa
Implement utoa
Implement ltoa
Implement ultoa
Implement dtostrf
Implement dtostre

ArduboyAudio

Implement everything

ArduboyBeep

Implement everything

And outside of that I have various notes on paper.
(I prefer paper for notes because I’m always misplacing my .txt files and struggling to think of names for them. A piece of paper doesn’t need a name, just some physical space to occupy.)

Mr.Blinky · September 14, 2018, 8:36pm

Just an idea. How about emulating Arduboy EEPROM by saving and loading to SD card?

Pharap · December 10, 2018, 4:40pm

I hope to get back to working on this if the interest is still there.

Today I implemented itoa, utoa, ltoa and ultoa.
They’re written in a way that’s portable and doesn’t have the bugs that pokItoa, pokUtoa and pokLtoa have (although they might not be as small/fast).

Vampirics · December 10, 2018, 5:00pm

I think there might be interests in thus. But what’s killing it is the fact that the 128*64 res makes it that not many will want to play them that small on the Pokitto…

Pharap · December 10, 2018, 6:00pm

I keep hearing this argument, but if you look at them side by side you realise it’s not actually that much of a difference:

(Sorry these are poor quality, my camera doesn’t like artificial light.)
(Also getting the timers 10 ticks apart was completely unintentional.)

The Pokitto’s screen is actually significantly larger than the Arduboy’s screen,
but the individual pixels aren’t that much smaller.
At most, the Pokitto’s pixels are 2/3 of the Arduboy’s.

In the future it might be possible to scale it up slightly,
but ultimately it’ll come down to the quality of scaling algorithm that can be developed.

Scaling algorithms aren’t easy at the best of times, but most are designed for scales of x2, x3 or x4 and what we’d be looking at here is more like x1.5 or x1.6 (1.75 would be too big unless you’re happy to have some pixels chopped off and the LED indicator to be above or below the gameplay area).

If the Pokitto has enough processing power then scaling up 3x and then scaling down x0.5 might be an option, but I’ll cross that bridge when I come to it.
For now I want to focus on other things, like getting an on-screen LED indicator.

Vampirics · December 10, 2018, 6:27pm

I didn’t mean to say it in a bad way. I know it’s not that much smaller then on the actual Arduboy. But we have to take into account to that since the Pokitto screen is quite bigger, that makes it ‘look’ even smaller.

I just think that some might skip the Arduboy ports because for them, it doesn’t take advantage of the Pokitto.

I am all for Arduboy ports, mainly because it could port most of the game I made with Filmote on the pokitto, but knowing that a minority will really try them makes me want to maybe think twice about it before doing anything.

Now, if someone find a voodoo magic way to scale it well on the Pokitto or make the library adjustable in some ways to help make hybrids pokitto/Arduboy games…

Pharap · December 10, 2018, 7:14pm

I have a solution for that, but I haven’t got to that part yet.

You don’t really have to do much.

As long as an Arduboy game obeys the rules of standard C++ then all you have to do is rename the .ino to .cpp and hook up the library and everything works automatically.

It took me a mere 8 commits (of which 1 was initialising the repo and 1 was uploading the original files) to get CastleBoy into a playable state, and half of that was commenting out stuff I haven’t got round to implementing.

If the library had been in a fully working state then I would have only had to make only one change:
renaming CastleBoy.ino to CastleBoy.cpp.

That’s what this library is going to be capable of when it’s finished.
The point of this library isn’t to make porting Arduboy2 games to the Pokitto easier,
it’s to make Arduboy2 games work for the Pokitto practically out of the box, no extra effort involved.

Unfortunately there will be cases where this won’t work because people have depended too much on the Arduino environment
(i.e. -fpermissive, the behaviour of Arduino’s .ino file processing and GCC’s compiler extensions),
but apart from those cases this library should make direct porting of Arduboy games take practically zero effort.

Like I said, that’s going to be difficult because scaling algorithms aren’t easy.

Scaling algorithms are the kind of thing computer science professors sit around writing really boring mathematical papers about.

Like I said earlier,
I think the best hope for scaling will be to use a x3 scaling algorithm and then downscale to x0.5,
but I’m not even going to think about that until I’ve fixed more of the unimplemented stuff.

If I fixed it tomorrow the games would look better,
but you’d still be left with no LED, no ‘flashlight mode’, no sound,
so you’d still have people saying they don’t want to play it.

Hanski · December 10, 2018, 9:17pm

That is equal than scaling to x1.5 like this( e.g. b/w horizontal pattern):
10101010 ==> 100100100100
I do not think it will look good.
If we do filtered scaling it will decay contrast and eat cpu.

Pharap · December 10, 2018, 10:54pm

It depends which scaling algorithm is used.
I haven’t specified anything.
It could be nearest neighbour, it could be linear, it could be bilinear…

But again, I’m not even thinking about that right now,
scaling is not even close to being the first thing on my todo list.

I’ve got all these functions to implement first:

dtostrf
dtostre
Sprites::drawBitmap (SPRITE_PLUS_MASK)
TXLED0
TXLED1
init()
Arduboy2Base::flashlight
Arduboy2Core::setCPUSpeed8MHz
Arduboy2Core::boot
Arduboy2Core::bootOLED
Arduboy2Core::idle
Arduboy2Core::bootPowerSaving
Arduboy2Core::exitToBootloader
Arduboy2Core::displayOff
Arduboy2Core::displayOn
Arduboy2Core::paint8Pixels
Arduboy2Core::sendLCDCommand
Arduboy2Core::invert
Arduboy2Core::allPixelsOn
Arduboy2Core::flipVertical
Arduboy2Core::flipHorizontal
Arduboy2Core::setRGBled(r, g, b)
Arduboy2Core::setRGBled(colour, value)
Arduboy2Core::freeRGBled
Arduboy2Core::digitalWriteRGB(r, g, b)
Arduboy2Core::digitalWriteRGB(colour, value)
Arduboy2Core::mainNoUSB

And that’s not even counting the sound functions.

drummyfish · December 11, 2018, 4:02am

Don’t forget we have grayscale on Pokitto. Or we can do the interlacing I experimented with in the other thread.

My guess is the 3/2 scale can be done in one step and pretty quickly. I might try to do this once the code is open. I like experimenting with this kind of things.

Things like these could be a compile time option. I’m also thinking about additional options like different colors, or something like a “CRT” look – maybe the bright pixels could have a small “glow” around them. I’ll try to make some mockups when I come home.

going slightly offtopic

Nice postprocessing can do miracles. Old games were made with the CRT look in mind, take a look at that difference:

From this very nice article.

jonne · December 11, 2018, 6:39am

I’ve done the “dumb 1.5x scaling” versions on Pokitto (every second pixel 2 pixels wide).

I don’t think it looks half bad.

But thats just me

Hanski · December 11, 2018, 10:51am

If you can do that fast enough for the full screen, that would be very nice indeed!

Pharap · December 11, 2018, 12:31pm

Technically 2/3rds is already open source* (I haven’t got round to adding avr-libc’s licence because for that one I’ve been working from the documentation, not the actual source code),
but I probably won’t be accepting any PRs until I’m done because later on I’m going to split the repo into three parts so we end up with an avr-libc implementation, an Arduino implementation and an Arduboy2 implementation.

* I didn’t exactly get a say in the matter, I modified the original source code so I have to obey the already existing licences, LGPL for Arduino and MIT for Arduboy2.

Like I say though, I’m not even thinking about the scaling until I’ve got the other stuff out of the way.

Core functionality before anything flashy.

drummyfish · December 11, 2018, 2:19pm

The core libs are LGPL, no? That means you can license your code what you want if it only uses the library. The modified library is still copyleft.

Sure, I don’t want to push it, I would just try it myself and leave the code somewhere as usual. Unfortunately I’ve discovered a new game and am spending most time playing it now, not doing much coding :[