Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Libretro: add gameboy printer support (save to png / real printer on DevTerm) #2886

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

msizov
Copy link

@msizov msizov commented Apr 16, 2023

Hi! Currently Libretro build doesn't have any support of GB Printer unlike Qt version. I've made somewhat niche Retroarch build with GBPrinter support.

  1. (Bug) Although LIBRETRO builds with defines COLOR_5_6_5 and COLOR_16BIT, correct png files couldn't be created. mImageSavePNG currently saves only XBGR8 or ABGR8 formats, and all other formats converted to those two, it calls PNGWritePixels function which has COLOR_5_6_5 define to store pixels in 565 format, which is incompatible with XBGR8 format. First commit prevents conversion to XBGR8 format when COLOR_5_6_5 is defined. This fix is used in the next commit.

  2. (Feature) Related to this issue: [Feature Request] Proper Game Boy Camera/Game Boy Printer support libretro/RetroArch#13080
    If USE_PNG cmake option is enabled in LIBRETRO build, Game Boy printer emulator connects to the core. It's print function stores image as 4-color grayscale png with timestamp in the ROM folder.

  3. (Feature) Clockwork Pi manufactures a laptop called DevTerm which has a thermal printer onboard. When USE_DEVTERM cmake option is enabled in LIBRETRO build, Gameboy Printer emulator sends data into actual thermal printer using a text protocol from the firmware. Devterm printer's width is 386 dots, so I upscale image 2x (from 160 to 320), align it in the middle of paper roll and chose thermal paper exposure value from the middle of possible range. Stock Devterm firmware supports only monochrome printing, so I reduced image colors from 4 to 2. Not sure if this feature is useful enough for the mainline code, but it works:)

A picture with working demo:
photo_2023-04-17_03-15-25

@msizov msizov changed the title Feature/libretro gameboy printer Libretro: add gameboy printer support (save to png / real printer on DevTerm) Apr 16, 2023
@endrift
Copy link
Member

endrift commented Apr 17, 2023

There's so much going on here I don't even know where to begin.

First of all, the bug you list isn't a bug. PNG does not support channel depths that aren't powers of 2, and does not support non-uniform channel depth between channels anyway. The fact that whatever you've done here works at all is definitely by accident. There isn't even any reason the image you're creating needs to be in 565 in the first place, since you're creating it by hand. If anything, it should be L8, but mGBA doesn't support writing grayscale out at the moment.

Second of all, libretro builds should not be using external dependencies like libpng. The libretro API is...well, terribly misdesigned and external dependencies for optional features like this will lead to a lot of problems down the line with portability.

Third of all, doing all this for one specific niche device is far, far too niche for me to accept here. It's a neat idea, but I simply can't accept something like this, especially given how many questionable implementation issues you hit in the process. While I support the idea of there being some way of using the thermal printer for GB printer stuff (it's really neat!), this...really is not the way to go about it.

@endrift endrift closed this Apr 17, 2023
@msizov
Copy link
Author

msizov commented Apr 17, 2023

Thanks for a quick feedback:)

It seems that you missed the issue of first commit. With current code, there is no way to save correct png by mImageSavePNG when COLOR_5_6_5 is defined. It may be irrelevant in current use cases, esp. given that libpng shouldn't be used with libretro, but code is still a bit inconsistent.

There isn't even any reason the image you're creating needs to be in 565 in the first place, since you're creating it by hand.

Reasoning is the following: COLOR_5_6_5 is extensively used in libretro. You can create XBGR8 image, and everything will work fine up until PNGWritePixels that ALWAYS interprets data as 565 format because of that COLOR_5_6_5 define.

