Small Screen Rotation Library for Joyhat

Okay here’s my best idea:

Write a small lib that subclasses the Pokitto library and:

  • To the button functions add checks of to the joyhat buttons.
  • To the frame update function add framebuffer rotation and scale before it is presented to the screen. Simply resolve this for each screen mode with ifdefs – yes, this won’t work for direct mode but at least it’s something.

Interpolation doesn’t have to be done, for scaling create an array that remaps a 110/220 line to 88/176 line, then apply that to all lines of framebuffer, then rotate it whole 90 degrees. It won’t look as good if you simply leave out some columns but can be workable.

Then to port a game simply change the global Pokitto instance to the new class and you have an JoyHat version.

The design smell I am talking about is in the screen code. If rotating and scaling screen requires a huge amount of work (as @spinal said), something has been badly designed. I thought all this huge C++/OOP infrastructure was there exactly to prevent issues like this. Aight, rambling over :slight_smile: Yeah, we might rather be trying to actually solve this. What do you think of my idea above?

The part of the code that would be hard to adjust is in assembly, the C++ part probably wouldn’t need much work, if anything. Making the code more flexible in the off-chance that someone would want to sacrifice performance/RAM and butcher the graphics just to avoid the polarization? That’s a smell to me.

Try grabbing some games’ screenshots and resizing with nearest-neighbor in a graphics editor. I much prefer the filter in front of the screen idea.

I’m talking about having a choice for those who prefer playing in the correct polarization even for the cost of some performance and deformed look, not forcing everyone to play like that. I don’t think you’re against freedom, it’s probably just misunderstanding.

EDIT:

Someone who knows C++ (@pharap?) pls tell me why this doesn’t work (write to a nono address).

#include "../PokittoLib/Pokitto/Pokitto.h"

class PokittoJoy: public Pokitto::Core
{
  public:
  PokittoJoy(): Core()
  {
  }

  static bool update(bool useDirectMode=false, uint8_t updRectX=0, uint8_t updRectY=0, uint8_t updRectW=LCDWIDTH, uint8_t updRectH=LCDHEIGHT)
  {
    Pokitto::Core::update(useDirectMode,updRectX,updRectY,updRectW,updRectH);
  }
};

PokittoJoy pokitto;

int main()
{
  pokitto.begin(); 

  pokitto.setFrameRate(60);
  pokitto.display.setFont(fontTiny);
  pokitto.display.setInvisibleColor(-1);
  pokitto.display.bgcolor = 10;

  while (pokitto.isRunning())
    if (pokitto.update())
    {
      pokitto.display.drawLine(1,1,20 + pokitto.frameCount % 16,15);
      pokitto.display.drawCircle(70,60,16);
    } 
 
  return 0;
}

I’ll start with the code assistance before I get to the other points…

I don’t know if it’s what’s causing your problem, but one thing I notice is that you’re not forwarding the return result from the update call…

static bool update(bool useDirectMode = false, uint8_t updRectX = 0, uint8_t updRectY = 0, uint8_t updRectW = LCDWIDTH, uint8_t updRectH = LCDHEIGHT)
{
	return Pokitto::Core::update(useDirectMode,updRectX,updRectY,updRectW,updRectH);
}

Perhaps your font is in the wrong format?
Do you have the width and height prefixed?

I was a bit confused over whether that code was correct for a moment, but I believe it is indeed supposed to be a dereference and then an addition to the dereferenced value…

This is precisely why I prefer using & and [] over pointer arithmetic, it can be hard to verify intent when pointer arithmetic is mixed with integer arithmetic.

Hence:

void Display::setFont(const unsigned char * f)
{
	font = f;
	fontWidth = font[0] + 1;
	fontHeight = font[1] + 1;
}

Yes and no.

If the objective was flexibility, then yes, it’s an unfortunate oversight.

If flexibility wasn’t a concern, which it most likely wasn’t in this case considering the library was written for a fixed set of hardware, then it’s not unusual to prefer efficiency over flexibility.

With the PokittoLib, as with Arduboy 2, the goal is to work for the console they are designed for. Portability is either a secondary goal or simply not considered.

My main reason for agreeing that the flexibility might be useful is in case (for example) that a hat providing a second screen is developed, or there’s cause for sending the screen buffer over serial. (I know this ‘screen mirroring’ technique is used on the Arduboy sometimes.)

