Card layout has significant rendering issues, LCD rendering issues too.

[davetcc]

Discussed in https://github.com/davetcc/tcMenu/discussions/307

^{Originally posted by **vzahradnik** February 21, 2023} Hello, I'm wondering whether it would be possible to create a top level menu completely as a slideshow. After clicking on the selected icon, users would see respective submenu. If it's not supported, is it something you'd appreciate as well? Attached are the example designs to help you imagine what I mean.  

[davetcc]

So to formalize the requirements around this, the ItemDisplayPropertiesFactory system already allows for layout control, where menu items can change they way they draw. In this case there would be an entry for all the menu item that should be presented as icons with text.

Any menu item can be presented in this card layout, the title will work as now if needed, and any title widgets would be able to be presented as usual. It is just an alternative layout basically.

There will be a new method on the renderer itself that allows this mode to be selected for some submenus.

[davetcc]

See https://github.com/davetcc/tcMenuLib/issues/187 for the library change.

[vzahradnik]

@davetcc I'd like to reopen this issue. While testing with tcMenu 3.2 master code, I found two issues. Later I will attach a video to better demonstrate what I mean.

[vzahradnik]

Here is the video: https://www.youtube.com/watch?v=l288it2UPF0

Issues:

1. When I hide the top bar, items are shifted by 1, e.g. I don't see Automation icon but when I click on Date and Time, the Automation SubMenu is shown, etc. This occurs only when the top bar is hidden.

// In ThemeMonoBordered.h
bgr.setTitleMode(BaseGraphicalRenderer::TitleMode::NO_TITLE);

2. After powering up the device, not the first SubMenu is selected but the second menu in the list.
3. The pre-selected submenu behaves differently. When I click on the icon, its subitems are shown in the cardview mode instead of the standard layout. Other icons behave as expected.

[davetcc]

another issue in 3.2 for investigation

[davetcc]

For 1 & 2 are you calling setTitleMode before setupMenu is called?

I'd expect that a complete re-evaluation of the drawing list would be required. This probably needs a bit more thought and testing because the title mode was never envisaged to be changed that frequently. It may be that menuMgr.notifyStructureChanged() would be enough but I'm not sure.

[davetcc]

For 3, can you provide the code you're using to enable and configure card mode?

[vzahradnik]

For 1 & 2 are you calling setTitleMode before setupMenu is called?

Most likely yes. I call this code in the ThemeMonoBordered.h where the theme is being set up.

This probably needs a bit more thought and testing.

I agree. Because this is a new feature, my suggestion is that we can postpone this to the 3.3 release. At this point, nobody is affected by this feature and the bug doesn't affect normal menu views.

For 3, can you provide the code you're using to enable and configure card mode?

Sure. Let me put it here.

#ifndef TCMENU_THEME_MONO_BORDER
#define TCMENU_THEME_MONO_BORDER

color_t defaultItemPaletteMono[] = {WHITE, BLACK, WHITE, WHITE};

#define TITLE_BORDER_THICKNESS 2
#define TITLE_SPACING 2

void installMonoBorderedTheme(GraphicsDeviceRenderer& bgr, const MenuFontDef& itemFont, const MenuFontDef& titleFont, bool needEditingIcons) {
    bgr.setDisplayDimensions(bgr.getDeviceDrawable()->getDisplayDimensions().x, bgr.getDeviceDrawable()->getDisplayDimensions().y);
    auto& factory = bgr.getGraphicsPropertiesFactory();

    factory.setSelectedColors(0, 1);

    MenuPadding titlePadding(1);
    MenuPadding itemPadding(1);
    int titleHeight = bgr.heightForFontPadding(titleFont.fontData, titleFont.fontMag, titlePadding);
    int itemHeight = bgr.heightForFontPadding(itemFont.fontData, itemFont.fontMag, itemPadding);

    factory.addImageToCache(DrawableIcon(SPECIAL_ID_EDIT_ICON, Coord(8, 6),DrawableIcon::ICON_XBITMAP, loResEditingIcon));
    factory.addImageToCache(DrawableIcon(SPECIAL_ID_ACTIVE_ICON, Coord(8, 6),DrawableIcon::ICON_XBITMAP, loResActiveIcon));

    factory.setDrawingPropertiesDefault(ItemDisplayProperties::COMPTYPE_TITLE, defaultItemPaletteMono, titlePadding, titleFont.fontData, titleFont.fontMag,
                                        TITLE_SPACING, titleHeight + 1, GridPosition::JUSTIFY_TITLE_LEFT_WITH_VALUE,
                                        MenuBorder(0, 0, TITLE_BORDER_THICKNESS, 0));
    factory.setDrawingPropertiesDefault(ItemDisplayProperties::COMPTYPE_ITEM, defaultItemPaletteMono, itemPadding, itemFont.fontData, itemFont.fontMag,
                                        1, itemHeight, GridPosition::JUSTIFY_TITLE_LEFT_VALUE_RIGHT , MenuBorder(0));
    factory.setDrawingPropertiesDefault(ItemDisplayProperties::COMPTYPE_ACTION, defaultItemPaletteMono, itemPadding, itemFont.fontData, itemFont.fontMag,
                                        1, itemHeight, GridPosition::JUSTIFY_TITLE_LEFT_WITH_VALUE, MenuBorder(0));

    tcgfx::ConfigurableItemDisplayPropertiesFactory::refreshCache();
    bgr.setTitleMode(BaseGraphicalRenderer::TitleMode::NO_TITLE);
}

#endif //TCMENU_THEME_MONO_BORDER

[davetcc]

I suspect if you flip around refreshCache and setTitleMode it will calculate the items properly

[vzahradnik]

I can try. If this fixes the issue, documenting it in an example should be fine.

