Major menu API change, please review

Started by a1ex, February 03, 2013, 09:19:15 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

a1ex

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

Quote


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
EOS 1100D | EOS 650 (No, I didn't forget the D) | Ye Olde Canon EF Lenses ('87): 50 f/1.8 - 28 f/2.8 - 70-210 f/4 | EF-S 18-55 f/3.5-5.6 | Metz 36 AF-5

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.
Help us with datasheets - Help us with register dumps
magic lantern: 1Magic9991E1eWbGvrsx186GovYCXFbppY, server expenses: [email protected]
ONLY donate for things we have done, not for things you expect!

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?
I'm using ML2.3 for photography with:
EOS 600DML | EOS 400Dplus | EOS 5D MLbeta5- EF 100mm f/2.8 USM Macro  - EF-S 17-85mm f4-5.6 IS USM - EF 70-200mm f/4 L USM - 580EXII - OsX, PS, LR, RawTherapee, LightZone -no video experience-

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?
I'm using ML2.3 for photography with:
EOS 600DML | EOS 400Dplus | EOS 5D MLbeta5- EF 100mm f/2.8 USM Macro  - EF-S 17-85mm f4-5.6 IS USM - EF 70-200mm f/4 L USM - 580EXII - OsX, PS, LR, RawTherapee, LightZone -no video experience-

1%

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


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

Quote from: 1% on February 03, 2013, 04:34:42 PM
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

Quote from: scrax on February 04, 2013, 06:38:49 AM
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 ;)
Help us with datasheets - Help us with register dumps
magic lantern: 1Magic9991E1eWbGvrsx186GovYCXFbppY, server expenses: [email protected]
ONLY donate for things we have done, not for things you expect!

scrax

I'm using ML2.3 for photography with:
EOS 600DML | EOS 400Dplus | EOS 5D MLbeta5- EF 100mm f/2.8 USM Macro  - EF-S 17-85mm f4-5.6 IS USM - EF 70-200mm f/4 L USM - 580EXII - OsX, PS, LR, RawTherapee, LightZone -no video experience-

a1ex

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

scrax

Quote from: a1ex on February 07, 2013, 05:32:25 PM
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
I'm using ML2.3 for photography with:
EOS 600DML | EOS 400Dplus | EOS 5D MLbeta5- EF 100mm f/2.8 USM Macro  - EF-S 17-85mm f4-5.6 IS USM - EF 70-200mm f/4 L USM - 580EXII - OsX, PS, LR, RawTherapee, LightZone -no video experience-

scrax

Now audio is not working for 600D, will try to compare changes to fix it but probably in a couple of day
I'm using ML2.3 for photography with:
EOS 600DML | EOS 400Dplus | EOS 5D MLbeta5- EF 100mm f/2.8 USM Macro  - EF-S 17-85mm f4-5.6 IS USM - EF 70-200mm f/4 L USM - 580EXII - OsX, PS, LR, RawTherapee, LightZone -no video experience-

mkorho

Quote from: scrax on February 08, 2013, 12:09:58 AM
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

Quote from: mkorho on February 08, 2013, 12:39:46 AM
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.
I'm using ML2.3 for photography with:
EOS 600DML | EOS 400Dplus | EOS 5D MLbeta5- EF 100mm f/2.8 USM Macro  - EF-S 17-85mm f4-5.6 IS USM - EF 70-200mm f/4 L USM - 580EXII - OsX, PS, LR, RawTherapee, LightZone -no video experience-