I can also imagine a scenario where someone might attempt to (somehow) create a ‘GPU’ hat , in which case exchanging normal calls to drawLine and co with calls that forward commands to the ‘GPU’ might be desirable.

It’s not really ‘huge’, and frankly the library’s not even very ‘OOP’ (in the ‘classical’ sense) beyond the fact it’s using multiple classes.

(As ever, this explanation/reminder is as much for the benefit of those who are less likely to know what goes on under the hood as for yourself…)

The way the library has currently been written, most of it could have been functions and variables in a namespace anyway because nearly everything is static, and as far as I’m aware there’s no virtuality being used.

Without virtuality, a C++ class with member functions is no more expensive than a C struct with related nonmember functions because that’s more or less how non-virtual classes are actually implemented. Specifically, non-virtual member functions are effectively turned into free functions, they aren’t stored as pointers to functions within the object if that’s what you’re thinking.

(One some platforms member functions use a slightly different calling convention known as ‘thiscall’, but that aside they’re practically the same.)

Similarly, static member variables and functions have the same implementation (and thus the same cost) as ‘free’ variables and functions. Scoping is nothing but compiler ‘magic’.

Virtual functions are a different story, but even then the function pointers are stored per class (in a read-only array of member function pointers) rather than per object. The object only gets one pointer of overhead - a pointer to the array of function pointers (the ‘virtual table’ or ‘vtable’), and the real cost is the runtime cost of the dereference and the virtual call. (Since you mentioned Haskell earlier, it’s worth pointing out that this is actually more or less how Haskell implements typeclasses - each typeclass instance generates an array of function pointers.)

That all said, as far as I’m aware I don’t think anything in the PokittoLib is actually using virtual functions.


Making it known that one believes something to be a bad/misjudged idea is not the same as preventing someone else from carrying out that idea. If anything, people are most likely just trying to save you from spending effort on something that isn’t likely to see much use.

But if you think you will find use for it then by all means carry on. Nobody is stopping you.

1 Like

Can anyone tell me why these variables are private and not protected? I literally can’t reimplement the function in the subclass cause I can’t access the member vars.

Best guess: the original author never expected/intended the Core class to be inherited.

(Thus violating the open-closed principle. Let’s be honest, it’s not the only SOLID principle the PokittoLib breaks.)

Correct. I didn’t think we would get this far to be honest.

1 Like

I don’t blame the author, I blame the language with baby safety locks :baby: Anyway this can thankfully be fixed with one commit.

@drummyfish and @Pharap … Pokitto::Core was intended to be a singleton, and the method by which this was done was by declaring all members static.

How would you make that extendable yet maintain it as singleton? Genuine question.

It’d be something like this:

class Core {
  static inline Core *instance;

public:
  Core(){ instance = this; }
  static Core &getInstance(){ return *(instance ?: new Core()); }
  virtual void methodA();
};

auto& core = Core::getInstance();
core.methodA();

If a class is a singleton, that means there’s only one instance of it, not that all its methods are static.
By having only static methods you can partially extend a class (as drummyfish is doing) but it will require all references to the base class to be redirected. Neither situation is ideal, which is why some consider singletons an anti-pattern.

4 Likes

Before we go too far down the singletons rabbit hole, remember that the issue @drummyfish was having was actually that he wanted/needed to access some private variables, which is actually a completely separate issue…


If we’re going to blame languages for providing the ability to hide things, there are very few languagages that couldn’t be criticised.

For example, C has the same functionality via the ability to hide variables and functions away in .c files, by making them static and not declaring them in the header.


I genuinely question the motivation behind this.

Looking at the code, Core's constructor doesn’t appear to do anything and there don’t appear to be any non-static member variables either…

Theoretically you could have achieved the same effect as what Core currently achieves by making Core a namespace and populating it with free functions and global variables, which to me seems as if the code were written in a C style and then ‘artificially’ placed inside a class shell for reasons unknown.

@FManga’s example demonstrates one way it could be done using the ‘classic’ singleton pattern. There’s also a very interesting implementation here.

However, I would argue that allowing a singleton to be inherited sort of defeats the point of a singleton since it then allows there to be more than one instance.

Personally I generally avoid singletons in the first place because they’re very easy to misuse and I find they’re almost never justified.

