NiLuJe / FBInk

FrameBuffer eInker, a small tool & library to print text & images to an eInk Linux framebuffer
https://www.mobileread.com/forums/showthread.php?t=299110
GNU General Public License v3.0
332 stars 23 forks source link

Add more functionality to the API #65

Closed Rain92 closed 3 years ago

Rain92 commented 3 years ago

These are my proposed changes for the FBInk API.

Some new members to the FBInkState struct: ~~uint8_t current_rota_native; // native screen rotaiton; vInfo.rotate (current rotation, c.f., <linux/fb.h>) uint8_t current_rota_canonical; // canonical screen rotation~~ uint32_t screen_stride; // screen line length in bytes;

A new method to access the underlying framebuffer: // Grants direct access to the frame buffer pointer as well as the size of the frame buffer allocation. // If the framebuffer is not yet allocated it will do so and return the result of the operation. FBINK_API int fbink_get_fb_pointer(int fbfd, FBPtrInfo *fbInfo);

A new method to set the framebuffer rotation and bpp. The code is mostly copied from fbdepth and probably needs some tinkering: // Sets the framebuffer bits per pixel and rotation and invoke a reinit afterwards // rota will be the rotation in natve format; use fbink_rota_canonical_to_native to convert it to canonical // bpp will remain unchanged if the value is < 8 // req_gray will remain unchanged if the value is < 0 // on sunxi devices it will invoke fbink_sunxi_ntx_enforce_rota FBINK_API int fbink_set_fb_info(int fbFd, int32_t bpp, int8_t rota, int32_t req_gray, const FBInkConfig* restrict fbink_cfg);


This change is Reviewable

NiLuJe commented 3 years ago

I'll have more time this week-end, so, very, very quick comments:

NiLuJe commented 3 years ago
* The get_fb_data stuff probably ought to export const pointers to const data to vInfo & fInfo, because PocketBook suffers from the same inconsistencies we discussed ;).

Then again, the rota/bitdepth stuff isn't really supported on PB (as it's even quirkier than on Kobo), and the only things one could really have missed were indeed line_length & smem_len ;).

Plus, I really like the get_fb_ptr name, and that wouldn't fit as well if it did other stuff ;p.

Rain92 commented 3 years ago

I agree it's not really nesassary to expose vInfo & fInfo right now (except for the color format in vinfo but let's just ignore that). I can rename it to get_fb_ptr. Do you propose to put the allocation size into the FBInkState struct instead?

Rain92 commented 3 years ago

I am not sure how to mark the exported pointer as const tho as it has to be written somewhere if it is passed as an argument. And if it is passed as the return value it would be a bit wanky because the method has to return an exit code.

NiLuJe commented 3 years ago

I was vaguely thinking of something along the lines of unsigned char* fbink_get_fb_pointer(int fbfd, size_t* alloc_size);, i.e., the alloc size as an outptr, and returning NULL on error ;).

EDIT: Can't const rvalues anyway, brain is fried ^^.

The only way I can think of to return a const something would be with an unwieldier double pointer with something like an unsigned char * restrict const * fb_ptr; member in the struct set to &fbPtr. But that's a bit icky ;p.

TL;DR: I'll sleep on it some more. Not super fond of the brand new struct for this, but I don't hate it either, hence the proposed signature above. And I do like keeping this separate and those two together (so, no moving alloc_size to FBInkState).

NiLuJe commented 3 years ago

As far as set_fbinfo is concerned (don't mind the names I'm using here, writing this quickly without actual reference, and nothing's set in stone ;p), I'd move it to fbink_rota_quirks.c.

As for its actual code, I've got a few quick notes jotted down, but I'll probably simply handle some minor cleanup post-merge myself, as the API looks fine ;).

I do like the sunxi shortcut for rota, I hadn't quite thought it through yet, and my original plan was simply to throw an ENOTSUP on sunxi, but this is much better, and actually makes sense ;p.

EDIT: I would possibly just move rota before bpp, as leaving bpp & gray alone is going to be far more common than tweaking rota, and bpp & gray kinda go hand in hand anyway, so I like 'em close together ;).

(FWIW, part of the stuff I jotted down involves making the magic "don't change me" values actual defines to make that clearer in the API docs).

NiLuJe commented 3 years ago

Thanks!