mirror of
https://github.com/daveallie/crosspoint-reader.git
synced 2026-02-04 14:47:37 +03:00
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
This commit is contained in:
parent
999e60b75e
commit
93a94ea838
@ -11,6 +11,22 @@
|
||||
|
||||
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 ---
|
||||
class Container : public UIElement {
|
||||
protected:
|
||||
@ -263,8 +279,8 @@ class ProgressBar : public UIElement {
|
||||
std::string valStr = context.evaluatestring(valueExpr);
|
||||
std::string maxStr = context.evaluatestring(maxExpr);
|
||||
|
||||
int value = valStr.empty() ? 0 : std::stoi(valStr);
|
||||
int maxVal = maxStr.empty() ? 100 : std::stoi(maxStr);
|
||||
int value = parseIntSafe(valStr, 0);
|
||||
int maxVal = parseIntSafe(maxStr, 100);
|
||||
if (maxVal <= 0) maxVal = 100;
|
||||
|
||||
float ratio = static_cast<float>(value) / static_cast<float>(maxVal);
|
||||
@ -367,7 +383,7 @@ class BatteryIcon : public UIElement {
|
||||
if (!isVisible(context)) return;
|
||||
|
||||
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);
|
||||
uint8_t color = Color::parse(colStr).value;
|
||||
|
||||
@ -85,7 +85,11 @@ class HStack : public Container {
|
||||
// Add child's own Y offset to the calculated position
|
||||
childY += childYOffset;
|
||||
|
||||
child->layout(context, currentX, childY, childW, childH);
|
||||
// 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;
|
||||
availableW -= (childW + spacing);
|
||||
if (availableW < 0) availableW = 0;
|
||||
@ -168,7 +172,11 @@ class VStack : public Container {
|
||||
// Add child's own X offset to the calculated position
|
||||
childX += childXOffset;
|
||||
|
||||
child->layout(context, childX, currentY, childW, childH);
|
||||
// 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;
|
||||
availableH -= (childH + spacing);
|
||||
if (availableH < 0) availableH = 0;
|
||||
@ -211,8 +219,10 @@ class Grid : public Container {
|
||||
|
||||
if (children.empty()) return;
|
||||
|
||||
int availableW = absW - 2 * padding - (columns - 1) * colSpacing;
|
||||
int cellW = availableW / columns;
|
||||
// Guard against division by zero
|
||||
int cols = columns > 0 ? columns : 1;
|
||||
int availableW = absW - 2 * padding - (cols - 1) * colSpacing;
|
||||
int cellW = availableW / cols;
|
||||
int availableH = absH - 2 * padding;
|
||||
|
||||
int row = 0, col = 0;
|
||||
@ -228,7 +238,7 @@ class Grid : public Container {
|
||||
if (childH > maxRowHeight) maxRowHeight = childH;
|
||||
|
||||
col++;
|
||||
if (col >= columns) {
|
||||
if (col >= cols) {
|
||||
col = 0;
|
||||
row++;
|
||||
currentY += maxRowHeight + rowSpacing;
|
||||
@ -587,15 +597,7 @@ class TabBar : public Container {
|
||||
// Draw selection indicator
|
||||
if (showIndicator && !children.empty()) {
|
||||
std::string selStr = context.evaluatestring(selectedExpr);
|
||||
int selectedIdx = 0;
|
||||
if (!selStr.empty()) {
|
||||
// Try to parse as number
|
||||
try {
|
||||
selectedIdx = std::stoi(selStr);
|
||||
} catch (...) {
|
||||
selectedIdx = 0;
|
||||
}
|
||||
}
|
||||
int selectedIdx = parseIntSafe(selStr, 0);
|
||||
|
||||
if (selectedIdx >= 0 && selectedIdx < static_cast<int>(children.size())) {
|
||||
UIElement* tab = children[selectedIdx];
|
||||
@ -686,9 +688,9 @@ class ScrollIndicator : public UIElement {
|
||||
std::string totalStr = context.evaluatestring(totalExpr);
|
||||
std::string visStr = context.evaluatestring(visibleExpr);
|
||||
|
||||
float position = posStr.empty() ? 0 : std::stof(posStr);
|
||||
int total = totalStr.empty() ? 1 : std::stoi(totalStr);
|
||||
int visible = visStr.empty() ? 1 : std::stoi(visStr);
|
||||
float position = parseFloatSafe(posStr, 0.0f);
|
||||
int total = parseIntSafe(totalStr, 1);
|
||||
int visible = parseIntSafe(visStr, 1);
|
||||
|
||||
if (total <= visible) {
|
||||
// No need to show scrollbar
|
||||
|
||||
@ -26,6 +26,7 @@ struct ScreenCache {
|
||||
uint32_t contextHash = 0; // Hash of context data to detect changes
|
||||
bool valid = false;
|
||||
|
||||
ScreenCache() = default;
|
||||
~ScreenCache() {
|
||||
if (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; }
|
||||
};
|
||||
|
||||
|
||||
@ -126,6 +126,7 @@ void Label::draw(const GfxRenderer& renderer, const ThemeContext& context) {
|
||||
int lineHeight = renderer.getLineHeight(fontId);
|
||||
|
||||
std::vector<std::string> lines;
|
||||
lines.reserve(maxLines); // Pre-allocate to avoid reallocations
|
||||
if (absW > 0 && textWidth > absW && maxLines > 1) {
|
||||
// Logic to wrap text
|
||||
std::string remaining = finalStr;
|
||||
@ -374,6 +375,13 @@ void List::draw(const GfxRenderer& renderer, const ThemeContext& context) {
|
||||
int itemW = getItemWidth();
|
||||
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
|
||||
if (direction == Direction::Horizontal || layoutMode == LayoutMode::Grid) {
|
||||
// Horizontal or Grid layout
|
||||
@ -389,9 +397,16 @@ void List::draw(const GfxRenderer& renderer, const ThemeContext& context) {
|
||||
}
|
||||
|
||||
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
|
||||
ThemeContext itemContext(&context);
|
||||
std::string prefix = source + "." + std::to_string(i) + ".";
|
||||
|
||||
// Standard list item variables - include all properties for full flexibility
|
||||
std::string nameVal = context.getString(prefix + "Name");
|
||||
@ -470,9 +485,16 @@ void List::draw(const GfxRenderer& renderer, const ThemeContext& context) {
|
||||
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
|
||||
ThemeContext itemContext(&context);
|
||||
std::string prefix = source + "." + std::to_string(i) + ".";
|
||||
|
||||
// Standard list item variables - include all properties for full flexibility
|
||||
std::string nameVal = context.getString(prefix + "Name");
|
||||
|
||||
@ -55,9 +55,6 @@ UIElement* ThemeManager::createElement(const std::string& id, const std::string&
|
||||
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) {
|
||||
const auto elemType = elem->getType();
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user