Compare commits

...

2 Commits

Author SHA1 Message Date
Brackyt
1d357629d7 feat: bug fixes and optimizations
- fix double-free
- safe int/float parse
- grid division guard
- list string allocations
- label vector reserve
- skip second layout pass for hstack/vstack
2026-01-28 23:54:54 +01:00
Brackyt
b02617a1ce feat: add valign and halign to stack elements 2026-01-28 23:32:04 +01:00
5 changed files with 174 additions and 42 deletions

View File

@ -11,6 +11,22 @@
namespace ThemeEngine { namespace ThemeEngine {
// Safe integer parsing (no exceptions)
inline int parseIntSafe(const std::string& s, int defaultVal = 0) {
if (s.empty()) return defaultVal;
char* end;
long val = strtol(s.c_str(), &end, 10);
return (end != s.c_str()) ? static_cast<int>(val) : defaultVal;
}
// Safe float parsing (no exceptions)
inline float parseFloatSafe(const std::string& s, float defaultVal = 0.0f) {
if (s.empty()) return defaultVal;
char* end;
float val = strtof(s.c_str(), &end);
return (end != s.c_str()) ? val : defaultVal;
}
// --- Container --- // --- Container ---
class Container : public UIElement { class Container : public UIElement {
protected: protected:
@ -263,8 +279,8 @@ class ProgressBar : public UIElement {
std::string valStr = context.evaluatestring(valueExpr); std::string valStr = context.evaluatestring(valueExpr);
std::string maxStr = context.evaluatestring(maxExpr); std::string maxStr = context.evaluatestring(maxExpr);
int value = valStr.empty() ? 0 : std::stoi(valStr); int value = parseIntSafe(valStr, 0);
int maxVal = maxStr.empty() ? 100 : std::stoi(maxStr); int maxVal = parseIntSafe(maxStr, 100);
if (maxVal <= 0) maxVal = 100; if (maxVal <= 0) maxVal = 100;
float ratio = static_cast<float>(value) / static_cast<float>(maxVal); float ratio = static_cast<float>(value) / static_cast<float>(maxVal);
@ -367,7 +383,7 @@ class BatteryIcon : public UIElement {
if (!isVisible(context)) return; if (!isVisible(context)) return;
std::string valStr = context.evaluatestring(valueExpr); std::string valStr = context.evaluatestring(valueExpr);
int percentage = valStr.empty() ? 0 : std::stoi(valStr); int percentage = parseIntSafe(valStr, 0);
std::string colStr = context.evaluatestring(colorExpr); std::string colStr = context.evaluatestring(colorExpr);
uint8_t color = Color::parse(colStr).value; uint8_t color = Color::parse(colStr).value;

View File

@ -12,9 +12,13 @@ namespace ThemeEngine {
// --- HStack: Horizontal Stack Layout --- // --- HStack: Horizontal Stack Layout ---
// Children are arranged horizontally with optional spacing // Children are arranged horizontally with optional spacing
class HStack : public Container { class HStack : public Container {
public:
enum class VAlign { Top, Center, Bottom };
private:
int spacing = 0; // Gap between children int spacing = 0; // Gap between children
int padding = 0; // Internal padding int padding = 0; // Internal padding
bool centerVertical = false; VAlign vAlign = VAlign::Top;
public: public:
HStack(const std::string& id) : Container(id) {} HStack(const std::string& id) : Container(id) {}
@ -29,8 +33,18 @@ class HStack : public Container {
padding = p; padding = p;
markDirty(); markDirty();
} }
void setCenterVertical(bool c) { void setVAlign(VAlign a) {
centerVertical = c; vAlign = a;
markDirty();
}
void setVAlignFromString(const std::string& s) {
if (s == "center" || s == "Center") {
vAlign = VAlign::Center;
} else if (s == "bottom" || s == "Bottom") {
vAlign = VAlign::Bottom;
} else {
vAlign = VAlign::Top;
}
markDirty(); markDirty();
} }
@ -48,13 +62,34 @@ class HStack : public Container {
int childW = child->getAbsW(); int childW = child->getAbsW();
int childH = child->getAbsH(); int childH = child->getAbsH();
// Re-layout with proper position // Extract child's own Y offset (from first layout pass)
int childYOffset = child->getAbsY() - (absY + padding);
// Calculate base position based on vertical alignment
int childY = absY + padding; int childY = absY + padding;
if (centerVertical && childH < availableH) { if (childH < availableH) {
childY = absY + padding + (availableH - childH) / 2; switch (vAlign) {
case VAlign::Center:
childY = absY + padding + (availableH - childH) / 2;
break;
case VAlign::Bottom:
childY = absY + padding + (availableH - childH);
break;
case VAlign::Top:
default:
childY = absY + padding;
break;
}
} }
child->layout(context, currentX, childY, childW, childH); // Add child's own Y offset to the calculated position
childY += childYOffset;
// Only do second layout pass if position changed from first pass
int firstPassY = child->getAbsY();
if (childY != firstPassY) {
child->layout(context, currentX, childY, childW, childH);
}
currentX += childW + spacing; currentX += childW + spacing;
availableW -= (childW + spacing); availableW -= (childW + spacing);
if (availableW < 0) availableW = 0; if (availableW < 0) availableW = 0;
@ -65,9 +100,13 @@ class HStack : public Container {
// --- VStack: Vertical Stack Layout --- // --- VStack: Vertical Stack Layout ---
// Children are arranged vertically with optional spacing // Children are arranged vertically with optional spacing
class VStack : public Container { class VStack : public Container {
public:
enum class HAlign { Left, Center, Right };
private:
int spacing = 0; int spacing = 0;
int padding = 0; int padding = 0;
bool centerHorizontal = false; HAlign hAlign = HAlign::Left;
public: public:
VStack(const std::string& id) : Container(id) {} VStack(const std::string& id) : Container(id) {}
@ -82,8 +121,18 @@ class VStack : public Container {
padding = p; padding = p;
markDirty(); markDirty();
} }
void setCenterHorizontal(bool c) { void setHAlign(HAlign a) {
centerHorizontal = c; hAlign = a;
markDirty();
}
void setHAlignFromString(const std::string& s) {
if (s == "center" || s == "Center") {
hAlign = HAlign::Center;
} else if (s == "right" || s == "Right") {
hAlign = HAlign::Right;
} else {
hAlign = HAlign::Left;
}
markDirty(); markDirty();
} }
@ -100,12 +149,34 @@ class VStack : public Container {
int childW = child->getAbsW(); int childW = child->getAbsW();
int childH = child->getAbsH(); int childH = child->getAbsH();
// Extract child's own X offset (from first layout pass)
int childXOffset = child->getAbsX() - (absX + padding);
// Calculate base position based on horizontal alignment
int childX = absX + padding; int childX = absX + padding;
if (centerHorizontal && childW < availableW) { if (childW < availableW) {
childX = absX + padding + (availableW - childW) / 2; switch (hAlign) {
case HAlign::Center:
childX = absX + padding + (availableW - childW) / 2;
break;
case HAlign::Right:
childX = absX + padding + (availableW - childW);
break;
case HAlign::Left:
default:
childX = absX + padding;
break;
}
} }
child->layout(context, childX, currentY, childW, childH); // Add child's own X offset to the calculated position
childX += childXOffset;
// Only do second layout pass if position changed from first pass
int firstPassX = child->getAbsX();
if (childX != firstPassX) {
child->layout(context, childX, currentY, childW, childH);
}
currentY += childH + spacing; currentY += childH + spacing;
availableH -= (childH + spacing); availableH -= (childH + spacing);
if (availableH < 0) availableH = 0; if (availableH < 0) availableH = 0;
@ -148,8 +219,10 @@ class Grid : public Container {
if (children.empty()) return; if (children.empty()) return;
int availableW = absW - 2 * padding - (columns - 1) * colSpacing; // Guard against division by zero
int cellW = availableW / columns; int cols = columns > 0 ? columns : 1;
int availableW = absW - 2 * padding - (cols - 1) * colSpacing;
int cellW = availableW / cols;
int availableH = absH - 2 * padding; int availableH = absH - 2 * padding;
int row = 0, col = 0; int row = 0, col = 0;
@ -165,7 +238,7 @@ class Grid : public Container {
if (childH > maxRowHeight) maxRowHeight = childH; if (childH > maxRowHeight) maxRowHeight = childH;
col++; col++;
if (col >= columns) { if (col >= cols) {
col = 0; col = 0;
row++; row++;
currentY += maxRowHeight + rowSpacing; currentY += maxRowHeight + rowSpacing;
@ -524,15 +597,7 @@ class TabBar : public Container {
// Draw selection indicator // Draw selection indicator
if (showIndicator && !children.empty()) { if (showIndicator && !children.empty()) {
std::string selStr = context.evaluatestring(selectedExpr); std::string selStr = context.evaluatestring(selectedExpr);
int selectedIdx = 0; int selectedIdx = parseIntSafe(selStr, 0);
if (!selStr.empty()) {
// Try to parse as number
try {
selectedIdx = std::stoi(selStr);
} catch (...) {
selectedIdx = 0;
}
}
if (selectedIdx >= 0 && selectedIdx < static_cast<int>(children.size())) { if (selectedIdx >= 0 && selectedIdx < static_cast<int>(children.size())) {
UIElement* tab = children[selectedIdx]; UIElement* tab = children[selectedIdx];
@ -623,9 +688,9 @@ class ScrollIndicator : public UIElement {
std::string totalStr = context.evaluatestring(totalExpr); std::string totalStr = context.evaluatestring(totalExpr);
std::string visStr = context.evaluatestring(visibleExpr); std::string visStr = context.evaluatestring(visibleExpr);
float position = posStr.empty() ? 0 : std::stof(posStr); float position = parseFloatSafe(posStr, 0.0f);
int total = totalStr.empty() ? 1 : std::stoi(totalStr); int total = parseIntSafe(totalStr, 1);
int visible = visStr.empty() ? 1 : std::stoi(visStr); int visible = parseIntSafe(visStr, 1);
if (total <= visible) { if (total <= visible) {
// No need to show scrollbar // No need to show scrollbar

View File

@ -26,6 +26,7 @@ struct ScreenCache {
uint32_t contextHash = 0; // Hash of context data to detect changes uint32_t contextHash = 0; // Hash of context data to detect changes
bool valid = false; bool valid = false;
ScreenCache() = default;
~ScreenCache() { ~ScreenCache() {
if (buffer) { if (buffer) {
free(buffer); free(buffer);
@ -33,6 +34,37 @@ struct ScreenCache {
} }
} }
// Prevent double-free from copy
ScreenCache(const ScreenCache&) = delete;
ScreenCache& operator=(const ScreenCache&) = delete;
// Allow move
ScreenCache(ScreenCache&& other) noexcept
: buffer(other.buffer),
bufferSize(other.bufferSize),
screenName(std::move(other.screenName)),
contextHash(other.contextHash),
valid(other.valid) {
other.buffer = nullptr;
other.bufferSize = 0;
other.valid = false;
}
ScreenCache& operator=(ScreenCache&& other) noexcept {
if (this != &other) {
if (buffer) free(buffer);
buffer = other.buffer;
bufferSize = other.bufferSize;
screenName = std::move(other.screenName);
contextHash = other.contextHash;
valid = other.valid;
other.buffer = nullptr;
other.bufferSize = 0;
other.valid = false;
}
return *this;
}
void invalidate() { valid = false; } void invalidate() { valid = false; }
}; };

View File

@ -126,6 +126,7 @@ void Label::draw(const GfxRenderer& renderer, const ThemeContext& context) {
int lineHeight = renderer.getLineHeight(fontId); int lineHeight = renderer.getLineHeight(fontId);
std::vector<std::string> lines; std::vector<std::string> lines;
lines.reserve(maxLines); // Pre-allocate to avoid reallocations
if (absW > 0 && textWidth > absW && maxLines > 1) { if (absW > 0 && textWidth > absW && maxLines > 1) {
// Logic to wrap text // Logic to wrap text
std::string remaining = finalStr; std::string remaining = finalStr;
@ -374,6 +375,13 @@ void List::draw(const GfxRenderer& renderer, const ThemeContext& context) {
int itemW = getItemWidth(); int itemW = getItemWidth();
int itemH = getItemHeight(); int itemH = getItemHeight();
// Pre-allocate string buffers to avoid repeated allocations
std::string prefix;
prefix.reserve(source.length() + 16);
std::string key;
key.reserve(source.length() + 32);
char numBuf[12];
// Handle different layout modes // Handle different layout modes
if (direction == Direction::Horizontal || layoutMode == LayoutMode::Grid) { if (direction == Direction::Horizontal || layoutMode == LayoutMode::Grid) {
// Horizontal or Grid layout // Horizontal or Grid layout
@ -382,16 +390,23 @@ void List::draw(const GfxRenderer& renderer, const ThemeContext& context) {
int currentX = absX; int currentX = absX;
int currentY = absY; int currentY = absY;
// For grid, calculate item width based on columns // For grid, calculate item width based on columns only if not explicitly set
if (layoutMode == LayoutMode::Grid && columns > 1) { if (layoutMode == LayoutMode::Grid && columns > 1 && itemWidth == 0) {
int totalSpacing = (columns - 1) * spacing; int totalSpacing = (columns - 1) * spacing;
itemW = (absW - totalSpacing) / columns; itemW = (absW - totalSpacing) / columns;
} }
for (int i = 0; i < count; ++i) { for (int i = 0; i < count; ++i) {
// Build prefix efficiently: "source.i."
prefix.clear();
prefix += source;
prefix += '.';
snprintf(numBuf, sizeof(numBuf), "%d", i);
prefix += numBuf;
prefix += '.';
// Create item context with scoped variables // Create item context with scoped variables
ThemeContext itemContext(&context); ThemeContext itemContext(&context);
std::string prefix = source + "." + std::to_string(i) + ".";
// Standard list item variables - include all properties for full flexibility // Standard list item variables - include all properties for full flexibility
std::string nameVal = context.getString(prefix + "Name"); std::string nameVal = context.getString(prefix + "Name");
@ -470,9 +485,16 @@ void List::draw(const GfxRenderer& renderer, const ThemeContext& context) {
break; break;
} }
// Build prefix efficiently: "source.i."
prefix.clear();
prefix += source;
prefix += '.';
snprintf(numBuf, sizeof(numBuf), "%d", i);
prefix += numBuf;
prefix += '.';
// Create item context with scoped variables // Create item context with scoped variables
ThemeContext itemContext(&context); ThemeContext itemContext(&context);
std::string prefix = source + "." + std::to_string(i) + ".";
// Standard list item variables - include all properties for full flexibility // Standard list item variables - include all properties for full flexibility
std::string nameVal = context.getString(prefix + "Name"); std::string nameVal = context.getString(prefix + "Name");

View File

@ -55,9 +55,6 @@ UIElement* ThemeManager::createElement(const std::string& id, const std::string&
return nullptr; return nullptr;
} }
// Parse integer safely - returns 0 on error
static int parseIntSafe(const std::string& val) { return static_cast<int>(std::strtol(val.c_str(), nullptr, 10)); }
void ThemeManager::applyProperties(UIElement* elem, const std::map<std::string, std::string>& props) { void ThemeManager::applyProperties(UIElement* elem, const std::map<std::string, std::string>& props) {
const auto elemType = elem->getType(); const auto elemType = elem->getType();
@ -144,13 +141,13 @@ void ThemeManager::applyProperties(UIElement* elem, const std::map<std::string,
} else if (elemType == UIElement::ElementType::List) { } else if (elemType == UIElement::ElementType::List) {
static_cast<List*>(elem)->setSpacing(parseIntSafe(val)); static_cast<List*>(elem)->setSpacing(parseIntSafe(val));
} }
} else if (key == "CenterVertical") { } else if (key == "VAlign") {
if (elemType == UIElement::ElementType::HStack) { if (elemType == UIElement::ElementType::HStack) {
static_cast<HStack*>(elem)->setCenterVertical(val == "true" || val == "1"); static_cast<HStack*>(elem)->setVAlignFromString(val);
} }
} else if (key == "CenterHorizontal") { } else if (key == "HAlign") {
if (elemType == UIElement::ElementType::VStack) { if (elemType == UIElement::ElementType::VStack) {
static_cast<VStack*>(elem)->setCenterHorizontal(val == "true" || val == "1"); static_cast<VStack*>(elem)->setHAlignFromString(val);
} }
} }