fix(gtk_demo): assert the recompute invariant, not a strip width; make exit idempotent

The exit-frame self-check compared the y label strip to the pixel and printed a
FAIL at GDK_SCALE=2 where it measured 174 -> 172. Measured (temporary probe,
reverted): the strip is `widest major tick label + padding`, and the tick step
follows the axes length, which the resize changes -- 18 majors over 1440 device px
with "-800m" widest, 8 over 1080 with "-500m", 78 vs 76 px in the bitmap font. The
layout was recomputed; the equality was the wrong assertion. The check now asserts
what actually distinguishes a recompute from a scaled stale layout: the axes and
its strips tile the *current* viewport, and the strip is nearer the previous strip
than that strip scaled by the resize ratio (2 px of drift against a scaled 118).

`on_exit_timer`'s main-context pump can re-enter its own 20 ms timeout, which ran
the exit block twice in one observed run; `on_teardown` is guarded the same way so
the producer join, the window destruction and the quit happen exactly once. A window
closed by hand takes the same ordering through `GtkWindow::close-request`, which
`gtk_window_destroy()` does not emit.
This commit is contained in:
Martino Ferrari committed 2026-09-28 03:04:15 +02:00
1 parent 0445a1861b
commit 377cca8e9b
1 file changed
+63 -11
+63 -11
View File
@@ -273,6 +273,11 @@ struct Demo {
const char* dump_path = nullptr;
bool exit_pending = false;
bool summary_printed = false;
/// The two exit sequences, each guarded so it runs exactly once: the timer's
/// block (which pumps the main context and can be re-entered by its own timeout)
/// and the teardown (scheduled as an idle source).
bool exit_started = false;
bool torn_down = false;
bool cursors_follow = true;
Meter meter;
@@ -297,14 +302,25 @@ void capture_snapshot(Demo& demo) {
}
}
/// The repaint pump. GTK only redraws the area when something asks it to, so this
/// is the frame rate the demo runs at (a real application drives the same timer,
/// or its toolkit's equivalent, from whatever it animates). Nothing here is
/// library API: `gtk_gl_area_queue_render` is GTK's.
/// A window closed by hand -- or by the window manager -- takes the ordering the
/// automated path takes: the producer is joined *before* GTK destroys the widget,
/// and with it the renderer whose frames read the stream. GTK emits this before its
/// own handler destroys the window, and returning FALSE lets that default happen.
gboolean on_close_request(GtkWindow*, gpointer user_data) {
Demo& demo = *static_cast<Demo*>(user_data);
demo.producer->stop();
return FALSE;
}
/// Runs the window's destruction and the quit outside any signal emission: see
/// `on_render_after`.
gboolean on_teardown(gpointer user_data) {
Demo& demo = *static_cast<Demo*>(user_data);
// The teardown runs once, whatever scheduled it (the exit frame's idle source, or
// a second one if two renders were pending): the window is destroyed and the
// application quit here, and both of those must not happen twice.
if (demo.torn_down) return G_SOURCE_REMOVE;
demo.torn_down = true;
// The producer is joined *first*, while the widget and the stream it feeds are
// both still alive: the join is what makes "no thread pushes into the stream
// after this point" true for the destruction below, and for the frames the
@@ -319,6 +335,10 @@ gboolean on_teardown(gpointer user_data) {
return G_SOURCE_REMOVE;
}
/// The repaint pump. GTK only redraws the area when something asks it to, so this
/// is the frame rate the demo runs at (a real application drives the same timer,
/// or its toolkit's equivalent, from whatever it animates). Nothing here is
/// library API: `gtk_gl_area_queue_render` is GTK's.
gboolean on_repaint(gpointer user_data) {
Demo& demo = *static_cast<Demo*>(user_data);
if (demo.widget == nullptr) { // the window was closed: the wrapper is gone too
@@ -376,9 +396,15 @@ void on_fit_clicked(GtkButton*, gpointer user_data) {
gboolean on_exit_timer(gpointer user_data) {
Demo& demo = *static_cast<Demo*>(user_data);
if (!demo.automated || now_s() - demo.run_start < demo.exit_after_s) return G_SOURCE_CONTINUE;
// The pump below iterates the main context, so this very timeout can fire from
// inside this call; the guard is what keeps the exit sequence -- the scripted
// resize, its check, the exit frame -- to one run.
if (demo.exit_started) return G_SOURCE_REMOVE;
demo.exit_started = true;
if (demo.widget == nullptr) return G_SOURCE_REMOVE; // the window is already gone
const plot::PlotLayout before = std::as_const(*demo.graph).layout();
const plot::Rect viewport_before = demo.area->device_viewport();
const int width_before = gtk_widget_get_width(demo.area->widget());
const int height_before = gtk_widget_get_height(demo.area->widget());
@@ -395,10 +421,29 @@ gboolean on_exit_timer(gpointer user_data) {
const plot::Rect viewport = demo.area->device_viewport();
// The evidence that the layout was *recomputed and not scaled*: the y label
// strip is derived from the font metrics, so it keeps its width while the axes
// take the rest of the new allocation. A scaled stale layout would have scaled
// the strip with it.
const bool recomputed = after.axes.w != before.axes.w && after.axes.x == before.axes.x;
// strip is a number of *font pixels*, so it stays the same size while the axes takes
// the rest of the new allocation. Two things are checked, and neither is "the
// strip is the same number":
//
// * the axes and its strips tile the *current* viewport exactly -- a layout
// left over from the old geometry would still tile the old one;
// * the strip is nearer the previous strip than that strip scaled by the resize
// ratio -- a layout whose strips were *scaled* would land on the second.
//
// The strip is not invariant, and the check must not pretend it is: it is the
// widest major tick label plus padding, and the tick *step* follows the axes
// length, which the resize changes. Measured at scale 2: 18 majors over 1440
// device px with "-800m" the widest label, 8 over 1080 with "-500m" -- 78 and
// 76 px in this bitmap font, whose '8' is the wider glyph. That is 2 px of
// recomputed strip, against 174 * 1620/2380 = 118 px of scaled strip.
const f32 resize_ratio = viewport_before.w > 0.0f ? viewport.w / viewport_before.w : 1.0f;
const f32 scaled_strip = before.axes.x * resize_ratio;
const f32 drift = std::fabs(after.axes.x - before.axes.x);
const bool tiles = std::fabs(after.axes.w - (viewport.w - after.axes.x)) <= 0.5f &&
std::fabs(after.axes.h - (viewport.h - after.axes.y - after.x_axis_height -
after.x_title_height)) <= 0.5f;
const bool metric_strip = drift < std::fabs(after.axes.x - scaled_strip);
const bool recomputed = after.axes.w != before.axes.w && tiles && metric_strip;
std::printf("\n[gtk_demo] ---- exit frame ----\n");
std::printf("[gtk_demo] resize logical %dx%d -> %dx%d, device viewport %ux%u, scale %g\n",
width_before, height_before, gtk_widget_get_width(demo.area->widget()),
@@ -409,10 +454,13 @@ gboolean on_exit_timer(gpointer user_data) {
static_cast<double>(before.axes.x), static_cast<double>(before.axes.y),
static_cast<double>(after.axes.w), static_cast<double>(after.axes.h),
static_cast<double>(after.axes.x), static_cast<double>(after.axes.y),
recomputed ? "recomputed (label strip unchanged)" : "FAIL: stale or rescaled layout");
std::printf("[gtk_demo] layout y label strip %.0f -> %.0f px of a %.0f -> %.0f px wide axes\n",
recomputed ? "recomputed for the new viewport (strips metric, not scaled)"
: "FAIL: stale or rescaled layout");
std::printf("[gtk_demo] layout y label strip %.0f -> %.0f px of a %.0f -> %.0f px wide axes,"
" %.0f px of drift against a scaled %.0f\n",
static_cast<double>(before.axes.x), static_cast<double>(after.axes.x),
static_cast<double>(before.axes.w), static_cast<double>(after.axes.w));
static_cast<double>(before.axes.w), static_cast<double>(after.axes.w),
static_cast<double>(drift), static_cast<double>(scaled_strip));
std::printf("[gtk_demo] init %s: %s\n", plot::to_string(demo.area->init_status().code),
demo.area->init_status().message);
std::fflush(stdout);
@@ -431,6 +479,10 @@ void on_activate(GtkApplication* app, gpointer user_data) {
demo.window = GTK_WINDOW(window);
gtk_window_set_title(GTK_WINDOW(window), "plot gtk4 demo");
gtk_window_set_default_size(GTK_WINDOW(window), kWindowWidth, kWindowHeight);
// The hand-close path's half of the shutdown ordering (see `on_close_request`);
// the automated path stops the producer in `on_teardown` instead, because
// `gtk_window_destroy()` does not emit this signal.
g_signal_connect(window, "close-request", G_CALLBACK(on_close_request), &demo);
// The plot area is the window's content. GTK owns the widget from here on: the
// wrapper is freed with it, which is also what releases the plot's GL resources.