From 794e4aa3d829a224d6bb39eb2d62cf502d332a90 Mon Sep 17 00:00:00 2001 From: Leonhardmaster2 Date: Sat, 5 Sep 2026 11:05:57 +0200 Subject: [PATCH] fix(qt): blend the industrial rows in and decline unbounded ranges Follow-up to the review on #47. Unbounded sliders were broken. can_render() accepted any range where max exceeds min, which FLT_MAX and INT_MAX both satisfy, so the industrial slider claimed rows it cannot draw: every real value lands in the first pixel of the rail and a drag moves the value by astronomical steps. Both sliders now decline those, so they fall through to stock, whose SliderFloat has a proper unbounded mode where the handle sits centred and drags relatively. The sentinel test matches stock's is_range_bounded() exactly rather than using a threshold of its own, otherwise a merely huge range would be declined here and still counted as bounded there, breaking it in both designs instead of neither. An industrial unbounded slider wants a relative-drag handle of its own. That is a design job rather than a fix, so it is left for a later pass. The rows also read as a separate piece of UI sitting next to the rest of the application. Two causes, both gone: - attribute keys were letter-spaced and fully uppercased in a probed font family. They now use the host application's font at row size, via one row_label_font() rather than the same two lines repeated in five row types - the section surface was a 0.30 lift off the window colour, which on a #2B2B2B window gives about #6B6B6B and stands out strongly. It is now 0.15, landing on #4B4B4B, and still derived rather than hardcoded so it tracks whatever palette the host runs --- .../designs/industrial/slider_chrome.hpp | 30 +++++++++++++++++++ MetaUI/qt/include/meta_qt/ui/theme.hpp | 15 ++++++++++ .../qt/src/designs/industrial/check_row.cpp | 3 +- MetaUI/qt/src/designs/industrial/combo.cpp | 3 +- .../qt/src/designs/industrial/int_slider.cpp | 7 +++-- .../src/designs/industrial/param_slider.cpp | 7 +++-- .../src/designs/industrial/slider_chrome.cpp | 3 +- MetaUI/qt/src/ui/theme.cpp | 21 +++++++++++-- 8 files changed, 74 insertions(+), 15 deletions(-) diff --git a/MetaUI/qt/include/meta_qt/designs/industrial/slider_chrome.hpp b/MetaUI/qt/include/meta_qt/designs/industrial/slider_chrome.hpp index 2c52c12..0970fba 100644 --- a/MetaUI/qt/include/meta_qt/designs/industrial/slider_chrome.hpp +++ b/MetaUI/qt/include/meta_qt/designs/industrial/slider_chrome.hpp @@ -2,6 +2,8 @@ Public License. The full license is in the file LICENSE, distributed with this software. */ #pragma once +#include + #include #include #include @@ -56,6 +58,34 @@ void paint_slider_row(QPainter &painter, int height); /// Stylesheet for the value field, following the theme and row state. +/** @brief True when a rail can meaningfully represent the range [lo, hi]. + * + * A bound of FLT_MAX or INT_MAX does not mean "a very wide slider", it means + * "no limit". A rail a couple of hundred pixels wide cannot show that: every + * value a user would type lands in the first pixel, and a drag moves the value + * by astronomical steps. That is the broken behaviour on the unbounded rows. + * + * Declining them here lets them fall through to stock, whose SliderFloat has a + * proper unbounded mode: the handle sits centred at rest and drags relatively + * instead of mapping to an absolute position. Using the fallback chain is the + * point of the design registry, so this belongs in can_render() rather than as + * a special case inside the paint code. + * + * The sentinel test deliberately matches stock's `is_range_bounded()` exactly, + * against the type's own limits. A looser threshold would leave a gap where a + * merely huge range is declined here but still counted as bounded there, which + * breaks it in both designs rather than neither. + */ +template bool has_usable_range(T lo, T hi) +{ + // Written as a positive test so a NaN bound falls out here rather than + // passing an inverted comparison. + if (!(hi > lo)) return false; + + return lo > std::numeric_limits::lowest() && + hi < std::numeric_limits::max(); +} + QString field_stylesheet(const Theme &theme, bool editing, bool modified, diff --git a/MetaUI/qt/include/meta_qt/ui/theme.hpp b/MetaUI/qt/include/meta_qt/ui/theme.hpp index f5fbd61..b07f074 100644 --- a/MetaUI/qt/include/meta_qt/ui/theme.hpp +++ b/MetaUI/qt/include/meta_qt/ui/theme.hpp @@ -25,6 +25,21 @@ QFont mono_font(int pixel_size); /// The UI sans face. No family override -- the system font is correct here. QFont ui_font(int pixel_size, bool bold = false, qreal letter_spacing = 0.0); +/** @brief Font for an attribute row's label. + * + * The host application's font, at the row size. Deliberately not a probed + * family, not letter-spaced and not uppercased. + * + * The design originally set all three, which made a panel of parameter names + * read as its own separate piece of UI sitting next to the rest of the + * application rather than part of it. Attribute keys are words the user reads, + * not chrome, and uppercasing them costs legibility for styling. + * + * Defined once because five row types were each setting the same two lines, + * which is five places for them to drift apart. + */ +QFont row_label_font(); + /** @brief Geometry and timing constants for a design. * * Separate from the palette because a colourway swap changes colours only, diff --git a/MetaUI/qt/src/designs/industrial/check_row.cpp b/MetaUI/qt/src/designs/industrial/check_row.cpp index d32eb03..b0e2d76 100644 --- a/MetaUI/qt/src/designs/industrial/check_row.cpp +++ b/MetaUI/qt/src/designs/industrial/check_row.cpp @@ -71,8 +71,7 @@ void CheckRow::paintEvent(QPaintEvent *) const bool locked = is_locked(); // --- label - QFont label_font = ui_font(12, false, 1.0); - label_font.setCapitalization(QFont::AllUppercase); + QFont label_font = row_label_font(); painter.setFont(label_font); painter.setPen(t.state_ink(is_modified(), locked)); const int label_w = width() - m.switch_width - m.gap; diff --git a/MetaUI/qt/src/designs/industrial/combo.cpp b/MetaUI/qt/src/designs/industrial/combo.cpp index b30f8f3..9621e20 100644 --- a/MetaUI/qt/src/designs/industrial/combo.cpp +++ b/MetaUI/qt/src/designs/industrial/combo.cpp @@ -260,8 +260,7 @@ void paint_combo_field(QWidget &widget, m.label_min_width, m.label_max_width)); - QFont label_font = ui_font(12, false, 1.0); - label_font.setCapitalization(QFont::AllUppercase); + QFont label_font = row_label_font(); painter.setFont(label_font); painter.setPen(theme.state_ink(modified, locked)); painter.drawText(QRect(0, 0, label_width, height), diff --git a/MetaUI/qt/src/designs/industrial/int_slider.cpp b/MetaUI/qt/src/designs/industrial/int_slider.cpp index 91251d4..6154bb4 100644 --- a/MetaUI/qt/src/designs/industrial/int_slider.cpp +++ b/MetaUI/qt/src/designs/industrial/int_slider.cpp @@ -95,7 +95,9 @@ bool IntSlider::can_render(const Attribute &attr) !metadata.find(meta::keys::constraints::max)) return false; - return meta::common::max(attr) > meta::common::min(attr); + // Declines unbounded ranges so they fall through to the stock input, which is + // the right control for a number with no limits. + return has_usable_range(meta::common::min(attr), meta::common::max(attr)); } void IntSlider::set(const int &value) @@ -140,8 +142,7 @@ void IntSlider::paintEvent(QPaintEvent *) visual.modified = is_modified(); visual.locked = is_locked(); - QFont label_font = ui_font(12, false, 1.0); - label_font.setCapitalization(QFont::AllUppercase); + QFont label_font = row_label_font(); visual.label = elide_label(QString::fromStdString(label_), label_font, geometry.label.width()); diff --git a/MetaUI/qt/src/designs/industrial/param_slider.cpp b/MetaUI/qt/src/designs/industrial/param_slider.cpp index 01c80db..a0b7898 100644 --- a/MetaUI/qt/src/designs/industrial/param_slider.cpp +++ b/MetaUI/qt/src/designs/industrial/param_slider.cpp @@ -114,7 +114,9 @@ bool ParamSlider::can_render(const Attribute &attr) !metadata.find(meta::keys::constraints::max)) return false; - return meta::common::max(attr) > meta::common::min(attr); + // Declines unbounded ranges so they fall through to the stock input, which is + // the right control for a number with no limits. + return has_usable_range(meta::common::min(attr), meta::common::max(attr)); } void ParamSlider::set(const float &value) @@ -183,8 +185,7 @@ void ParamSlider::paintEvent(QPaintEvent *) visual.modified = is_modified(); visual.locked = is_locked(); - QFont label_font = ui_font(12, false, 1.0); - label_font.setCapitalization(QFont::AllUppercase); + QFont label_font = row_label_font(); visual.label = elide_label(QString::fromStdString(label_), label_font, geometry.label.width()); diff --git a/MetaUI/qt/src/designs/industrial/slider_chrome.cpp b/MetaUI/qt/src/designs/industrial/slider_chrome.cpp index b1a97a5..9bdccdb 100644 --- a/MetaUI/qt/src/designs/industrial/slider_chrome.cpp +++ b/MetaUI/qt/src/designs/industrial/slider_chrome.cpp @@ -72,8 +72,7 @@ void paint_slider_row(QPainter &painter, const Metrics &m = theme.metrics; // --- label. Text is the only thing state is allowed to change. - QFont label_font = ui_font(12, false, 1.0); - label_font.setCapitalization(QFont::AllUppercase); + QFont label_font = row_label_font(); painter.setFont(label_font); painter.setPen(theme.state_ink(visual.modified, visual.locked)); painter.drawText(geometry.label, diff --git a/MetaUI/qt/src/ui/theme.cpp b/MetaUI/qt/src/ui/theme.cpp index 3ffaecc..c1cff23 100644 --- a/MetaUI/qt/src/ui/theme.cpp +++ b/MetaUI/qt/src/ui/theme.cpp @@ -70,6 +70,16 @@ QFont ui_font(int pixel_size, bool bold, qreal letter_spacing) return font; } +QFont row_label_font() +{ + // QApplication::font() rather than ui_font(): the point is to match whatever + // the host is using, so the panel blends into the application instead of + // announcing itself. + QFont font = QApplication::font(); + font.setPixelSize(12); + return font; +} + // --- Theme namespace @@ -108,9 +118,14 @@ Theme Theme::from_palette(const QPalette &palette, const std::string &name) // --- surfaces t.page = window; t.bar = sink(window, 0.06); - // Deliberately a clear step off the page, not a hint of one. A card that is - // barely lighter than its background does not group anything. - t.section_surface = lift(window, 0.30); + // A clear step off the page, but only just. At a 0.30 lift the cards read as + // a separate piece of UI laid on top of the application rather than part of + // it, which is the main thing that made the panel stand out. + // + // 0.15 is roughly one step, and on a host whose window is #2B2B2B it lands on + // #4B4B4B, the same value such applications typically use for their secondary + // surface. Derived rather than hardcoded, so it still tracks the palette. + t.section_surface = lift(window, 0.15); // The header shares the card surface so a section reads as one block rather // than a bar with a differently coloured body under it. Hover and press are