for (i = 0; i < height; ++i) {
unsigned x;
for (x = 0; x < width; ++x) {
#ifdef COLOR_16_BIT
uint16_t c = ((uint16_t*) pixelData)[stride * i + x];
#ifdef COLOR_5_6_5
row[x * 3] = (c >> 8) & 0xF8;
row[x * 3 + 1] = (c >> 3) & 0xFC;
row[x * 3 + 2] = (c << 3) & 0xF8;
#else
row[x * 3] = (c >> 7) & 0xF8;
row[x * 3 + 1] = (c >> 2) & 0xF8;
row[x * 3 + 2] = (c << 3) & 0xF8;
#endif
#else
#ifdef __BIG_ENDIAN__
row[x * 3] = pixelData[stride * i * 4 + x * 4 + 3];
row[x * 3 + 1] = pixelData[stride * i * 4 + x * 4 + 2];
row[x * 3 + 2] = pixelData[stride * i * 4 + x * 4 + 1];
#else
row[x * 3] = pixelData[stride * i * 4 + x * 4];
row[x * 3 + 1] = pixelData[stride * i * 4 + x * 4 + 1];
row[x * 3 + 2] = pixelData[stride * i * 4 + x * 4 + 2];
#endif
#endif
}
png_write_row(png, row);
}

You will get mangled image as a result. And because of format conversion to XBGR8 in mImageSavePNG, images would use incompatible format.

mgba/src/util/image.c

Lines 210 to 242 in 225456a

bool mImageSavePNG(const struct mImage* image, struct VFile* vf) {
if (image->format != mCOLOR_XBGR8 && image->format != mCOLOR_ABGR8) {
struct mImage* newImage;
if (mColorFormatHasAlpha(image->format)) {
newImage = mImageConvertToFormat(image, mCOLOR_ABGR8);
} else {
newImage = mImageConvertToFormat(image, mCOLOR_XBGR8);
}
bool ret = mImageSavePNG(newImage, vf);
mImageDestroy(newImage);
return ret;
}
png_structp png = PNGWriteOpen(vf);
png_infop info = NULL;
bool ok = false;
if (png) {
if (image->format == mCOLOR_XBGR8) {
info = PNGWriteHeader(png, image->width, image->height);
if (info) {
ok = PNGWritePixels(png, image->width, image->height, image->stride, image->data);
}
} else {
info = PNGWriteHeaderA(png, image->width, image->height);
if (info) {
ok = PNGWritePixelsA(png, image->width, image->height, image->stride, image->data);
}
}
PNGWriteClose(png, info);
}
return ok;
}
#endif

So with COLOR_5_6_5, either format must be 565 and conversion to BGR8 disabled or PNGWritePixels / PNGWritePixelsA need to be changed to handle only XRGB8 images.

Second of all, libretro builds should not be using external dependencies like libpng.

Would it be appropriate if instead of introducing dependency on libpng, libretro build would save prints as bmp files using header only library such as this one ?

While I support the idea of there being some way of using the thermal printer for GB printer stuff (it's really neat!), this...really is not the way to go about it.

Thermal printer doesn't use any of png-related code, it only adds GB_SIO to libretro build and sends some text data to a special device. What do you think could be a valid approach to implement such thermal printer support?

@endrift
Copy link
Member

endrift commented Apr 17, 2023

Oh, I see what you mean about the PNG issue. That's definitely distinct and the result of some code I wrote recently around that older code. I should fix that first. Give me a bit, and I can reopen this, but I'd prefer it if you switch this to draft in the meantime. Part of the problem is that libretro itself shouldn't be doing file I/O directly at all if it can avoid it, which leaves this in a weird situation.

@endrift endrift reopened this Apr 17, 2023
@msizov msizov marked this pull request as draft April 17, 2023 07:45
@msizov
Copy link
Author

msizov commented Apr 17, 2023

Ok, marked PR as a draft:)

@msizov
Copy link
Author

msizov commented Apr 17, 2023

Part of the problem is that libretro itself shouldn't be doing file I/O directly at all if it can avoid it, which leaves this in a weird situation.

Yeah, both features are impossible without working with I/O :(

Tbh, I've implemented them in the LIBRETRO build only because of Retroachievments :)

@endrift
Copy link
Member

endrift commented Apr 17, 2023

Okay, I've fixed up mImage/PNG writing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants