diff --git a/docs/superpowers/plans/2026-09-28-plot-dpi-grid-follow.md b/docs/superpowers/plans/2026-09-28-plot-dpi-grid-follow.md index 42c6acc..d96eb05 100644 --- a/docs/superpowers/plans/2026-09-28-plot-dpi-grid-follow.md +++ b/docs/superpowers/plans/2026-09-28-plot-dpi-grid-follow.md @@ -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** diff --git a/include/plot/input/controller.hpp b/include/plot/input/controller.hpp index ff577b9..34eb09a 100644 --- a/include/plot/input/controller.hpp +++ b/include/plot/input/controller.hpp @@ -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 `+`/`-` diff --git a/include/plot/plot/trace.hpp b/include/plot/plot/trace.hpp index cdc6305..e1a867d 100644 --- a/include/plot/plot/trace.hpp +++ b/include/plot/plot/trace.hpp @@ -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 diff --git a/include/plot/render/style.hpp b/include/plot/render/style.hpp index 99428fd..974a0e6 100644 --- a/include/plot/render/style.hpp +++ b/include/plot/render/style.hpp @@ -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 ------------ diff --git a/src/input/controller.cpp b/src/input/controller.cpp index 86c3c05..1a93d23 100644 --- a/src/input/controller.cpp +++ b/src/input/controller.cpp @@ -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) { diff --git a/src/render/renderer.cpp b/src/render/renderer.cpp index 5d741e5..9c85d30 100644 --- a/src/render/renderer.cpp +++ b/src/render/renderer.cpp @@ -743,7 +743,12 @@ void Renderer::draw_traces(const Plot& plot, const PlotLayout& layout) { stage_xy(gather_.data(), static_cast(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)); } } diff --git a/tests/test_render_smoke.cpp b/tests/test_render_smoke.cpp index 6f62e04..7d7b8b8 100644 --- a/tests/test_render_smoke.cpp +++ b/tests/test_render_smoke.cpp @@ -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(got) >= want * 255.0f * 0.33f; + }; + const bool grey = static_cast(px[2]) - static_cast(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(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 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(scale * static_cast(kW)); + const plot::u32 h = static_cast(scale * static_cast(kH)); + plot::Plot p; + p.set_viewport({0.0f, 0.0f, static_cast(w), static_cast(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(w), static_cast(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(p.y_transform(ay).to_screen_y(0.5)); + const plot::u32 tx0 = static_cast(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(lay.axes.x) - 2u, + static_cast(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(two.trace) / static_cast(one.trace); + const plot::f32 axis_ratio = + static_cast(two.axis) / static_cast(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(trace_ratio), + static_cast(axis_ratio), + static_cast(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(); +}