AEFeinstein / Super-2024-Swadge-FW

Firmware for the Super Magfest 2024 Swadge
https://adam.feinste.in/Super-2024-Swadge-FW/
MIT License
8 stars 9 forks source link

Paint fixes (dylwhich's version) #180

Closed dylwhich closed 6 months ago

dylwhich commented 6 months ago

Description

All of Bryce's fixes rebased onto current main, plus I got rid of the green plus in the image title that shouldn't have made it into the final version.

Test Instructions

Use paint, particularly the menu and saving and things, make sure there's no bugs.

Ticket Links

Readiness Checklist

AEFeinstein commented 6 months ago

Note to self, when this gets merged, also delete https://github.com/AEFeinstein/Swadge-IDF-5.0/tree/paint-fixes

AEFeinstein commented 6 months ago

Two more notes.

  1. I think there's something off in paintGalleryModeButtonCb(). The reminder text says

    A: Load Pause: Delete

    But "A" is pulling up more reminder text and Pause is full-screening the image. That matches the source code, so I think it's the reminder text that's the issue. Should paintGalleryModeButtonCb() be calling imageBrowserButton() fore more of the buttons?

  2. This patch should clean up the strlen() stuff I noted.

    diff --git a/main/modes/mfpaint/paint_browser.c b/main/modes/mfpaint/paint_browser.c
    index 07bf8b8f..99de9c41 100644
    --- a/main/modes/mfpaint/paint_browser.c
    +++ b/main/modes/mfpaint/paint_browser.c
    @@ -301,8 +301,8 @@ void drawImageBrowser(imageBrowser_t* browser)
    
         if (drawList)
         {
    -        const uint8_t topTextLen = ARRAY_SIZE(browserTitleStr) + browserActionToStringLen(browser->primaryAction)
    -                                   + browserActionToStringLen(browser->secondaryAction) - 4 + 1;
    +        const uint8_t topTextLen = strlen(browserTitleStr) + browserActionToStringLen(browser->primaryAction)
    +                                   + browserActionToStringLen(browser->secondaryAction) + 1;
             char topText[topTextLen];
             snprintf(topText, topTextLen, browserTitleStr, browserActionToString(browser->primaryAction),
                      browserActionToString(browser->secondaryAction));
    @@ -690,31 +690,10 @@ static const char* browserActionToString(imageBrowserAction_t action)
    
     static uint8_t browserActionToStringLen(imageBrowserAction_t action)
     {
    -    switch (action)
    +    const char* str = browserActionToString(action);
    +    if (NULL != str)
         {
    -        case BROWSER_EXIT:
    -        {
    -            return dialogOptionCancelStrLen;
    -        }
    -
    -        case BROWSER_OPEN:
    -        {
    -            return toolWheelLoadStrLen;
    -        }
    -
    -        case BROWSER_SAVE:
    -        {
    -            return dialogOptionSaveAsStrLen;
    -        }
    -
    -        case BROWSER_DELETE:
    -        {
    -            return ARRAY_SIZE(deleteStr);
    -        }
    -
    -        default:
    -        {
    -            return 0;
    -        }
    +        return strlen(str);
         }
    +    return 0;
     }
    \ No newline at end of file
    diff --git a/main/modes/mfpaint/paint_draw.c b/main/modes/mfpaint/paint_draw.c
    index 519e3581..caadca0e 100644
    --- a/main/modes/mfpaint/paint_draw.c
    +++ b/main/modes/mfpaint/paint_draw.c
    @@ -93,10 +93,6 @@ const char dialogOptionSaveAsStr[]      = "Save as...";
     static const char dialogOptionExitStr[] = "Quit";
     static const char dialogOptionOkStr[]   = "OK";
    
    -const uint8_t toolWheelLoadStrLen      = ARRAY_SIZE(toolWheelLoadStr);
    -const uint8_t dialogOptionCancelStrLen = ARRAY_SIZE(dialogOptionCancelStr);
    -const uint8_t dialogOptionSaveAsStrLen = ARRAY_SIZE(dialogOptionSaveAsStr);
    -
     static paletteColor_t defaultPalette[] = {
         c000, // black
         c555, // white
    diff --git a/main/modes/mfpaint/paint_draw.h b/main/modes/mfpaint/paint_draw.h
    index 8e28a812..69f1b4cd 100644
    --- a/main/modes/mfpaint/paint_draw.h
    +++ b/main/modes/mfpaint/paint_draw.h
    @@ -14,10 +14,6 @@ extern const char toolWheelLoadStr[];
     extern const char dialogOptionCancelStr[];
     extern const char dialogOptionSaveAsStr[];
    
    -extern const uint8_t toolWheelLoadStrLen;
    -extern const uint8_t dialogOptionCancelStrLen;
    -extern const uint8_t dialogOptionSaveAsStrLen;
    -
     // Mostly for PAINT_DIE
     void paintSetupDialog(paintDialog_t dialog);
    
dylwhich commented 6 months ago
  1. I think there's something off in paintGalleryModeButtonCb(). The reminder text says

    A: Load Pause: Delete

    But "A" is pulling up more reminder text and Pause is full-screening the image. That matches the source code, so I think it's the reminder text that's the issue. Should paintGalleryModeButtonCb() be calling imageBrowserButton() fore more of the buttons?

Ah... right, the browser was supposed to be pretty generic, that's why the gallery just draws the controls on top of it. I'll just add a flag to disable displaying the controls in the browser, I guess.

dylwhich commented 6 months ago

I think that's everything? And apologies for the negative hideControls bool, but doing it that way means I only have to change it in one place instead of all the other places since it gets calloc()'d...

AEFeinstein commented 6 months ago

Everything as far as I can tell!