From b4e88fe0c1514195334cf646786f3def82643592 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Sun, 27 Sep 2026 22:10:53 +0200 Subject: [PATCH] fix(render): give degenerate segments area, pin font bit order, clarify vertex_count --- include/plot/render/primitives.hpp | 6 +++++- include/plot/render/shaders.hpp | 5 +++++ src/render/shaders.cpp | 17 +++++++++------ tests/test_gl_context.cpp | 29 ++++++++++++++++++------- tests/test_primitives.cpp | 34 ++++++++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 15 deletions(-) diff --git a/include/plot/render/primitives.hpp b/include/plot/render/primitives.hpp index c1fe5f1..5acab47 100644 --- a/include/plot/render/primitives.hpp +++ b/include/plot/render/primitives.hpp @@ -34,7 +34,11 @@ class LineBatch { /// Segments staged so far, one instance each. u32 segment_count() const noexcept { return static_cast(v_.size() / 4u); } - /// Vertices the pipeline will rasterise: two per staged segment. + /// Staging vertices, two per staged segment. The pipeline draws one instanced + /// quad per segment (its four `GL_TRIANGLE_STRIP` corners come from + /// `gl_VertexID`), so the instance count a draw passes is `vertex_count() / 2` + /// — handing `vertex_count()` to `glDrawArraysInstanced` would rasterise + /// twice the geometry the batch staged. u32 vertex_count() const noexcept { return segment_count() * 2u; } /// Interleaved `x0,y0,x1,y1`, four floats per staged segment. diff --git a/include/plot/render/shaders.hpp b/include/plot/render/shaders.hpp index 363845f..9d8055c 100644 --- a/include/plot/render/shaders.hpp +++ b/include/plot/render/shaders.hpp @@ -17,6 +17,11 @@ namespace plot::render { /// `plot::Rect` is top-left origin while GL is bottom-left, so the y axis is /// flipped exactly once, here. /// +/// A degenerate segment (`a_p0 == a_p1`) keeps a finite normal and is stretched +/// into a square mark one stroke width and fringe across, rather than +/// collapsing to zero area and rasterising nothing — the dense path stages +/// `min == max` buckets as exactly such segments. +/// /// The fragment shader discards outside the fringe and ramps alpha over the last /// pixel of the half width with `smoothstep`, which is what keeps a 1px trace /// from aliasing into dashes. diff --git a/src/render/shaders.cpp b/src/render/shaders.cpp index 5f77848..c04be5c 100644 --- a/src/render/shaders.cpp +++ b/src/render/shaders.cpp @@ -25,15 +25,20 @@ void main() { vec2 p1 = a_p1 * u_scale + u_offset; vec2 d = p1 - p0; float len = length(d); - // A degenerate segment has no direction: pin the normal so the quad stays - // finite and the instance draws as a dot instead of NaN geometry. - vec2 n = len > 1e-6 ? vec2(-d.y, d.x) / len : vec2(0.0, 1.0); float half_w = u_width * 0.5 + 1.0; // + one pixel of AA fringe + // A degenerate segment has no direction: pin the normal, and give the quad + // area by stretching its two ends apart along x. Collapsing all four corners + // onto p0 would rasterise nothing, and a flat trace staged as p == p (the + // dense path's min == max buckets) would then silently vanish. When + // len > 1e-6 the two ends are p0 and p1 exactly as before. + vec2 n = len > 1e-6 ? vec2(-d.y, d.x) / len : vec2(0.0, 1.0); + vec2 e0 = len > 1e-6 ? p0 : p0 - vec2(half_w, 0.0); + vec2 e1 = len > 1e-6 ? p1 : p0 + vec2(half_w, 0.0); - // gl_VertexID picks the corner: 0,1 are the sides of a_p0 and 2,3 of a_p1, - // which is the order GL_TRIANGLE_STRIP expects. + // gl_VertexID picks the corner: 0,1 are the sides of the segment's start and + // 2,3 of its end, which is the order GL_TRIANGLE_STRIP expects. float side = (gl_VertexID & 1) == 0 ? -1.0 : 1.0; - vec2 base = (gl_VertexID & 2) == 0 ? p0 : p1; + vec2 base = (gl_VertexID & 2) == 0 ? e0 : e1; vec2 px = base + n * (side * half_w); // Rect is top-left origin, GL is bottom-left: flip y once, here, and never diff --git a/tests/test_gl_context.cpp b/tests/test_gl_context.cpp index 0ea1038..d9cd250 100644 --- a/tests/test_gl_context.cpp +++ b/tests/test_gl_context.cpp @@ -269,13 +269,15 @@ PLOT_TEST(line_pipeline_draws_an_aa_strip) { plot::render::LineVertexArray lines; PLOT_CHECK(lines.create(&st)); - // Two instances, so a wrong attribute divisor cannot pass: with a divisor of - // zero the second quad would be assembled from the first instance's leftover - // attribute values instead of its own segment. Data space is pixels here, and - // u_offset lands the two axes at Rect y = 100 and Rect y = 120. - const f32 segments[8] = {0.0f, 0.0f, 40.0f, 0.0f, 0.0f, 20.0f, 40.0f, 20.0f}; - lines.upload(segments, 2); - PLOT_CHECK(lines.instance_count() == 2); + // Three instances: two segments at different y, so a wrong attribute divisor + // cannot pass (with a divisor of zero the second quad would be assembled from + // the first instance's leftover attribute values), plus a degenerate one at + // Rect (64, 140) — the shape a dense trace's min == max bucket stages, which + // must still leave a mark instead of collapsing to zero area. + const f32 segments[12] = {0.0f, 0.0f, 40.0f, 0.0f, 0.0f, 20.0f, + 40.0f, 20.0f, 0.0f, 40.0f, 0.0f, 40.0f}; + lines.upload(segments, 3); + PLOT_CHECK(lines.instance_count() == 3); plot::gl::glViewport(0, 0, static_cast(ctx.width), static_cast(ctx.height)); @@ -291,7 +293,7 @@ PLOT_TEST(line_pipeline_draws_an_aa_strip) { program.set_width(3.0f); program.set_color(plot::Color::hex(0xFF00FF)); lines.bind(); - plot::gl::glDrawArraysInstanced(plot::gl::GL_TRIANGLE_STRIP, 0, 4, 2); + plot::gl::glDrawArraysInstanced(plot::gl::GL_TRIANGLE_STRIP, 0, 4, 3); plot::u8 px[4] = {}; // Rect y = 100 is GL row 155.5, so rows 155 and 156 straddle the axis @@ -318,6 +320,17 @@ PLOT_TEST(line_pipeline_draws_an_aa_strip) { // Past the end of the segment (flat cap) and far from both strips. PLOT_CHECK(ctx.read_pixel(110, 155, px)); PLOT_CHECK(px[0] + px[2] < 40); + // The degenerate instance: Rect y = 140 is rows 115/116 at x = 64, and the + // mark must be visible rather than a zero-area quad, while staying inside its + // own stroke width + fringe (columns 62..66). + PLOT_CHECK(ctx.read_pixel(64, 115, px)); + PLOT_CHECK(px[0] > 200 && px[2] > 200); + PLOT_CHECK(ctx.read_pixel(64, 116, px)); + PLOT_CHECK(px[0] > 200 && px[2] > 200); + PLOT_CHECK(ctx.read_pixel(64, 121, px)); + PLOT_CHECK(px[0] + px[2] < 40); + PLOT_CHECK(ctx.read_pixel(70, 115, px)); + PLOT_CHECK(px[0] + px[2] < 40); PLOT_CHECK(ctx.read_pixel(128, 0, px)); PLOT_CHECK(px[0] + px[2] < 40); PLOT_CHECK(plot::gl::glGetError() == plot::gl::GL_NO_ERROR); diff --git a/tests/test_primitives.cpp b/tests/test_primitives.cpp index c0c1df8..e936b7f 100644 --- a/tests/test_primitives.cpp +++ b/tests/test_primitives.cpp @@ -151,6 +151,40 @@ PLOT_TEST(bitmap_font_decodes_si_units_and_falls_back_visibly) { PLOT_CHECK(any_ink); } +// The packed atlas must be left-to-right in absolute terms: `(row >> x) & 1` is +// also how every other expectation here is derived, so a table transcribed with +// the opposite in-row bit order would keep those tests green while drawing +// mirrored text. 'A' cannot catch that (it is +// symmetric about its vertical axis); 'L', 'b' and '/' can. +PLOT_TEST(bitmap_font_bit_order_runs_left_to_right) { + using plot::render::BitmapFont; + const plot::u8* atlas = BitmapFont::atlas_image(); + auto ink = [atlas](char c, plot::u32 col, plot::u32 row) { + const plot::u32 slot = static_cast(c) - BitmapFont::kFirstChar; + const plot::Vec2 origin = BitmapFont::atlas_origin(slot); + const auto x = static_cast(origin.x) + col; + const auto y = static_cast(origin.y) + row; + return atlas[y * BitmapFont::kAtlasWidth + x] != 0u; + }; + // 'L': stem and top serif on the left, the foot running right along the + // bottom row and stopping one texel short of the right edge. + PLOT_CHECK(ink('L', 0u, 0u)); + PLOT_CHECK(!ink('L', 7u, 0u)); + PLOT_CHECK(ink('L', 1u, 3u)); + PLOT_CHECK(!ink('L', 6u, 3u)); + PLOT_CHECK(ink('L', 6u, 6u)); + PLOT_CHECK(!ink('L', 7u, 6u)); + // 'b': the stem is the leftmost column of the upper rows. + PLOT_CHECK(ink('b', 0u, 0u)); + PLOT_CHECK(!ink('b', 7u, 0u)); + // '/': starts at the right of the top row and ends at the left of the last + // one — the opposite of '\'. + PLOT_CHECK(ink('/', 6u, 0u)); + PLOT_CHECK(!ink('/', 0u, 0u)); + PLOT_CHECK(ink('/', 0u, 6u)); + PLOT_CHECK(!ink('/', 6u, 6u)); +} + PLOT_TEST(bitmap_font_atlas_is_uploadable) { using plot::render::BitmapFont; PLOT_CHECK(BitmapFont::kAtlasWidth == BitmapFont::kAtlasCols * BitmapFont::kGlyphPx);