There are some potentially valid use cases, but I can only really think of one offhand: they somewhat make sense if you’re only targetting a single set of hardware and the singleton represents a piece of hardware that there will only ever be one of, because in that situation you generally don’t want two instances trying to control the same piece of hardware at the same time. (To be honest, I’m not entirely sure it’s justifiable even in that situation.)

That aside, I can’t think of many situations where it’s genuinely desirable to force there to only ever be a single instance of something versus just trusting the programmer to decide how many instances are required.

To give a particularly relevant example: notice that when controlling the PEX people have to create instances of classes (e.g. DigitalIn, DigitalOut) to control the pins despite there only being one of each pin. Theoretically if there’s only one of each pin on the board you could argue that’s justification to use a singleton, and yet mbed chose not to and it generally doesn’t cause any problems.

There are some downsides to that approach too, of course. For example, if someone wants to share a global instance across multiple files they have to know about extern or (as of C++17) inline. But that’s a ‘problem’ most people are going to run into at some point anyway, so it’s not really a major downside.

As for Core, I think the fact it’s a bit of a ‘god object’ is more concerning than the fact everything is static.


That’s not legal C++; C++ doesn’t have an ‘Elvis operator’.
It would have to be *(instance ? instance : new Core());.

It’s also probably worth pointing out that were this actually being used for something, the constructor should probably be protected, otherwise it’s possible to do this:

auto & coreA = Core::getInstance();
Core core = Core();

And then there’s a memory leak if coreA goes out of scope.

Besides which, leaving the constructor public sort of defeats the point of having a singleton since it allows more than one instance to be created.

It is not standard C++, but it’s accepted by both Clang and GCC. With all the other changes to the language, I wonder why they never legalized Elvis. :thinking:

I was just keeping things simple, it should definitely be protected.

Drawbacks considered, I actually prefer the static class that the PokittoLib uses. It’s simpler and at least it allows static overriding, which wouldn’t be possible if it were just loose functions or in a namespace. I’d use something like the CRTP version for work code, but it doesn’t make much sense in a hobby context where it needs to be easily understood by people still learning the language.

Exactly, it’s not legal C++. :P

It’s probably as simple as nobody having actually proposed it.

It took until C++17 for std::clamp to be proposed and until C++20 for the introduction of std::numbers::pi.

Compared to what?

If you mean compared to a ‘classic’ singleton, then yes, I agree.

Technically it’s not overriding because the functions aren’t virtual, but I think I know what you’re referring to. C# calls the practice ‘member hiding’, but I don’t think C++ has any equivalent terminology. I’ve seem some people call it ‘name hiding’, but others use ‘name hiding’ to refer to ‘variable shadowing’, which complicates matters.

Assuming you mean what I think you mean, it actually would be possible to do with a namespace:

namespace A
{
	int myVariable = 0;

	int getMyVariable()
	{
		return myVariable;
	}
}

namespace B
{
	// Does the same job as inheritance
	using namespace A;

	int getMyVariable()
	{
		return 42;
	}
}

void test()
{
	std::cout << A::myVariable << '\n';
	std::cout << A::getMyVariable() << '\n';
	std::cout << B::myVariable << '\n';
	std::cout << B::getMyVariable() << '\n';
}

(I’ve actually done this for something, but I can’t remember what offhand…)

At any rate, I wasn’t necessarily advocating namespaces as an alternative way to build it, I was using that analogy to highlight that it’s not really making use of being a class - it’s just a bag of globals with no instance functionality.

(Note: One thing a class can do that a namespace can’t is to be associated with a type alias.)

If both myself and @drummyfish (neither of whom are novices) have had trouble unpicking the library, I’m not sure the library in its current state is ‘easily understood by people still learning the language’.

Besides which, the library’s already using more esoteric things than CRTP in some places, e.g. using + on a lambda to turn it into a function pointer.

Again though, I wasn’t suggesting CRTP be used in the library. Jonne wanted examples of how it could be done, so I provided one.

(It’s probably best to move this part of the discussion into a yet another “C++ discussion topic” as it doesn’t bring any kind of value to the current topic)

1 Like

I’ve forked it off at the point @drummyfish announced his screen rotation library since that seemed to make the most sense.

(@drummyfish is free to edit title, tags and category if need be.)

1 Like
3 Likes

@Pharap thanks, going to take a look at that.

1 Like