From bce42bb25c94309da8813ce4d04d8a857ae1d4c7 Mon Sep 17 00:00:00 2001 From: Max Paperno Date: Sat, 17 Dec 2016 17:29:26 -0500 Subject: [PATCH] Fix static initialization order problem with Horus GUI (#4141) * [480x272] Fix static initialization order problem with Layout, Theme, & Widget factories. * [480x272] Fix return from drawTimerMode(). * [480x272] Remove use of macros for registered Layout, Widget, & Theme class lists; remove extern declarations (cosmetic). * [480x272] Convert Layout and Theme registry to dynamic lists (still max. of 10 each); Hide indicator arrow when user is at start/end of available widgets in spinner UI; Add some sanity checking to pointer usage in screens_setup; Add an (arbitrary) limit of 20 Widgets; And a line break! :) --- radio/src/gui/480x272/layout.cpp | 24 +++-- radio/src/gui/480x272/layout.h | 8 +- radio/src/gui/480x272/lcd.cpp | 9 +- radio/src/gui/480x272/screens_setup.cpp | 116 +++++++++++++++--------- radio/src/gui/480x272/theme.cpp | 32 ++++--- radio/src/gui/480x272/theme.h | 10 +- radio/src/gui/480x272/widget.cpp | 21 +++-- radio/src/gui/480x272/widget.h | 4 +- 8 files changed, 137 insertions(+), 87 deletions(-) diff --git a/radio/src/gui/480x272/layout.cpp b/radio/src/gui/480x272/layout.cpp index 72a2f4f12..e221e3357 100644 --- a/radio/src/gui/480x272/layout.cpp +++ b/radio/src/gui/480x272/layout.cpp @@ -20,22 +20,26 @@ #include "opentx.h" -const LayoutFactory * registeredLayouts[MAX_REGISTERED_LAYOUTS]; // TODO dynamic -unsigned int countRegisteredLayouts = 0; +std::list & getRegisteredLayouts() +{ + static std::list layouts; + return layouts; +} + void registerLayout(const LayoutFactory * factory) { - if (countRegisteredLayouts < MAX_REGISTERED_LAYOUTS) { + if (getRegisteredLayouts().size() < MAX_REGISTERED_LAYOUTS) { TRACE("register layout %s", factory->getName()); - registeredLayouts[countRegisteredLayouts++] = factory; + getRegisteredLayouts().push_back(factory); } } const LayoutFactory * getLayoutFactory(const char * name) { - for (unsigned int i=0; igetName())) { - return factory; + std::list::const_iterator it = getRegisteredLayouts().cbegin(); + for (; it != getRegisteredLayouts().cend(); ++it) { + if (!strcmp(name, (*it)->getName())) { + return (*it); } } return NULL; @@ -60,8 +64,8 @@ void loadCustomScreens() customScreens[i] = loadLayout(name, &g_model.screenData[i].layoutData); } - if (customScreens[0] == NULL) { - customScreens[0] = registeredLayouts[0]->create(&g_model.screenData[0].layoutData); + if (customScreens[0] == NULL && getRegisteredLayouts().size()) { + customScreens[0] = getRegisteredLayouts().front()->create(&g_model.screenData[0].layoutData); } topbar->load(); diff --git a/radio/src/gui/480x272/layout.h b/radio/src/gui/480x272/layout.h index 00b89e680..5ff0293ae 100644 --- a/radio/src/gui/480x272/layout.h +++ b/radio/src/gui/480x272/layout.h @@ -21,6 +21,7 @@ #ifndef _LAYOUT_H_ #define _LAYOUT_H_ +#include #include "widgets_container.h" #define MAX_LAYOUT_ZONES 10 @@ -113,11 +114,10 @@ class BaseLayoutFactory: public LayoutFactory const ZoneOption * options; }; -#define MAX_REGISTERED_LAYOUTS 10 - -extern unsigned int countRegisteredLayouts; -extern const LayoutFactory * registeredLayouts[MAX_REGISTERED_LAYOUTS]; Layout * loadLayout(const char * name, Layout::PersistentData * persistentData); void loadCustomScreens(); +#define MAX_REGISTERED_LAYOUTS 10 +std::list & getRegisteredLayouts(); + #endif // _LAYOUT_H_ diff --git a/radio/src/gui/480x272/lcd.cpp b/radio/src/gui/480x272/lcd.cpp index b87e1904b..b4f90395d 100644 --- a/radio/src/gui/480x272/lcd.cpp +++ b/radio/src/gui/480x272/lcd.cpp @@ -279,10 +279,13 @@ void drawCurveName(coord_t x, coord_t y, int8_t idx, LcdFlags flags) void drawTimerMode(coord_t x, coord_t y, int32_t mode, LcdFlags att) { if (mode >= 0) { - if (mode < TMRMODE_COUNT) - return lcdDrawTextAtIndex(x, y, STR_VTMRMODES, mode, att); - else + if (mode < TMRMODE_COUNT) { + lcdDrawTextAtIndex(x, y, STR_VTMRMODES, mode, att); + return; + } + else { mode -= (TMRMODE_COUNT-1); + } } drawSwitch(x, y, mode, att); } diff --git a/radio/src/gui/480x272/screens_setup.cpp b/radio/src/gui/480x272/screens_setup.cpp index 4d8b4f1f3..8811c610c 100644 --- a/radio/src/gui/480x272/screens_setup.cpp +++ b/radio/src/gui/480x272/screens_setup.cpp @@ -209,9 +209,9 @@ bool menuWidgetSettings(event_t event) bool menuWidgetChoice(event_t event) { - static Widget * previousWidget; - static Widget * currentWidget; - static std::list::iterator iterator; + static Widget * previousWidget = NULL; + static Widget * currentWidget = NULL; + static std::list::const_iterator iterator; static Widget::PersistentData tempData; switch (event) { @@ -219,45 +219,55 @@ bool menuWidgetChoice(event_t event) { previousWidget = currentContainer->getWidget(currentZone); currentContainer->setWidget(currentZone, NULL); - iterator = registeredWidgets.begin(); + iterator = getRegisteredWidgets().cbegin(); if (previousWidget) { const WidgetFactory * factory = previousWidget->getFactory(); - for (std::list::iterator it = registeredWidgets.begin(); it != registeredWidgets.end(); ++it) { + std::list::const_iterator it = getRegisteredWidgets().cbegin(); + for (; it != getRegisteredWidgets().cend(); ++it) { if (factory->getName() == (*it)->getName()) { iterator = it; break; } } } - currentWidget = (*iterator)->create(currentContainer->getZone(currentZone), &tempData); + if (iterator != getRegisteredWidgets().cend()) + currentWidget = (*iterator)->create(currentContainer->getZone(currentZone), &tempData); break; } case EVT_KEY_FIRST(KEY_EXIT): - delete currentWidget; - currentContainer->setWidget(currentZone, previousWidget); + if (previousWidget) { + if (currentWidget) + delete currentWidget; + currentContainer->setWidget(currentZone, previousWidget); + } popMenu(); return false; case EVT_KEY_FIRST(KEY_ENTER): - delete previousWidget; - currentContainer->createWidget(currentZone, *iterator); - storageDirty(EE_MODEL); + if (iterator != getRegisteredWidgets().cend()) { + if (previousWidget) + delete previousWidget; + currentContainer->createWidget(currentZone, *iterator); + storageDirty(EE_MODEL); + } popMenu(); return false; case EVT_ROTARY_RIGHT: - if (iterator != registeredWidgets.end() && iterator != --registeredWidgets.end()) { + if (iterator != getRegisteredWidgets().cend() && iterator != --getRegisteredWidgets().cend()) { ++iterator; - delete currentWidget; + if (currentWidget) + delete currentWidget; currentWidget = (*iterator)->create(currentContainer->getZone(currentZone), &tempData); } break; case EVT_ROTARY_LEFT: - if (iterator != registeredWidgets.begin()) { + if (iterator != getRegisteredWidgets().cbegin()) { --iterator; - delete currentWidget; + if (currentWidget) + delete currentWidget; currentWidget = (*iterator)->create(currentContainer->getZone(currentZone), &tempData); } break; @@ -271,13 +281,19 @@ bool menuWidgetChoice(event_t event) lcdDrawFilledRect(zone.x-2, 0, zone.w+4, zone.y-2, SOLID, OVERLAY_COLOR | (8<<24)); lcdDrawFilledRect(zone.x-2, zone.y+zone.h+2, zone.w+4, LCD_H-zone.y-zone.h-2, SOLID, OVERLAY_COLOR | (8<<24)); - currentWidget->refresh(); + if (currentWidget) + currentWidget->refresh(); - lcdDrawBitmapPattern(zone.x-10, zone.y+zone.h/2-10, LBM_SWIPE_CIRCLE, TEXT_INVERTED_BGCOLOR); - lcdDrawBitmapPattern(zone.x-10, zone.y+zone.h/2-10, LBM_SWIPE_LEFT, TEXT_INVERTED_COLOR); - lcdDrawBitmapPattern(zone.x+zone.w-9, zone.y+zone.h/2-10, LBM_SWIPE_CIRCLE, TEXT_INVERTED_BGCOLOR); - lcdDrawBitmapPattern(zone.x+zone.w-9, zone.y+zone.h/2-10, LBM_SWIPE_RIGHT, TEXT_INVERTED_COLOR); - lcdDrawText(zone.x + zone.w, zone.y-1, currentWidget->getFactory()->getName(), RIGHT | TEXT_COLOR | SMLSIZE | INVERS); + if (iterator != getRegisteredWidgets().cbegin()) { + lcdDrawBitmapPattern(zone.x-10, zone.y+zone.h/2-10, LBM_SWIPE_CIRCLE, TEXT_INVERTED_BGCOLOR); + lcdDrawBitmapPattern(zone.x-10, zone.y+zone.h/2-10, LBM_SWIPE_LEFT, TEXT_INVERTED_COLOR); + } + if (iterator != --getRegisteredWidgets().cend()) { + lcdDrawBitmapPattern(zone.x+zone.w-9, zone.y+zone.h/2-10, LBM_SWIPE_CIRCLE, TEXT_INVERTED_BGCOLOR); + lcdDrawBitmapPattern(zone.x+zone.w-9, zone.y+zone.h/2-10, LBM_SWIPE_RIGHT, TEXT_INVERTED_COLOR); + } + if (currentWidget) + lcdDrawText(zone.x + zone.w, zone.y-1, currentWidget->getFactory()->getName(), RIGHT | TEXT_COLOR | SMLSIZE | INVERS); return true; } @@ -351,16 +367,20 @@ bool menuWidgetsSetup(event_t event) } template -T * editThemeChoice(coord_t x, coord_t y, T * array[], uint8_t count, T * current, bool needsOffsetCheck, LcdFlags attr, event_t event) +T * editThemeChoice(coord_t x, coord_t y, std::list & elList, T * current, bool needsOffsetCheck, LcdFlags attr, event_t event) { - static uint8_t menuHorizontalOffset; - + static uint8_t menuHorizontalOffset = 0; + uint8_t elCount = elList.size(), last, idx; + coord_t pos; int currentIndex = 0; - for (unsigned int i=0; i::const_iterator elItr = elList.cbegin(); + + if (!elCount) + return NULL; + + for (; elItr != elList.cend(); ++elItr, ++currentIndex) { + if ((*elItr) == current) break; - } } if (event == EVT_ENTRY) { @@ -378,7 +398,7 @@ T * editThemeChoice(coord_t x, coord_t y, T * array[], uint8_t count, T * curren } if (attr) { if (menuHorizontalPosition < 0) { - lcdDrawSolidFilledRect(x-3, y-1, min(4, count)*56+1, 2*FH-5, TEXT_INVERTED_BGCOLOR); + lcdDrawSolidFilledRect(x-3, y-1, min(4, elCount)*56+1, 2*FH-5, TEXT_INVERTED_BGCOLOR); } else { if (needsOffsetCheck) { @@ -392,20 +412,29 @@ T * editThemeChoice(coord_t x, coord_t y, T * array[], uint8_t count, T * curren } } } - unsigned int last = min(menuHorizontalOffset + 4, count); - for (unsigned int i=menuHorizontalOffset, pos=x; idrawThumb(pos, y+1, current == element ? ((attr && menuHorizontalPosition < 0) ? TEXT_INVERTED_COLOR : TEXT_INVERTED_BGCOLOR) : LINE_COLOR); + last = min(menuHorizontalOffset + 4, elCount); + idx = menuHorizontalOffset; + pos = x; + elItr = elList.cbegin(); + std::advance(elItr, min(menuHorizontalOffset, elCount - 1)); + for (; idx < last && elItr != elList.cend(); ++idx, ++elItr, pos += 56) { + (*elItr)->drawThumb(pos, y+1, current == (*elItr) ? ((attr && menuHorizontalPosition < 0) ? TEXT_INVERTED_COLOR : TEXT_INVERTED_BGCOLOR) : LINE_COLOR); } - if (count > 4) { + if (elCount > 4) { lcdDrawBitmapPattern(x - 12, y+1, LBM_CARROUSSEL_LEFT, menuHorizontalOffset > 0 ? LINE_COLOR : CURVE_AXIS_COLOR); - lcdDrawBitmapPattern(x + 4 * 56, y+1, LBM_CARROUSSEL_RIGHT, last < countRegisteredLayouts ? LINE_COLOR : CURVE_AXIS_COLOR); + lcdDrawBitmapPattern(x + 4 * 56, y+1, LBM_CARROUSSEL_RIGHT, last < getRegisteredLayouts().size() ? LINE_COLOR : CURVE_AXIS_COLOR); } if (attr && menuHorizontalPosition >= 0) { lcdDrawSolidRect(x + (menuHorizontalPosition - menuHorizontalOffset) * 56 - 3, y - 1, 57, 35, 1, TEXT_INVERTED_BGCOLOR); if (event == EVT_KEY_FIRST(KEY_ENTER)) { s_editMode = 0; - return array[menuHorizontalPosition]; + elItr = elList.cbegin(); + if (menuHorizontalPosition < (int)elList.size()) + std::advance(elItr, menuHorizontalPosition); + if (elItr != elList.cend()) + return (*elItr); + else + return NULL; } } return NULL; @@ -424,7 +453,7 @@ bool menuScreensTheme(event_t event) linesCount = ITEM_SCREEN_SETUP_THEME_OPTION1 + optionsCount + 1; menuPageCount = updateMainviewsMenu(); - uint8_t mstate_tab[2 + MAX_THEME_OPTIONS + 1] = { uint8_t(NAVIGATION_LINE_BY_LINE|uint8_t(countRegisteredThemes-1)), ORPHAN_ROW }; + uint8_t mstate_tab[2 + MAX_THEME_OPTIONS + 1] = { uint8_t(NAVIGATION_LINE_BY_LINE | uint8_t(getRegisteredThemes().size()-1)), ORPHAN_ROW }; for (int i=0; i(SCREENS_SETUP_2ND_COLUMN, y, registeredThemes, countRegisteredThemes, theme, needsOffsetCheck, attr, event); + Theme * new_theme = editThemeChoice(SCREENS_SETUP_2ND_COLUMN, y, getRegisteredThemes(), theme, needsOffsetCheck, attr, event); if (new_theme) { new_theme->init(); loadTheme(new_theme); @@ -506,7 +535,7 @@ bool menuScreenSetup(int index, event_t event) if (menuPageCount > 3) ++linesCount; - uint8_t mstate_tab[2 + MAX_LAYOUT_OPTIONS + 1] = { uint8_t(NAVIGATION_LINE_BY_LINE|uint8_t(countRegisteredLayouts-1)), ORPHAN_ROW }; + uint8_t mstate_tab[2 + MAX_LAYOUT_OPTIONS + 1] = { uint8_t(NAVIGATION_LINE_BY_LINE | uint8_t(getRegisteredLayouts().size()-1)), ORPHAN_ROW }; for (int i=0; i(SCREENS_SETUP_2ND_COLUMN, y, registeredLayouts, countRegisteredLayouts, currentScreen->getFactory(), needsOffsetCheck, attr, event); + const LayoutFactory * factory = editThemeChoice(SCREENS_SETUP_2ND_COLUMN, y, getRegisteredLayouts(), currentScreen->getFactory(), needsOffsetCheck, attr, event); if (factory) { delete customScreens[index]; currentScreen = customScreens[index] = factory->create(&g_model.screenData[index].layoutData); @@ -610,9 +639,10 @@ bool menuScreenAdd(event_t event) { menuPageCount = updateMainviewsMenu(); - if (event == EVT_KEY_FIRST(KEY_ENTER)) { - customScreens[menuPageCount-2] = registeredLayouts[0]->create(&g_model.screenData[menuPageCount-2].layoutData); - strncpy(g_model.screenData[menuPageCount-2].layoutName, registeredLayouts[0]->getName(), sizeof(g_model.screenData[menuPageCount-2].layoutName)); + if (event == EVT_KEY_FIRST(KEY_ENTER) && getRegisteredLayouts().size()) { + const LayoutFactory * lf = getRegisteredLayouts().front(); + customScreens[menuPageCount-2] = lf->create(&g_model.screenData[menuPageCount-2].layoutData); + strncpy(g_model.screenData[menuPageCount-2].layoutName, lf->getName(), sizeof(g_model.screenData[menuPageCount-2].layoutName)); s_editMode = 0; menuHorizontalPosition = -1; killEvents(KEY_ENTER); diff --git a/radio/src/gui/480x272/theme.cpp b/radio/src/gui/480x272/theme.cpp index 4a4affed7..2fccc8ab5 100644 --- a/radio/src/gui/480x272/theme.cpp +++ b/radio/src/gui/480x272/theme.cpp @@ -24,6 +24,20 @@ const BitmapBuffer * Theme::asterisk = NULL; const BitmapBuffer * Theme::question = NULL; const BitmapBuffer * Theme::busy = NULL; +std::list & getRegisteredThemes() +{ + static std::list themes; + return themes; +} + +void registerTheme(Theme * theme) +{ + if (getRegisteredThemes().size() < MAX_REGISTERED_THEMES) { + TRACE("register theme %s", theme->getName()); + getRegisteredThemes().push_back(theme); + } +} + void Theme::init() const { memset(&g_eeGeneral.themeData, 0, sizeof(Theme::PersistentData)); @@ -112,22 +126,12 @@ void Theme::drawMessageBox(const char * title, const char * text, const char * a } } -Theme * registeredThemes[MAX_REGISTERED_THEMES]; // TODO dynamic -unsigned int countRegisteredThemes = 0; -void registerTheme(Theme * theme) -{ - if (countRegisteredThemes < MAX_REGISTERED_THEMES) { - TRACE("register theme %s", theme->getName()); - registeredThemes[countRegisteredThemes++] = theme; - } -} - Theme * getTheme(const char * name) { - for (unsigned int i=0; igetName())) { - return theme; + std::list::const_iterator it = getRegisteredThemes().cbegin(); + for (; it != getRegisteredThemes().cend(); ++it) { + if (!strcmp(name, (*it)->getName())) { + return (*it); } } return NULL; diff --git a/radio/src/gui/480x272/theme.h b/radio/src/gui/480x272/theme.h index e5c93420e..0c8849a85 100644 --- a/radio/src/gui/480x272/theme.h +++ b/radio/src/gui/480x272/theme.h @@ -21,6 +21,8 @@ #ifndef _THEME_H_ #define _THEME_H_ +#include + class BitmapBuffer; #define MAX_THEME_OPTIONS 5 @@ -91,13 +93,11 @@ inline const char * getThemePath(const char * filename) return theme->getFilePath(filename); } -#define MAX_REGISTERED_THEMES 10 -extern unsigned int countRegisteredThemes; -void registerTheme(Theme * theme); -extern Theme * registeredThemes[MAX_REGISTERED_THEMES]; // TODO dynamic - Theme * getTheme(const char * name); void loadTheme(Theme * theme); void loadTheme(); +#define MAX_REGISTERED_THEMES 10 +std::list & getRegisteredThemes(); + #endif // _THEME_H_ diff --git a/radio/src/gui/480x272/widget.cpp b/radio/src/gui/480x272/widget.cpp index 04f9e7c59..cc62b51b4 100644 --- a/radio/src/gui/480x272/widget.cpp +++ b/radio/src/gui/480x272/widget.cpp @@ -20,19 +20,26 @@ #include "opentx.h" -std::list registeredWidgets; +std::list & getRegisteredWidgets() +{ + static std::list widgets; + return widgets; +} + void registerWidget(const WidgetFactory * factory) { - TRACE("register widget %s", factory->getName()); - registeredWidgets.push_back(factory); + if (getRegisteredWidgets().size() < MAX_REGISTERED_WIDGETS) { + TRACE("register widget %s", factory->getName()); + getRegisteredWidgets().push_back(factory); + } } const WidgetFactory * getWidgetFactory(const char * name) { - for (std::list::iterator it = registeredWidgets.begin(); it != registeredWidgets.end();++it) { - const WidgetFactory * factory = *it; - if (!strcmp(name, factory->getName())) { - return factory; + std::list::const_iterator it = getRegisteredWidgets().cbegin(); + for (; it != getRegisteredWidgets().cend();++it) { + if (!strcmp(name, (*it)->getName())) { + return (*it); } } return NULL; diff --git a/radio/src/gui/480x272/widget.h b/radio/src/gui/480x272/widget.h index a86aba272..1fc699b85 100644 --- a/radio/src/gui/480x272/widget.h +++ b/radio/src/gui/480x272/widget.h @@ -186,7 +186,9 @@ inline const ZoneOption * Widget::getOptions() const return getFactory()->getOptions(); } -extern std::list registeredWidgets; Widget * loadWidget(const char * name, const Zone & zone, Widget::PersistentData * persistentData); +#define MAX_REGISTERED_WIDGETS 20 // TODO: arbitrary limit, not checked +std::list & getRegisteredWidgets(); + #endif // _WIDGET_H_