Major menu API change, please review

[a1ex]

Rationale: http://www.magiclantern.fm/forum/index.php?topic=4386
Code: http://bitbucket.org/hudson/magic-lantern/commits/fe375866712147f48a15db87d94acf87e8aff988

Major menu API change - separating content for presentation. Please review it before converting all menu code.

- idea: let display functions sprintf to strings, let menu.c do all drawing
- we will have to convert ALL .display functions to new ones (.update). See the examples from shoot.c.
- convert bmp_printf's to MENU_SET_VALUE (keep the same string logic, remove the fonts, x, y, and feature name).
- override feature name with MENU_SET_NAME (should not needed; don't abuse this)
- replace menu_draw_icon to MENU_SET_ICON (same args, without x,y)
- replace *(int*)priv with CURRENT_VALUE
- set warnings regardless of current value (only look at other settings). e.g. for trap focus, if autofocus is enabled, issue a warning (even if trap focus is off). The menu backend will choose when and how to display the warning.
- MENU_SET_TRUTH_VALUE(value): set this to 1 if the feature is enabled, 0 if disabled (value field grayed out). Default setting: see entry_guess_truth_value.
- MENU_SET_HELP - set a custom help text (not currently used). The passive way is to have a multiline help2 string - a help line for each choice.
- custom_drawing: should provide a way for user code to draw the item, or to draw the entire menu (e.g. for flexinfo or task info).
- .select functions are declared as: static MENU_SELECT_FUNC(name). No changes to current definition.
- .update functions are declared as: static MENU_UPDATE_FUNC(name). They will get called before displaying the item, so you can override things if needed.
- .display functions are still there for now; will be deprecated and removed after all of them are converted to .update functions
- should be extensible (e.g. easy to add things to menu_display_info)
- conversion effort should be minimal (existing string logic, icons and warnings should be straightforward to convert)
- attention: issue warnings even if feature is off (for graying out). Old code only sends warnings if the feature is true; remove those if's.

As this change will require converting all menu code, please review the new API before continuing. Thanks.

[nanomad]

I'll look at the code later but I would rename MENU_SET_TRUTH_VALUE to MENU_SET_ENABLED

[g3gg0]

thanks a lot alex.
this kind of cleanup was absolutely necessary.

yeah, truth -> enabled as it is the best match.

also select_Q should maybe renamed into somewhat more abstract like select_options (preferred) or select_submenu.
will also read into code today, try to port flexinfo menu and give you feedback.

[1%]

Icon control is more straightforward. But all menus need rewrite

[scrax]

yes a lot of menu now have cut string or are out of box and so on. I've changed what was in the pull request so far.
And the new look is coming out great.
What about submenu full page like on the mockup in the other topic?

[1%]

i'd rather it not do full page so you know its a submenu.

[a1ex]

All features from nightly are now using the new menu API.

Advanced features (things not enabled by default) were not converted. Feel free to fix them.

There are still quirks inside (lots of them).

[scrax]

Since there is a lot of work going on here, will be possible with that new menu backend to easily implement a mymenu filled automatically with all items displayed in others menu but hidden so a user can unhide only what he wants here?

[1%]

I tried to fix extended bracketing. Shutter works fine but iso freezes when editing anything other than first iso.

Code: [Select]

static MENU_UPDATE_FUNC (hdrv_extended_shutter_display)
{

   uint8_t *shutter_index = (uint8_t *) hdrv_extended_shutter;
   uint8_t pos = hdrv_extended_step_edit - 1;
   
    if(CURRENT_VALUE)
    {
        MENU_SET_VALUE(
            "Shutter #%d : 1/%d",
            pos + 1,
            hdrv_shutter_table[shutter_index[pos]]
        );
    }
    else
    {
        MENU_SET_VALUE(
            "Shutter #%d : ---",
            pos + 1
        );
    }
   
    if(hdrv_extended_step_edit > hdrv_extended_steps)
    {
         MENU_SET_WARNING(MENU_WARN_NOT_WORKING, "This entry is unused. Increase step count.");
    }
}

static MENU_SELECT_FUNC (hdrv_extended_shutter_toggle)
{
    uint8_t *shutter_index = (uint8_t *)priv;
    uint8_t pos = hdrv_extended_step_edit - 1;
    int new_pos = 0;

    new_pos = COERCE(shutter_index[pos] + delta, 0, COUNT(hdrv_shutter_table) - 1);
    shutter_index[pos] = new_pos;
}

static MENU_SELECT_FUNC  (hdrv_extended_iso_toggle)
{
    uint8_t *iso_table = (uint8_t *)priv;
    uint8_t pos = hdrv_extended_step_edit - 1;
int new_pos = 0;
    do
    {
new_pos = mod(iso_table[pos] - 72 + delta, MAX_ISO_BV - 72 + 1) + 72;       
iso_table[pos] = new_pos;
    }
    while (!is_hdr_valid_iso(raw2iso(iso_table[pos])));
}

static MENU_UPDATE_FUNC (hdrv_extended_iso_display)
{
  //~  uint8_t *iso_table = (uint8_t *)priv;
uint8_t *iso_table = (uint8_t *) hdrv_extended_iso;
// uint8_t *iso_table = (uint8_t *) CURRENT_VALUE;
 
   uint8_t pos = hdrv_extended_step_edit - 1;
    int effective_iso = get_effective_hdr_iso_for_display(iso_table[pos]);
    int d = effective_iso - iso_table[pos];
    d = d * 10 / 8;
   
    if(iso_table[pos])
    {
        if (d)
        {
            MENU_SET_VALUE(
                "ISO #%d: %d (%d, %s%d.%d EV)",
                pos + 1,
                raw2iso(effective_iso),
                raw2iso(iso_table[pos]),
                d > 0 ? "+" : "-", ABS(d)/10, ABS(d)%10
            );
        }
        else
        {
            MENU_SET_VALUE(
                "ISO #%d     : %d",
                pos + 1,
                raw2iso(effective_iso)
            );
        }
    }
    else
    {
        MENU_SET_VALUE(
            "OFF #%d     : ---",
            pos + 1
        );
    }
    if(hdrv_extended_step_edit > hdrv_extended_steps)
    {
        MENU_SET_WARNING(MENU_WARN_NOT_WORKING, "This entry is unused. Increase step count.");
    }
}
#endif


[Marsu42]

i'd rather it not do full page so you know its a submenu.

+1 and just my 2ct: Just tried the new menu & ui (commit 31c81a3) and think the full screen submenus are a major slowdown for navigating ml - I have to re-think "where am I?" every back & forward. Actually it looks like ml is now Windows 8 tile-infected

I see the rationale to gain space for more verbose settings & descriptions, but imho it's too large a regression. As for the rest of the newer ui appeal: Well done, the colors and greyed out option make navigating much easier (until using a submenu that is...).
   

[g3gg0]

Since there is a lot of work going on here, will be possible with that new menu backend to easily implement a mymenu filled automatically with all items displayed in others menu but hidden so a user can unhide only what he wants here?

alex already implemented that

[scrax]

alex already implemented that

yes, after asked

[a1ex]

Can you check the audio menus on 600D, maybe post a screenshot? There is already a bug report.

[scrax]

Can you check the audio menus on 600D, maybe post a screenshot? There is already a bug report.

Yes they are broke I've fixed some thing but you preceded me, now pulling from main to check if changes works

[scrax]

Now audio is not working for 600D, will try to compare changes to fix it but probably in a couple of day

[mkorho]

Now audio is not working for 600D, will try to compare changes to fix it but probably in a couple of day

For me the latest code from repository works just fine. (600D)
Tested internal and external mic.
Not sure if its a bug or i just forgot to remove magic.cfg when testing different versions, the mic setting jumped from auto to internal mic only. 

Only thing that is still broken is in my ticket:
https://bitbucket.org/hudson/magic-lantern/issue/1520/audio-meters

[scrax]

For me the latest code from repository works just fine. (600D)
Tested internal and external mic.
Not sure if its a bug or i just forgot to remove magic.cfg when testing different versions, the mic setting jumped from auto to internal mic only. 

Only thing that is still broken is in my ticket:
https://bitbucket.org/hudson/magic-lantern/issue/1520/audio-meters

Ok, will check nightly, maybe my merge went bad.