From fb2d69681513e5a7d3d892726644acc6c561ee5b Mon Sep 17 00:00:00 2001 From: Mykhailo Shevchuk Date: Thu, 23 Jul 2026 20:19:10 +0300 Subject: [PATCH 1/2] Paint: fix stuck pen, add Move/Draw/Erase modes and canvas persistence Fixes #14 The pen appeared stuck because a long Ok press toggled a hidden isDrawing flag with no visual indication, and while it was set every short Ok press force-repainted the cell it had just toggled, so the pen could never be lifted or a dot erased. - Replace the hidden flag with three modes cycled by a long Ok press: Move, Draw, Erase; the XOR cursor shape shows the active mode (small dot / full cell / hollow frame), so an accidental long press is visible and one short press away from recovery - Short Ok toggles the cell in Move mode and lifts the pen or eraser in Draw/Erase modes - Arrows now react to press, long and repeat events, so holding a key keeps moving (and draws or erases a line with the pen down); strokes mark both the cell left and the cell entered - Save the canvas on exit (temp file + sync + rename so a failed write cannot destroy the previous save) and restore it on start; a failed save shows a storage error dialog instead of silently losing the drawing - Tear down the viewport before saving so a hanging SD write cannot freeze the GUI thread on a full event queue - Fix uninitialized PaintData, missing mutex use in the event loop, RECORD_NOTIFICATION closed without being opened, an event-queue leak on the mutex failure path and dead code in the clear handler - Add CHANGELOG.md backfilled from repo history; bump to 1.4 Co-Authored-By: Claude Opus 4.8 (1M context) --- apps_source_code/paint/CHANGELOG.md | 27 +++ apps_source_code/paint/application.fam | 4 +- apps_source_code/paint/paint.c | 265 +++++++++++++++++-------- 3 files changed, 215 insertions(+), 81 deletions(-) create mode 100644 apps_source_code/paint/CHANGELOG.md diff --git a/apps_source_code/paint/CHANGELOG.md b/apps_source_code/paint/CHANGELOG.md new file mode 100644 index 00000000..a61773a5 --- /dev/null +++ b/apps_source_code/paint/CHANGELOG.md @@ -0,0 +1,27 @@ +# Changelog + +## 1.4 + +- Fix the pen getting stuck down after pressing Ok together with a direction key: drawing mode was toggled by a long Ok press with no visual indication, and a short Ok press could not lift it +- Replace the hidden drawing flag with three modes cycled by a long Ok press: Move, Draw and Erase; the cursor shape shows the active mode (small dot, full cell, hollow frame) +- A short Ok press toggles the cell under the cursor in Move mode and lifts the pen or eraser in Draw and Erase modes +- Hold a direction key to keep moving (with the pen down this draws or erases a line); previously held keys did nothing +- Save the canvas on exit and restore it on the next start; a failed save shows a storage error message instead of silently losing the drawing +- Hold Back to clear the canvas and lift the pen +- Start with the cursor in the center of the canvas instead of the top-left corner + +## 1.3 + +- Version bump for catalog compatibility (no functional changes) + +## 1.2 + +- Version bump for catalog compatibility (no functional changes) + +## 1.1 + +- Version bump for catalog compatibility (no functional changes) + +## 1.0 + +- Initial release: draw on a 32x16 board with a dot cursor, hold Ok to toggle continuous drawing, hold Back to clear the screen diff --git a/apps_source_code/paint/application.fam b/apps_source_code/paint/application.fam index 2dc1bd3c..9f2aacf3 100644 --- a/apps_source_code/paint/application.fam +++ b/apps_source_code/paint/application.fam @@ -11,6 +11,6 @@ App( fap_category="Games", fap_author="@n-o-T-I-n-s-a-n-e", fap_weburl="https://github.com/n-o-T-I-n-s-a-n-e", - fap_version="1.3", - fap_description="A basic Paint app, Click Ok to draw dot, hold Ok to enable drawing continuously, hold Back to clear the screen", + fap_version="1.4", + fap_description="A basic Paint app. Ok: toggle dot or lift the pen, hold Ok: cycle Move/Draw/Erase mode, hold Back: clear, Back: save and exit. The canvas is restored on the next start", ) diff --git a/apps_source_code/paint/paint.c b/apps_source_code/paint/paint.c index 0d4124b0..758c157c 100644 --- a/apps_source_code/paint/paint.c +++ b/apps_source_code/paint/paint.c @@ -1,10 +1,27 @@ #include -#include +#include #include #include -#include -#include -#include // Header-file for boolean data-type. +#include +#include + +#define BOARD_WIDTH 32 +#define BOARD_HEIGHT 16 +#define CELL_SIZE 4 + +#define PAINT_SAVE_PATH APP_DATA_PATH("canvas.bin") +#define PAINT_SAVE_TMP_PATH APP_DATA_PATH("canvas.bin.tmp") + +//file header so a foreign file is not loaded as a canvas +//(truncation is caught by the exact-size reads) +static const char paint_save_magic[4] = {'P', 'N', 'T', '1'}; + +typedef enum { + PaintModeMove, //cursor moves without touching the board + PaintModeDraw, //cursor paints every cell it visits + PaintModeErase, //cursor clears every cell it visits + PaintModeCount, +} PaintMode; typedef struct selected_position { int x; @@ -14,8 +31,8 @@ typedef struct selected_position { typedef struct { FuriMutex* mutex; selected_position selected; - bool board[32][16]; - bool isDrawing; + bool board[BOARD_WIDTH][BOARD_HEIGHT]; + PaintMode mode; } PaintData; void paint_draw_callback(Canvas* canvas, void* ctx) { @@ -25,23 +42,34 @@ void paint_draw_callback(Canvas* canvas, void* ctx) { canvas_clear(canvas); canvas_set_color(canvas, ColorBlack); - //draw the canvas(32x16) on screen(144x64) using 4x4 tiles - for(int y = 0; y < 16; y++) { - for(int x = 0; x < 32; x++) { + //draw the board(32x16) on screen(128x64) using 4x4 tiles + for(int y = 0; y < BOARD_HEIGHT; y++) { + for(int x = 0; x < BOARD_WIDTH; x++) { if(paint_state->board[x][y]) { - canvas_draw_box(canvas, x * 4, y * 4, 4, 4); + canvas_draw_box(canvas, x * CELL_SIZE, y * CELL_SIZE, CELL_SIZE, CELL_SIZE); } } } - //draw cursor as a 4x4 black box with a 2x2 white box inside - canvas_set_color(canvas, ColorBlack); - canvas_draw_box(canvas, paint_state->selected.x * 4, paint_state->selected.y * 4, 4, 4); - canvas_set_color(canvas, ColorWhite); - canvas_draw_box( - canvas, paint_state->selected.x * 4 + 1, paint_state->selected.y * 4 + 1, 2, 2); + //the cursor is drawn in XOR so it stays visible on both painted and + //empty cells; its shape shows the active mode + int cursor_x = paint_state->selected.x * CELL_SIZE; + int cursor_y = paint_state->selected.y * CELL_SIZE; + canvas_set_color(canvas, ColorXOR); + switch(paint_state->mode) { + case PaintModeMove: //small dot: the pen is up + canvas_draw_box(canvas, cursor_x + 1, cursor_y + 1, 2, 2); + break; + case PaintModeDraw: //full cell: the pen is down + canvas_draw_box(canvas, cursor_x, cursor_y, CELL_SIZE, CELL_SIZE); + break; + case PaintModeErase: //hollow frame: the eraser is down + canvas_draw_frame(canvas, cursor_x, cursor_y, CELL_SIZE, CELL_SIZE); + break; + default: + break; + } - //release the mutex furi_mutex_release(paint_state->mutex); } @@ -51,16 +79,102 @@ void paint_input_callback(InputEvent* input_event, void* ctx) { furi_message_queue_put(event_queue, input_event, FuriWaitForever); } +//paint or erase the cell under the cursor according to the active mode +static void paint_apply_mode(PaintData* paint_state) { + if(paint_state->mode == PaintModeDraw) { + paint_state->board[paint_state->selected.x][paint_state->selected.y] = true; + } else if(paint_state->mode == PaintModeErase) { + paint_state->board[paint_state->selected.x][paint_state->selected.y] = false; + } +} + +static void paint_move_cursor(PaintData* paint_state, int dx, int dy) { + //apply the mode to the cell being left as well, so a stroke + //includes the cell it started on + paint_apply_mode(paint_state); + paint_state->selected.x = CLAMP(paint_state->selected.x + dx, BOARD_WIDTH - 1, 0); + paint_state->selected.y = CLAMP(paint_state->selected.y + dy, BOARD_HEIGHT - 1, 0); + paint_apply_mode(paint_state); +} + +//react to the initial press and keep moving while the key is held; +//InputTypeLong is included so movement does not stall between the +//press and the first repeat +static bool paint_is_move_event(InputType type) { + return type == InputTypePress || type == InputTypeLong || type == InputTypeRepeat; +} + +static void paint_load(PaintData* paint_state) { + Storage* storage = furi_record_open(RECORD_STORAGE); + File* file = storage_file_alloc(storage); + if(storage_file_open(file, PAINT_SAVE_PATH, FSAM_READ, FSOM_OPEN_EXISTING)) { + char magic[sizeof(paint_save_magic)]; + bool loaded = storage_file_read(file, magic, sizeof(magic)) == sizeof(magic) && + memcmp(magic, paint_save_magic, sizeof(magic)) == 0 && + storage_file_read(file, paint_state->board, sizeof(paint_state->board)) == + sizeof(paint_state->board); + if(loaded) { + //a bool must hold 0 or 1, file bytes can hold anything + uint8_t* cells = (uint8_t*)paint_state->board; + for(size_t i = 0; i < sizeof(paint_state->board); i++) { + cells[i] = (cells[i] != 0); + } + } else { + FURI_LOG_W("paint", "invalid save file, starting with an empty canvas"); + memset(paint_state->board, 0, sizeof(paint_state->board)); + } + } else if(storage_file_get_error(file) != FSE_NOT_EXIST) { + //a missing file is a normal first run; anything else is worth a trace + FURI_LOG_W("paint", "cannot read the save file: %s", storage_file_get_error_desc(file)); + } + storage_file_close(file); + storage_file_free(file); + furi_record_close(RECORD_STORAGE); +} + +static bool paint_save(const PaintData* paint_state) { + Storage* storage = furi_record_open(RECORD_STORAGE); + File* file = storage_file_alloc(storage); + //write to a temporary file first so a failed write cannot destroy + //the previous save + bool saved = storage_file_open(file, PAINT_SAVE_TMP_PATH, FSAM_WRITE, FSOM_CREATE_ALWAYS) && + storage_file_write(file, paint_save_magic, sizeof(paint_save_magic)) == + sizeof(paint_save_magic) && + storage_file_write(file, paint_state->board, sizeof(paint_state->board)) == + sizeof(paint_state->board) && + storage_file_sync(file); + if(!saved) { + FURI_LOG_E("paint", "cannot save the canvas: %s", storage_file_get_error_desc(file)); + } + storage_file_close(file); + if(saved) { + FS_Error error = storage_common_rename(storage, PAINT_SAVE_TMP_PATH, PAINT_SAVE_PATH); + if(error != FSE_OK) { + saved = false; + FURI_LOG_E("paint", "cannot save the canvas: %s", storage_error_get_desc(error)); + } + } + storage_file_free(file); + furi_record_close(RECORD_STORAGE); + return saved; +} + int32_t paint_app(void* p) { UNUSED(p); FuriMessageQueue* event_queue = furi_message_queue_alloc(8, sizeof(InputEvent)); PaintData* paint_state = malloc(sizeof(PaintData)); + memset(paint_state->board, 0, sizeof(paint_state->board)); + paint_state->selected.x = BOARD_WIDTH / 2; + paint_state->selected.y = BOARD_HEIGHT / 2; + paint_state->mode = PaintModeMove; + paint_load(paint_state); paint_state->mutex = furi_mutex_alloc(FuriMutexTypeNormal); if(!paint_state->mutex) { FURI_LOG_E("paint", "cannot create mutex\r\n"); free(paint_state); + furi_message_queue_free(event_queue); return -1; } @@ -73,80 +187,73 @@ int32_t paint_app(void* p) { Gui* gui = furi_record_open(RECORD_GUI); gui_add_view_port(gui, view_port, GuiLayerFullscreen); - //NotificationApp* notification = furi_record_open(RECORD_NOTIFICATION); - InputEvent event; + bool running = true; - while(furi_message_queue_get(event_queue, &event, FuriWaitForever) == FuriStatusOk) { - //break out of the loop if the back key is pressed - if(event.type == InputTypeShort && event.key == InputKeyBack) { - break; - } - - //check the key pressed and change x and y accordingly - if(event.type == InputTypeShort) { - switch(event.key) { - case InputKeyUp: - paint_state->selected.y -= 1; - break; - case InputKeyDown: - paint_state->selected.y += 1; - break; - case InputKeyLeft: - paint_state->selected.x -= 1; - break; - case InputKeyRight: - paint_state->selected.x += 1; - break; - case InputKeyOk: - paint_state->board[paint_state->selected.x][paint_state->selected.y] = - !paint_state->board[paint_state->selected.x][paint_state->selected.y]; - break; - - default: - break; - } + while(running && + furi_message_queue_get(event_queue, &event, FuriWaitForever) == FuriStatusOk) { + furi_mutex_acquire(paint_state->mutex, FuriWaitForever); - //check if cursor position is out of bounds and reset it to the closest position - if(paint_state->selected.x < 0) { - paint_state->selected.x = 0; - } - if(paint_state->selected.x > 31) { - paint_state->selected.x = 31; - } - if(paint_state->selected.y < 0) { - paint_state->selected.y = 0; - } - if(paint_state->selected.y > 15) { - paint_state->selected.y = 15; - } - if(paint_state->isDrawing == true) { - paint_state->board[paint_state->selected.x][paint_state->selected.y] = true; - } - view_port_update(view_port); - } - if(event.key == InputKeyBack && event.type == InputTypeLong) { - paint_state->board[1][1] = true; - for(int y = 0; y < 16; y++) { - for(int x = 0; x < 32; x++) { - paint_state->board[x][y] = false; + switch(event.key) { + case InputKeyUp: + if(paint_is_move_event(event.type)) paint_move_cursor(paint_state, 0, -1); + break; + case InputKeyDown: + if(paint_is_move_event(event.type)) paint_move_cursor(paint_state, 0, 1); + break; + case InputKeyLeft: + if(paint_is_move_event(event.type)) paint_move_cursor(paint_state, -1, 0); + break; + case InputKeyRight: + if(paint_is_move_event(event.type)) paint_move_cursor(paint_state, 1, 0); + break; + case InputKeyOk: + if(event.type == InputTypeShort) { + if(paint_state->mode == PaintModeMove) { + //toggle the cell under the cursor + paint_state->board[paint_state->selected.x][paint_state->selected.y] = + !paint_state->board[paint_state->selected.x][paint_state->selected.y]; + } else { + //a short press lifts the pen or eraser + paint_state->mode = PaintModeMove; } + } else if(event.type == InputTypeLong) { + paint_state->mode = (PaintMode)((paint_state->mode + 1) % PaintModeCount); } - view_port_update(view_port); - } - if(event.key == InputKeyOk && event.type == InputTypeLong) { - paint_state->isDrawing = !paint_state->isDrawing; - paint_state->board[paint_state->selected.x][paint_state->selected.y] = true; - view_port_update(view_port); + break; + case InputKeyBack: + if(event.type == InputTypeLong) { + //start over with an empty canvas and the pen lifted + memset(paint_state->board, 0, sizeof(paint_state->board)); + paint_state->mode = PaintModeMove; + } else if(event.type == InputTypeShort) { + running = false; + } + break; + default: + break; } + + furi_mutex_release(paint_state->mutex); + view_port_update(view_port); } + //tear down the viewport before the (potentially slow) save so input + //events can no longer pile up in a queue nobody drains anymore gui_remove_view_port(gui, view_port); view_port_free(view_port); furi_message_queue_free(event_queue); - furi_mutex_free(paint_state->mutex); - furi_record_close(RECORD_NOTIFICATION); furi_record_close(RECORD_GUI); + + if(!paint_save(paint_state)) { + //the drawing could not be written out - tell the user instead of + //silently losing it + DialogsApp* dialogs = furi_record_open(RECORD_DIALOGS); + dialog_message_show_storage_error(dialogs, "Cannot save\nthe canvas"); + furi_record_close(RECORD_DIALOGS); + } + + furi_mutex_free(paint_state->mutex); free(paint_state); return 0; From 59724fd6c87a14c5b16aca2f05fa2f90605d2555 Mon Sep 17 00:00:00 2001 From: Mykhailo Shevchuk Date: Thu, 23 Jul 2026 21:50:19 +0300 Subject: [PATCH 2/2] Paint: route canvas persistence through toolbox saved_struct Review/simplify follow-up. The hand-rolled save/load reimplemented lib/toolbox/saved_struct, the established persistence helper used by sibling apps (ir_intervalometer, air_arkanoid). Route through it: - saved_struct_save/saved_struct_load replace the manual open, read, write, close and size-check plumbing; they embed a magic, version and checksum, so the 4-byte magic constant and the 0/1 byte-normalization loop are gone (the checksum rejects a corrupt payload wholesale). - Keep the deliberate crash-safety: save to the temp file then rename it over the real save, so a failed write cannot destroy the previous canvas. Still memset the board to an empty canvas on any load failure. No behavior change: save on exit, restore on start, reject foreign or corrupt files (now checksum-backed), show the storage error dialog when the save cannot be written. Net -18 lines. Builds clean under ufbt (API 88.0), ufbt format applied. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps_source_code/paint/paint.c | 80 +++++++++++++--------------------- 1 file changed, 31 insertions(+), 49 deletions(-) diff --git a/apps_source_code/paint/paint.c b/apps_source_code/paint/paint.c index 758c157c..74732316 100644 --- a/apps_source_code/paint/paint.c +++ b/apps_source_code/paint/paint.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #define BOARD_WIDTH 32 @@ -12,9 +13,10 @@ #define PAINT_SAVE_PATH APP_DATA_PATH("canvas.bin") #define PAINT_SAVE_TMP_PATH APP_DATA_PATH("canvas.bin.tmp") -//file header so a foreign file is not loaded as a canvas -//(truncation is caught by the exact-size reads) -static const char paint_save_magic[4] = {'P', 'N', 'T', '1'}; +//saved_struct tags the file with this magic + version and a checksum, so a +//foreign, truncated or corrupt save is rejected on load +#define PAINT_SAVE_MAGIC 0x50 +#define PAINT_SAVE_VERSION 1 typedef enum { PaintModeMove, //cursor moves without touching the board @@ -105,58 +107,38 @@ static bool paint_is_move_event(InputType type) { } static void paint_load(PaintData* paint_state) { - Storage* storage = furi_record_open(RECORD_STORAGE); - File* file = storage_file_alloc(storage); - if(storage_file_open(file, PAINT_SAVE_PATH, FSAM_READ, FSOM_OPEN_EXISTING)) { - char magic[sizeof(paint_save_magic)]; - bool loaded = storage_file_read(file, magic, sizeof(magic)) == sizeof(magic) && - memcmp(magic, paint_save_magic, sizeof(magic)) == 0 && - storage_file_read(file, paint_state->board, sizeof(paint_state->board)) == - sizeof(paint_state->board); - if(loaded) { - //a bool must hold 0 or 1, file bytes can hold anything - uint8_t* cells = (uint8_t*)paint_state->board; - for(size_t i = 0; i < sizeof(paint_state->board); i++) { - cells[i] = (cells[i] != 0); - } - } else { - FURI_LOG_W("paint", "invalid save file, starting with an empty canvas"); - memset(paint_state->board, 0, sizeof(paint_state->board)); - } - } else if(storage_file_get_error(file) != FSE_NOT_EXIST) { - //a missing file is a normal first run; anything else is worth a trace - FURI_LOG_W("paint", "cannot read the save file: %s", storage_file_get_error_desc(file)); + //a missing or corrupt save just leaves the initialized empty canvas + if(!saved_struct_load( + PAINT_SAVE_PATH, + paint_state->board, + sizeof(paint_state->board), + PAINT_SAVE_MAGIC, + PAINT_SAVE_VERSION)) { + memset(paint_state->board, 0, sizeof(paint_state->board)); } - storage_file_close(file); - storage_file_free(file); - furi_record_close(RECORD_STORAGE); } static bool paint_save(const PaintData* paint_state) { - Storage* storage = furi_record_open(RECORD_STORAGE); - File* file = storage_file_alloc(storage); - //write to a temporary file first so a failed write cannot destroy - //the previous save - bool saved = storage_file_open(file, PAINT_SAVE_TMP_PATH, FSAM_WRITE, FSOM_CREATE_ALWAYS) && - storage_file_write(file, paint_save_magic, sizeof(paint_save_magic)) == - sizeof(paint_save_magic) && - storage_file_write(file, paint_state->board, sizeof(paint_state->board)) == - sizeof(paint_state->board) && - storage_file_sync(file); - if(!saved) { - FURI_LOG_E("paint", "cannot save the canvas: %s", storage_file_get_error_desc(file)); + //write to a temporary file first, then rename it over the real save, so a + //failed write cannot destroy the previously saved canvas + if(!saved_struct_save( + PAINT_SAVE_TMP_PATH, + paint_state->board, + sizeof(paint_state->board), + PAINT_SAVE_MAGIC, + PAINT_SAVE_VERSION)) { + FURI_LOG_E("paint", "cannot write the canvas"); + return false; } - storage_file_close(file); - if(saved) { - FS_Error error = storage_common_rename(storage, PAINT_SAVE_TMP_PATH, PAINT_SAVE_PATH); - if(error != FSE_OK) { - saved = false; - FURI_LOG_E("paint", "cannot save the canvas: %s", storage_error_get_desc(error)); - } - } - storage_file_free(file); + + Storage* storage = furi_record_open(RECORD_STORAGE); + FS_Error error = storage_common_rename(storage, PAINT_SAVE_TMP_PATH, PAINT_SAVE_PATH); furi_record_close(RECORD_STORAGE); - return saved; + if(error != FSE_OK) { + FURI_LOG_E("paint", "cannot save the canvas: %s", storage_error_get_desc(error)); + return false; + } + return true; } int32_t paint_app(void* p) {