fix(render,input): guard the zoom gain, clip annotation captions, no-throw dump
Three findings reachable by ordinary use: * `Controller::axis_zoom()` divided by `Plot::ui_scale()` directly, though `Style::device()` documents a non-positive scale as host state that must be tolerated. With `ui_scale == 0` and a negative drag delta, `logical_dy` was `-inf`, the factor `+inf`, and the guard `!(factor > 0.0)` did not reject it: `zoom_x_axis`/`zoom_y_axis` computed `anchor + (bound - anchor) * inf`, and `set_y_range()` stored it verbatim and cleared `auto_scale` -- an axis left non-finite until an auto-fit. A negative scale silently inverted the zoom. A scale that is not positive and finite now means no zoom at all. * `draw_annotations()` stages its caption into the frame's single text batch, which `flush_text()` draws *after* `clear_clip()`, so a caption staged while the axes clip was engaged was rasterised with the viewport clip. An annotation anchored outside the visible range kept a caption with no cross, and in a `Figure` -- whose rows tile the figure -- that caption painted into the neighbouring row. A caption whose anchor is outside `axes` is now skipped with its cross, rather than splitting the frame's one text draw call (which would cost a draw call per row and mis-clip the labels staged outside `axes`). * `dump_frame_ppm()` was the one public entry point not `noexcept`, so a `std::bad_alloc` crossed the API boundary. Its buffers are now sized before anything is on disk and a failure is reported as `ErrorCode::OutOfMemory`, exactly as `TrueTypeFont::from_file()` reports it. Tests: `ui_scale` 0 and -1 leave the axis range finite and unchanged with `auto_scale` intact; an annotation anchored outside a bottom figure row's y range leaves no caption ink in the row above, while an in-range one still draws; a `static_assert` pins the dump's `noexcept`.
This commit is contained in:
1 parent
9aff06acee
commit
2fbd1de7d4
5 files changed
+188
-19
No files matched your search
@@ -608,10 +608,18 @@ void Controller::axis_zoom(const PointerEvent& e) {
|
||||
// A pointer drag reports pixels, not wheel notches, so it uses its own
|
||||
// sensitivity: 200 px is one decade. The gain is a physical distance the hand
|
||||
// moves, so the device-pixel delta is converted to logical pixels first -- the
|
||||
// same drag must zoom the same amount on a 2x display. A scale no host should
|
||||
// ever set (0, or negative) makes the factor non-positive, and the guard below
|
||||
// turns that into no zoom at all rather than an axis re-centred on infinity.
|
||||
const f64 logical_dy = static_cast<f64>(e.dy) / static_cast<f64>(plot_->ui_scale());
|
||||
// same drag must zoom the same amount on a 2x display.
|
||||
//
|
||||
// A scale that is not a positive number is not a scale -- a host that has not
|
||||
// measured its display, a field nobody filled in -- the same state
|
||||
// `Style::device()` tolerates. Here it means *no zoom*: dividing by it would make
|
||||
// `logical_dy` infinite and the factor infinite (or NaN), and that factor reaches
|
||||
// `set_x_range()`/`set_y_range()`, which store it verbatim and clear `auto_scale`
|
||||
// -- an axis left non-finite until an auto-fit. A negative scale would silently
|
||||
// invert the zoom direction too, so the whole class is refused here.
|
||||
const f64 scale = static_cast<f64>(plot_->ui_scale());
|
||||
if (!(scale > 0.0) || !std::isfinite(scale)) return;
|
||||
const f64 logical_dy = static_cast<f64>(e.dy) / scale;
|
||||
const f64 factor = std::exp(-logical_dy * kZoomPerPixel);
|
||||
if (!(factor > 0.0)) return;
|
||||
if (mode_ == DragMode::AxisZoomX) {
|
||||
|
||||
+28
-6
@@ -12,6 +12,7 @@
|
||||
#include <cmath>
|
||||
#include <cstdio>
|
||||
#include <cstdlib>
|
||||
#include <new>
|
||||
#include <vector>
|
||||
|
||||
namespace plot {
|
||||
@@ -1123,6 +1124,10 @@ void Renderer::draw_measures(const Plot& plot, const PlotLayout& layout) {
|
||||
|
||||
void Renderer::draw_annotations(const Plot& plot) {
|
||||
const f32 line_h = font_backend_.height();
|
||||
// The clip every other overlay is drawn under, read here because the caption is staged
|
||||
// into the frame's single text batch -- flushed after `clear_clip()` -- so the scissor that
|
||||
// keeps the rest of this pass inside the axes does not reach it.
|
||||
const Rect& axes = plot.layout().axes;
|
||||
for (u32 i = 0; i < plot.annotation_count(); ++i) {
|
||||
const Annotation& annotation = plot.annotation(i);
|
||||
if (!annotation.visible) continue;
|
||||
@@ -1134,7 +1139,14 @@ void Renderer::draw_annotations(const Plot& plot) {
|
||||
lines_.add_segment({px - style_.device(3.0f), py}, {px + style_.device(3.0f), py});
|
||||
lines_.add_segment({px, py - style_.device(3.0f)}, {px, py + style_.device(3.0f)});
|
||||
flush_lines(pixel_mapping(), annotation.color, style_.device(1.0f));
|
||||
if (!annotation.text.empty()) {
|
||||
// A caption whose anchor is outside the axes is skipped with its cross, which the clip
|
||||
// has already removed: a caption with no cross is a label for a point the frame does not
|
||||
// show, and -- because the text batch is drawn *after* the clip is released -- it would
|
||||
// otherwise paint outside the axes it is measured in, into the neighbouring row of a
|
||||
// `Figure` whose rows tile the figure. Splitting the frame's one text draw call to clip
|
||||
// it instead would cost a draw call per row and mis-clip the tick labels, which are
|
||||
// staged outside `axes` on purpose and share the batch after this one.
|
||||
if (!annotation.text.empty() && axes.contains(px, py)) {
|
||||
stage_text({px + style_.device(5.0f), py - 0.5f * line_h}, annotation.text.c_str());
|
||||
}
|
||||
}
|
||||
@@ -1262,7 +1274,7 @@ void Renderer::draw_crosshair_readout(const Plot& plot, const PlotLayout& layout
|
||||
|
||||
// -------------------------------------------------------------------- dump
|
||||
|
||||
bool Renderer::dump_frame_ppm(const char* path, Status* st) const {
|
||||
bool Renderer::dump_frame_ppm(const char* path, Status* st) const noexcept {
|
||||
if (path == nullptr || path[0] == '\0') {
|
||||
return fail(st, ErrorCode::InvalidArgument, "PPM path is empty");
|
||||
}
|
||||
@@ -1275,9 +1287,20 @@ bool Renderer::dump_frame_ppm(const char* path, Status* st) const {
|
||||
return fail(st, ErrorCode::InvalidArgument, "no frame has been drawn yet");
|
||||
}
|
||||
|
||||
// RGBA, not RGB: 4-byte pixels have no row padding whatever the pack alignment,
|
||||
// so a host's GL_PACK_ALIGNMENT cannot make the read overrun the buffer.
|
||||
readback_.assign(usize(width) * height * 4u, 0u);
|
||||
// The two buffers are sized -- and can fail to size -- before anything is on disk, so an
|
||||
// allocation failure is reported through the `Status` rather than throwing `std::bad_alloc`
|
||||
// across the public boundary (which is `noexcept`) and leaving a half-written file behind.
|
||||
// The readback hole is RGBA, not RGB: 4-byte pixels have no row padding whatever the pack
|
||||
// alignment, so a host's GL_PACK_ALIGNMENT cannot make the read overrun the buffer. The row
|
||||
// is the PPM's own 3-byte stride, one line at a time.
|
||||
std::vector<u8> row;
|
||||
try {
|
||||
readback_.assign(usize(width) * height * 4u, 0u);
|
||||
row.assign(usize(width) * 3u, 0u);
|
||||
} catch (const std::bad_alloc&) {
|
||||
return fail(st, ErrorCode::OutOfMemory, "out of memory sizing the PPM readback");
|
||||
}
|
||||
|
||||
gl::glPixelStorei(gl::GL_PACK_ALIGNMENT, 4);
|
||||
gl::glReadPixels(static_cast<gl::GLint>(gl_viewport_.x), static_cast<gl::GLint>(gl_viewport_.y),
|
||||
static_cast<gl::GLsizei>(width), static_cast<gl::GLsizei>(height), gl::GL_RGBA,
|
||||
@@ -1288,7 +1311,6 @@ bool Renderer::dump_frame_ppm(const char* path, Status* st) const {
|
||||
return fail(st, ErrorCode::InvalidArgument, "cannot open the PPM path for writing");
|
||||
}
|
||||
std::fprintf(file, "P6\n%u %u\n255\n", width, height);
|
||||
std::vector<u8> row(usize(width) * 3u);
|
||||
bool ok = true;
|
||||
for (u32 y = 0; y < height && ok; ++y) {
|
||||
// glReadPixels hands back rows bottom-up; a PPM's rows run top-down.
|
||||
|
||||
Reference in new issue
Block a user