InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.63k stars 902 forks source link

State of GFX? #2008

Closed mark9064 closed 4 months ago

mark9064 commented 4 months ago

GFX within InifniTime (https://github.com/InfiniTimeOrg/InfiniTime/tree/main/src/components/gfx) isn't used by anything anywhere as far as I can tell?

Could anyone explain the history of this? It's taken me a while to realise it's not part of the OS render pipeline at all and its existence is pretty confusing!

Maybe it is time for it to be removed if it is legacy? At least for me dead code adds to the effort to understand InfiniTime and slows development down. Git will always remember it if we want to refer back

JF002 commented 4 months ago

I implemented the GFX class at the very beginning of the project, before we replaced it by LVGL. It's very basic, but allowed me to design and optimize the SPI and LCD drivers.

I thought that it was still being used in the recovery firmware but if it's not used, I'm 100% OK to remove it : I don't like to maintain dead code either!

KaffeinatedKat commented 4 months ago

if it is being used by the recovery firmware, it should be changed to just use LVGL like everything else and be removed. Don't see much of a reason to keep and maintain some legacy code just for the recovery firmware if it can just use LVGL like the rest of InfiniTime

JF002 commented 4 months ago

if it is being used by the recovery firmware, it should be changed to just use LVGL like everything else and be removed. Don't see much of a reason to keep and maintain some legacy code just for the recovery firmware if it can just use LVGL like the rest of InfiniTime

I do not agree with this : We must keep the code of the recovery firmware as simple as possible to reduce the risk of critical bugs (less code and less complexity -> less bugs). The recovery firmware does not integrate a complex UI, it simply displays a logo full screen, and it does not need the whole LVGL to do this. Also, the size of the recovery firmware must be lower than 265KB to fit in memory. Anyway, I've just checked : the recovery fw does not use LVGL and GFX and it directly use the lcd driver instead.

EDIT : sorry about closing/opening this issue again, my fingers probably slipped on my keyboard

mark9064 commented 4 months ago

Thanks for the discussion and review :)

I'll close this now