[vzahradnik]

I suspect if you flip around refreshCache and setTitleMode it will calculate the items properly

It is still broken. I think this one can wait after 3.2 release. Let's keep this open.

[davetcc]

I'm going to look at this now, because I suspect a breaking change will be needed to fix it, I think the way it is using root items to determine what is in card layout is wrong, and we may need to switch to using the submenu (or null for root)

[vzahradnik]

Ok, I can help with testing. Also, if it helps, I could provide the TcMenu project file. There's nothing sensitive.

[davetcc]

The showing of the 2nd item instead of the 1st is fixed. Difficult to track down. Now I'll look at the others.

[davetcc]

See tcMenuLib latest commit (accidentally committed against this Issue number)

[davetcc]

For the other issues, I worked out that you must configure the card layout before calling setupMenu, I'll update the examples. Once I did this, it started working properly for me. This test is working well with a 4way digital joystick on an ESP32S2 for me:

https://github.com/davetcc/tcLibraryDev/blob/master/esp32s2Keyb/esp32s2Keyb_main.cpp

Can you confirm if the latest code and following the above example works for you?

[vzahradnik]

It's still faulty. When I followed your example, this part worked fine. Specifically:

// Called before setupMenu()
void DisplayManager::setUpCardView() {
    static DrawableIcon iconLeft(-1, Coord(7, 12), tcgfx::DrawableIcon::ICON_XBITMAP, ArrowHoriz7x12BitmapLeft, nullptr);
    static DrawableIcon iconRight(-1, Coord(7, 12), tcgfx::DrawableIcon::ICON_XBITMAP, ArrowHoriz7x12BitmapRight, nullptr);

    // Card view
    renderer.enableCardLayout(iconLeft, iconRight, nullptr, true);
    renderer.setCardLayoutStatusForSubMenu(nullptr, true);
    // setupGridLayoutForCardView();
}

The problem shows when I enable code to add icons to the grid menu. You can see, it's commented out above.

Here is the full code of this method:

void DisplayManager::setupGridLayoutForCardView() {
    auto &factory = renderer.getGraphicsPropertiesFactory();

    const Coord iconSize(APPICONS_WIDTH, APPICONS_HEIGHT);
    factory.addImageToCache(DrawableIcon(menuDateAndTime.getId(), iconSize, DrawableIcon::ICON_XBITMAP, dateAndTimeMenuIcon));
    factory.addImageToCache(DrawableIcon(menuAutomation.getId(), iconSize, DrawableIcon::ICON_XBITMAP, automationMenuIcon));
    factory.addImageToCache(DrawableIcon(menuNetwork.getId(), iconSize, DrawableIcon::ICON_XBITMAP, networkMenuIcon));
    factory.addImageToCache(DrawableIcon(menuDisplay.getId(), iconSize, DrawableIcon::ICON_XBITMAP, displayMenuIcon));
    factory.addImageToCache(DrawableIcon(menuServiceMenu.getId(), iconSize, DrawableIcon::ICON_XBITMAP, serviceMenuIcon));

    factory.addGridPosition(&menuDateAndTime, GridPosition(GridPosition::DRAW_AS_ICON_TEXT, GridPosition::JUSTIFY_CENTER_NO_VALUE, 1, 25));
    factory.addGridPosition(&menuAutomation, GridPosition(GridPosition::DRAW_AS_ICON_TEXT, GridPosition::JUSTIFY_CENTER_NO_VALUE, 2, 25));
    factory.addGridPosition(&menuNetwork, GridPosition(GridPosition::DRAW_AS_ICON_TEXT, GridPosition::JUSTIFY_CENTER_NO_VALUE, 3, 25));
    factory.addGridPosition(&menuDisplay, GridPosition(GridPosition::DRAW_AS_ICON_TEXT, GridPosition::JUSTIFY_CENTER_NO_VALUE, 4, 25));
    factory.addGridPosition(&menuServiceMenu, GridPosition(GridPosition::DRAW_AS_ICON_TEXT, GridPosition::JUSTIFY_CENTER_NO_VALUE, 5, 25));

    tcgfx::ConfigurableItemDisplayPropertiesFactory::refreshCache();
}

[davetcc]

Also see other rendering issue here - https://github.com/davetcc/tcMenu/discussions/318

[davetcc]

Also see this one (similar issue I think) - https://github.com/davetcc/tcMenu/issues/324

[tomelgato]

Not sure if this issue is covered by vzahradnik comments already:

After startup i see as first item the project name. When i take over the display and give it back later to tcmenu it still appears as first item. But it disappears if i go into a submenu and back to the main menu.

[vzahradnik]

No, I didn't cover this usecase. I try to avoid taking display over as much as possible. I'd rather code missing feature into TcMenu.

[tomelgato]

When you need to output data on the display you need to take it over, no?

The bug i hit has nothing to do with takeover of the display, it appears as soon as you leave the root node in the menu, go to a submenu and jump back to root.

[vzahradnik]

You need to take over the display only if you want to have full control over what is being rendered on the display. For example, you want to display an image with some rotating text. In all other cases, TcMenu should cover all you need.

I tried to reproduce your issue on my board but it works OK for me. Perhaps you have some extra code which can cause this. No idea...

You could create a backup of your project and try to generate new TcMenu code fresh.

[tomelgato]

I think its based on the error i described in https://github.com/davetcc/tcMenu/issues/308 which Dave referenced to this thread. Thats why i posted the update here ;)

[davetcc]

https://github.com/davetcc/tcMenu/discussions/387

[davetcc]

I'm going to close this, a huge refactoring of the way active item and caching works has been finished just now. Even if issues still exist they will be much easier to track down. Please report anything that remains under #324