fix(render): convert Trace::width through the UI scale, and fold the controller's tolerance conversion into Style::device()

This commit is contained in:
Martino Ferrari committed 2026-09-28 12:26:06 +02:00
1 parent 7500fc90d2
commit 69495a3799
7 files changed
+158 -24

No files matched your search

@@ -19,7 +19,7 @@
- **Data-space geometry never scales.** Only chrome does. `dense_threshold_spp` stays a *device*-pixel ratio (it governs rasterisation density, not physical size); everything a user perceives as a size — font px, stroke widths, paddings, swatch and marker sizes, tick spacing — is logical.
- The renderer clears only its viewport region via scissor and hands back host GL state; `renderer.hpp`'s documented residue list stays accurate.
- OpenGL 3.3 core baseline.
- The 156 existing test cases keep passing; any signature change is called out in the task report.
- The 164 existing test cases keep passing (they are 171 from Task 1 on, which adds 7 of its own); any signature change is called out in the task report.
- Every task ends with `cmake --preset linux-debug && cmake --build build/debug && ctest --test-dir build/debug --output-on-failure` green and warning-clean, and commits with the message in its own section.
---
@@ -74,7 +74,7 @@ git add -A && git commit -m "feat(render): logical style metrics with a UI scale
- Test: `tests/test_plot.cpp`, `tests/test_render_smoke.cpp`
**Interfaces:**
- Consumes: the tick set (`plot::Tick{f64 value, bool major, std::string label}` and `generate_ticks`'s `include_minors`), `Style` from Task A.
- Consumes: the tick set (`plot::Tick{f64 value, bool major, std::string label}` and `generate_ticks`'s `include_minors`), `Style` from Task 1 (`ui_scale`, `device()`, and the logical/device classification it wrote down, which is what makes the two new widths logical).
- Produces: `Style::grid_major_width`, `Style::grid_minor_width` (logical), `Style::grid_major_visible`, `Style::grid_minor_visible`; grid segments tagged with their class.
- [ ] **Step 1: Per-class style**
+5 -3
View File
@@ -59,9 +59,11 @@ enum class DragMode {
/// therefore change its *physical* size with the display: a 5 px grab radius is
/// half the hand movement on a 2x panel, so the same cursor becomes harder to
/// catch there. Both tolerances -- the cursor grab radius and the box-zoom click
/// threshold -- are therefore logical constants multiplied by `Plot::ui_scale()`,
/// which is also why the controller reads the scale off the plot it already holds
/// instead of keeping a copy a host would have to keep in step.
/// threshold -- are therefore logical constants, converted through
/// `Style::device()` at the scale of the plot the controller already holds: that is
/// the single place the logical -> device rule lives, and the plot is the one that
/// knows what its device pixel is worth, so the controller keeps no copy of the scale
/// for a host to keep in step.
///
/// The middle-drag axis zoom's per-pixel gain is deliberately *not* in that group:
/// it is a data-navigation sensitivity rather than a size (the wheel and `+`/`-`
+5
View File
@@ -11,6 +11,9 @@ namespace plot {
struct TraceDesc {
const data::Series* series = nullptr;
Color color = Color::hex(0x4FA3FF);
/// The stroke's width in **logical** pixels, like every stroke in `Style` (see
/// that struct's unit rule): the renderer converts it once, so a trace has the
/// same weight against the axis lines whatever the display's DPI.
f32 width = 1.5f;
u32 axis_x = 0, axis_y = 0;
/// What the trace is called, for the legend. Empty means "do not list it", which
@@ -28,6 +31,8 @@ struct TraceDesc {
struct Trace {
const data::Series* series = nullptr;
Color color = Color::hex(0x4FA3FF);
/// Logical pixels, as in `TraceDesc::width`: converted to device pixels by the
/// renderer's one `Style::device()` call per stroke.
f32 width = 1.5f;
u32 axis_x = 0, axis_y = 0;
/// The legend's entry for this trace, empty when it has none. A name does not
+7 -2
View File
@@ -24,8 +24,13 @@ namespace plot::render {
/// chosen LOD level depend on the display and not on the data. It is used raw.
///
/// The same rule classifies the metrics that live outside this struct, which are
/// logical too and converted by the same call: `Legend`'s pad, swatch, gap and
/// margin, and the renderer's marker arm lengths and text offsets.
/// logical too and converted by the same call:
///
/// * `Legend`'s pad, swatch, gap and margin;
/// * `Trace`'s (and `TraceDesc`'s) `width` -- a stroke like `line_width_px`, and
/// the one a plot draws most of, so leaving it raw would invert the
/// trace/axis stroke hierarchy at any scale above 1;
/// * the renderer's marker arm lengths and text offsets.
struct Style {
// --- colours: neither logical nor device, so nothing to scale ------------
+9 -16
View File
@@ -21,10 +21,11 @@ constexpr f64 kKeyZoomNotches = 1.0;
/// kZoomPerNotch: 200 px of drag is one decade, exp(-200 * ln10/200) == 0.1.
constexpr f64 kZoomPerPixel = std::numbers::ln10 / 200.0;
/// How close the pointer must be to a cursor to grab it, in **logical** pixels: the
/// listener's aim is a physical distance, so the device-pixel radius it is compared
/// against is this times `Plot::ui_scale()` (see `grab_radius()`). An unscaled
/// constant would make grabbing physically harder -- half as forgiving -- on a 2x
/// display, which is the defect the scale exists to prevent.
/// listener's aim is a physical distance, so it is converted to the device-pixel
/// radius the comparison needs through `Style::device()` at the plot's own scale
/// (see `cursor_at()`). An unscaled constant would make grabbing physically harder
/// -- half as forgiving -- on a 2x display, which is the defect the scale exists to
/// prevent.
constexpr f32 kCursorHitRadiusLogical = 5.0f;
/// A box-zoom selection narrower than this on either side is a click. Logical for
/// the same reason: the gesture is the same hand movement on any display.
@@ -44,15 +45,6 @@ f64 from_space(f64 v, bool log) noexcept {
/// key) zoom in, so the span shrinks by `exp(-notches * kZoomPerNotch)`.
f64 zoom_factor(f64 notches) noexcept { return std::exp(-notches * kZoomPerNotch); }
/// Device pixels per logical unit for the tolerances below. Read from the plot the
/// controller already holds -- every forwarded coordinate is in device pixels, and
/// the plot is the one that knows what a device pixel is worth -- rather than kept
/// as a copy that a host would have to update in step with the renderer's.
f32 tolerance_scale(const Plot& plot) noexcept {
const f32 scale = plot.ui_scale();
return scale > 0.0f ? scale : 1.0f;
}
/// True when a coordinate is one arithmetic can work with. Every event here comes
/// from a host, and a host can hand over anything: a toolkit's "no pointer"
/// sentinel, a NaN from a degenerate mapping of its own, an infinity from a lost
@@ -283,8 +275,9 @@ void Controller::apply_box_zoom() {
const Rect sel = selection_;
// A selection under a few *logical* pixels on either side is a click: no zoom.
// The selection is measured in device pixels like everything the controller is
// handed, so the threshold is the logical one at this plot's scale.
const f32 min_box = kMinBoxLogical * tolerance_scale(*plot_);
// handed, so the threshold is converted at the plot's own scale -- through the one
// place that conversion lives, so a change to its fallback is picked up here too.
const f32 min_box = plot_->style().device(kMinBoxLogical);
if (sel.w < min_box || sel.h < min_box) return;
// Clamped to the axes rect the transforms are built from, so a drag that left
@@ -341,7 +334,7 @@ u64 Controller::cursor_at(f32 px, f32 py) const {
const PlotLayout& lay = std::as_const(*plot_).layout();
if (!lay.axes.contains(px, py)) return kInvalid;
const f32 radius = kCursorHitRadiusLogical * tolerance_scale(*plot_);
const f32 radius = plot_->style().device(kCursorHitRadiusLogical);
u64 best = kInvalid;
f32 best_d = 0.0f;
for (u32 i = 0; i < plot_->cursor_count(); ++i) {
+6 -1
View File
@@ -743,7 +743,12 @@ void Renderer::draw_traces(const Plot& plot, const PlotLayout& layout) {
stage_xy(gather_.data(), static_cast<u32>(gather_.size() / 2u),
plot.x_axis(trace.axis_x).log, plot.y_axis(trace.axis_y).log);
flush_lines(mapping(plot, trace.axis_x, trace.axis_y), trace.color, trace.width);
// A trace's width is a logical size like every other stroke here, so it is
// converted too: left raw, a 1.5 px trace would be drawn 1.5 device px against a
// 3 device px axis line at scale 2 -- half the logical weight, which inverts the
// trace/axis hierarchy on a high-DPI panel.
flush_lines(mapping(plot, trace.axis_x, trace.axis_y), trace.color,
style_.device(trace.width));
}
}
+124
View File
@@ -2418,6 +2418,20 @@ PLOT_TEST(legend_does_not_cover_the_plot_when_placed_bottom_left) {
namespace {
/// True for a pixel holding the axis line's chalk grey at a usable coverage: the line
/// is `axis_line` (0x8A94A6) over the frame's dark background, so a third of the
/// line's own level per channel is a pixel the stroke lifted off the background --
/// `text_ink`'s coverage test, with a tone test on top that no blue trace and no dark
/// grid line satisfies. The tick labels are the same chalk family, which is why the
/// scans below keep clear of the label strip's own columns.
bool axis_line_pixel(const plot::u8 px[4], plot::Color c) {
const auto lifted = [](plot::u8 got, plot::f32 want) {
return static_cast<plot::f32>(got) >= want * 255.0f * 0.33f;
};
const bool grey = static_cast<int>(px[2]) - static_cast<int>(px[0]) < 40;
return lifted(px[0], c.r) && lifted(px[1], c.g) && lifted(px[2], c.b) && grey;
}
/// The chrome one render reports, in **device** pixels. Both runs of the two-scale
/// tests below fill this from a frame they really drew, so a comparison of the two
/// figures is a comparison of two frames and not of two derivations of one rule.
@@ -2663,3 +2677,113 @@ PLOT_TEST(dense_threshold_stays_a_device_ratio) {
static_cast<double>(kThreshold));
ctx.destroy();
}
/// A trace's stroke is a chrome size like the axis lines', so the *weight* it is drawn
/// with -- and with it the trace/axis hierarchy a reader sees -- must not depend on the
/// display. `Trace::width` reaches the same `flush_lines()` the axis lines do, so it has
/// to go through the same `Style::device()` conversion; left raw, a 1.5 px trace is 1.5
/// *device* px against a 3 device px axis line at scale 2.
///
/// The measurement is the ink a fixed-size **device** rectangle holds over a flat trace
/// (so the count is the stroke's width times a constant length, and cleaning up the
/// anti-aliasing costs a fraction of a pixel rather than the whole figure), plus the
/// same count over the y axis line. The assertion is the ratio between them: it is 1 at
/// both scales when both strokes are converted, and it halves at scale 2 when the trace
/// alone is not.
PLOT_TEST(trace_stroke_weight_is_logical) {
constexpr plot::u32 kBigW = 2u * kW;
constexpr plot::u32 kBigH = 2u * kH;
/// The width both strokes are given, in logical pixels: equal on purpose, so the
/// trace/axis ratio the case pins is 1 rather than some arbitrary figure.
constexpr plot::f32 kWidth = 3.0f;
plottest::HeadlessContext ctx(kBigW, kBigH);
plot::Status st;
PLOT_CHECK(ctx.create(&st));
PLOT_CHECK(plot::gl::load(nullptr));
plot::Renderer r;
PLOT_CHECK(r.init(&st));
r.style().line_width_px = kWidth; // the axis lines and cursor markers
// A flat trace, so every column of the scan crosses it exactly once and the ink
// count is its width times the scan's length. y = 0.5 on a [0,1] axis is not a tick,
// so no grid line runs along it.
std::vector<plot::f32> flat(64, 0.5f);
plot::data::ArraySeries s(std::move(flat), 0.0, 1.0);
/// The device ink of both strokes at one scale: pixels of trace colour in a
/// 200x8 px box over the trace, and axis-line pixels in a 6x200 px box over the y
/// axis line. Both boxes are the same *device* size at every scale, which is what
/// makes the count proportional to the stroke's width alone.
struct StrokeInk {
plot::u32 trace = 0u;
plot::u32 axis = 0u;
};
const auto ink_at = [&](plot::f32 scale) {
const plot::u32 w = static_cast<plot::u32>(scale * static_cast<plot::f32>(kW));
const plot::u32 h = static_cast<plot::u32>(scale * static_cast<plot::f32>(kH));
plot::Plot p;
p.set_viewport({0.0f, 0.0f, static_cast<plot::f32>(w), static_cast<plot::f32>(h)});
const plot::u32 ax = p.add_x_axis({"t", {0.0, 10.0}, false, plot::AxisPlace::Bottom, false});
const plot::u32 ay = p.add_y_axis({"v", {0.0, 1.0}, false, plot::AxisPlace::Left, false});
p.add_trace({&s, plot::Color::hex(0x40C0FF), kWidth, ax, ay});
r.style().ui_scale = scale;
r.begin_frame(kBigW, kBigH, {0.0f, 0.0f, static_cast<plot::f32>(w), static_cast<plot::f32>(h)});
r.draw(p);
r.end_frame();
PLOT_CHECK(!r.last_frame_had_gl_error());
const plot::PlotLayout& lay = p.layout();
// Generic over the predicate: the two strokes are recognised by different rules
// (the trace by blue dominance, the axis line by its chalk grey).
const auto count = [&](plot::u32 x0, plot::u32 y0, plot::u32 bw, plot::u32 bh, auto pred) {
plot::u32 hits = 0;
for (plot::u32 y = y0; y < y0 + bh; ++y) {
for (plot::u32 x = x0; x < x0 + bw; ++x) {
plot::u8 px[4] = {};
PLOT_CHECK(ctx.read_pixel(x, kBigH - 1u - y, px));
if (pred(px)) ++hits;
}
}
return hits;
};
StrokeInk ink;
// The trace: a fixed device box inside the axes, centred on the stroke's row.
const plot::u32 ty = static_cast<plot::u32>(p.y_transform(ay).to_screen_y(0.5));
const plot::u32 tx0 = static_cast<plot::u32>(lay.axes.x) + 40u;
ink.trace = count(tx0, ty - 4u, 200u, 8u,
[](const plot::u8* px) { return trace_pixel(px); });
// The axis line: two columns left of it, so the tick labels' own ink (which ends
// `grid_label_pad` short of the line) is outside the scan at both scales.
const plot::Color line = r.style().axis_line;
ink.axis = count(static_cast<plot::u32>(lay.axes.x) - 2u,
static_cast<plot::u32>(lay.axes.y) + 40u, 6u, 200u,
[line](const plot::u8* px) { return axis_line_pixel(px, line); });
return ink;
};
const StrokeInk one = ink_at(1.0f);
const StrokeInk two = ink_at(2.0f);
const plot::f32 trace_ratio =
static_cast<plot::f32>(two.trace) / static_cast<plot::f32>(one.trace);
const plot::f32 axis_ratio =
static_cast<plot::f32>(two.axis) / static_cast<plot::f32>(one.axis);
std::printf("[trace] 1x trace %u axis %u; 2x trace %u axis %u; ratios %.3f / %.3f ->"
" weight ratio %.3f\n",
one.trace, one.axis, two.trace, two.axis, static_cast<double>(trace_ratio),
static_cast<double>(axis_ratio),
static_cast<double>(trace_ratio / axis_ratio));
// Not vacuous: the trace really is in the box (and the axis line in its own).
PLOT_CHECK(one.trace > 100u);
PLOT_CHECK(one.axis > 100u);
// The same logical width at twice the scale is twice the device ink -- for the trace
// exactly as for the axis line it is drawn against. A trace left in device pixels
// fails this: its ink count is the same at both scales.
PLOT_CHECK_NEAR(trace_ratio, 2.0f, 0.35f);
PLOT_CHECK_NEAR(axis_ratio, 2.0f, 0.35f);
PLOT_CHECK_NEAR(trace_ratio / axis_ratio, 1.0f, 0.15f);
ctx.destroy();
}