Skip to content

fix(cg): redraw() forced stable on every frame, defeating zoom cache #616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
softmarshmallow merged 1 commit into from
Mar 30, 2026
Merged

fix(cg): redraw() forced stable on every frame, defeating zoom cache #616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions .agents/skills/cg-perf/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,11 @@ reports `min/p50/p95/p99/MAX` plus per-stage breakdown and settle cost.
| `zigzag` | fast (continuous) / slow (with pauses) × fit/zoomed | Diagonal reading pattern with direction changes |
| `zoom` | slow/fast × around-fit/high | Zoom oscillation at different levels |
| `pan_with_settle` | slow/fast × fit/zoomed | Pan with settle frames interleaved every 12 frames |
| `zoom_with_settle`| slow/fast × fit/high | Zoom with settle frames interleaved every 12 frames — captures cache-cold spike after settle nukes zoom cache |
| `zoom_forced_stable` | slow/fast × fit/high (BUG prefix) | Forces `stable=true` on every zoom frame — reproduces the `redraw()` bug for A/B comparison |
| `realtime` | fast/slow × fit/zoomed | **Real-time event loop simulation** with sleep, 240Hz tick thread, and settle countdown matching the native viewer |
| `frameloop` | 16/50/80/120/200/300/500ms interval | **Real FrameLoop path** — the only bench that captures stable-frame jank during panning (see below) |
| `frameloop_zoom` | 16/50/80/120/200/500ms interval | **Real FrameLoop path for zoom** — captures stable-frame intrusion during zoom gestures |
| `resize` | alternating viewport sizes | `--resize` flag. Measures `resize()` + `redraw()` cost per cycle (layout rebuild + cache invalidation + repaint) |

**SurfaceUI overlay measurement (`--overlay`):**
Expand Down Expand Up @@ -169,15 +172,15 @@ and simulate the native viewer's 240Hz tick thread + settle countdown.
These produce frame timings that match what users actually see,
including settle-induced frame drops at their natural frequency.

The `frameloop` scenarios go through the actual `FrameLoop.poll()` /
`complete()` path — the same code path as `Application::frame()`. All
other pan/zoom scenarios bypass `FrameLoop` and call `queue_unstable()`
directly, which means they never produce stable frames mid-interaction.
The `frameloop` scenarios sweep scroll intervals from 16ms (fast flick)
to 500ms (discrete clicks) and reveal how `FrameLoop`'s stable-frame
decisions affect the frame time distribution at each speed. Use these
when investigating panning jank, adaptive timing, or pan/zoom image
cache behavior.
The `frameloop` and `frameloop_zoom` scenarios go through the actual
`FrameLoop.poll()` / `complete()` path — the same code path as
`Application::frame()`. All other pan/zoom scenarios bypass `FrameLoop`
and call `queue_unstable()` directly, which means they never produce
stable frames mid-interaction. The `frameloop` scenarios sweep event
intervals from 16ms (fast flick) to 500ms (discrete clicks) and reveal
how `FrameLoop`'s stable-frame decisions affect the frame time
distribution at each speed. Use these when investigating panning or
zooming jank, adaptive timing, or pan/zoom image cache behavior.

**Choosing scenes:** Use `--list-scenes` to see what's available. Pick
scenes that stress the subsystem you're optimizing. For effects/caching
Expand Down Expand Up @@ -527,10 +530,11 @@ catch settle-induced spikes.
All pan/zoom/circle/zigzag scenarios call `queue_unstable()` directly
— they never go through `FrameLoop.poll()`. This means they never
produce stable frames mid-interaction and cannot capture the jank
pattern where a stable frame interrupts slow panning. Only the
`frameloop` scenarios use the real `FrameLoop` decision path. When
investigating panning smoothness or adaptive timing, always use the
`frameloop` scenarios.
pattern where a stable frame interrupts slow panning or zooming. Only
the `frameloop` (pan) and `frameloop_zoom` scenarios use the real
`FrameLoop` decision path. When investigating panning or zooming
smoothness or adaptive timing, always use the `frameloop` /
`frameloop_zoom` scenarios.

### Stable frames must recapture caches

Expand Down
83 changes: 41 additions & 42 deletions crates/grida-canvas/src/runtime/scene.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,13 +273,11 @@ const COMPOSITOR_SURFACE_SIZE: i32 = 4096;
// No threshold-based invalidation — the settle frame handles full-quality
// rendering after the gesture ends.

/// Maximum zoom ratio (cached_zoom / current_zoom or inverse) before the
/// zoom image cache is invalidated. A ratio of 4.0 means we tolerate up to
/// 4× scale in either direction before the scaled texture becomes too blurry
/// or aliased. This is aggressive but acceptable during active interaction
/// (unstable frames) — the stable frame after interaction ends always
/// produces a full-quality render at the correct zoom level.
const ZOOM_IMAGE_CACHE_MAX_RATIO: f32 = 4.0;
// NOTE: The zoom image cache no longer has a hard eviction ratio. During
// active interaction, the cached texture is stretched at any zoom ratio —
// blurry content is acceptable and avoids catastrophic full-draw spikes.
// The settle frame always produces a full-quality render at the correct
// zoom level. See optimization.md item 21/22.

/// Cached GPU snapshot of the composited frame for pan-only fast path.
///
Expand Down Expand Up @@ -309,8 +307,6 @@ struct PanImageCache {
struct ZoomImageCache {
/// GPU texture snapshot of the composited frame.
image: Image,
/// Zoom level at capture time.
zoom: f32,
/// Full view matrix at capture time (includes translation + zoom).
view_matrix: math2::transform::AffineTransform,
}
Expand Down Expand Up @@ -1100,11 +1096,18 @@ impl Renderer {
}

// --- Try zoom image cache fast path (no plan needed) ---
if !stable
&& self.backend.is_gpu()
&& self.zoom_image_cache.is_some()
&& camera_change.zoom_changed()
{
//
// Use the zoom cache for:
// - Zoom-change frames (the primary use case during active zooming)
// - No-change frames when a zoom cache exists (zoom steps may
// quantize to identical values at gesture bounds, producing a
// no-change frame — blitting the existing cache is correct and
// avoids a catastrophic full draw on large scenes)
//
// Exclude pan-only frames — pan has its own faster blit cache.
let use_zoom_cache = self.zoom_image_cache.is_some()
&& (camera_change.zoom_changed() || camera_change == CameraChangeKind::None);
if !stable && self.backend.is_gpu() && use_zoom_cache {
let zoom_cache_hit = self.try_zoom_cache_blit(
surface,
scene,
Expand Down Expand Up @@ -1243,20 +1246,14 @@ impl Renderer {
//
// Triggers on:
// - Zoom-change frames (the primary use case during active zooming)
// - No-change frames during zoom gestures (zoom steps may quantize
// to identical values at bounds, but we still have a valid cache)
// - No-change frames when a zoom cache exists (zoom steps may
// quantize to identical values at gesture bounds, but we still
// have a valid cache — blitting it avoids catastrophic full draws)
//
// The image is rendered at a stale zoom level, so text and fine
// details are slightly blurry — acceptable during active interaction.
// The stable frame after interaction ends always does a full redraw.
// Don't use zoom cache for pan-only or no-change frames — pan has its
// own faster cache, and no-change frames (e.g. scene mutations without
// camera movement) must not replay a stale zoom snapshot.
if !plan.stable
&& self.backend.is_gpu()
&& self.zoom_image_cache.is_some()
&& plan.camera_change.zoom_changed()
{
// Excludes pan-only frames — those use the dedicated pan cache.
let zoom_cache_usable = self.zoom_image_cache.is_some()
&& (plan.camera_change.zoom_changed() || plan.camera_change == CameraChangeKind::None);
if !plan.stable && self.backend.is_gpu() && zoom_cache_usable {
let zoom_cache_hit = self.try_zoom_cache_blit(surface, scene, &plan);
if let Some((mid_flush_duration, frame_duration)) = zoom_cache_hit {
return FrameFlushStats {
Expand Down Expand Up @@ -1351,7 +1348,6 @@ impl Renderer {
// re-drawing.
self.zoom_image_cache = Some(ZoomImageCache {
image,
zoom: self.camera.get_zoom(),
view_matrix: vm,
});
}
Expand Down Expand Up @@ -1398,16 +1394,17 @@ impl Renderer {
_plan: &FramePlan,
) -> Option<(Duration, Duration)> {
let cache = self.zoom_image_cache.as_ref()?;
let current_zoom = self.camera.get_zoom();
let zoom_ratio = current_zoom / cache.zoom;

// Only use cache if zoom ratio is within acceptable range.
if !((1.0 / ZOOM_IMAGE_CACHE_MAX_RATIO)..=ZOOM_IMAGE_CACHE_MAX_RATIO).contains(&zoom_ratio)
{
// Too extreme — invalidate and fall through.
self.zoom_image_cache = None;
return None;
}
// Never evict the zoom cache during active interaction — even at
// extreme ratios the scaled blit is O(1) and avoids catastrophic
// frame spikes (50-60 ms full draws on large scenes). The settle
// frame always produces a full-quality render at the correct zoom.
//
// At ratios beyond ZOOM_IMAGE_CACHE_SOFT_RATIO the stretched
// texture is visibly blurry, but this is acceptable during fast
// interaction. Chromium's compositor uses the same strategy:
// stale tiles are stretched during pinch-zoom and re-rasterized
// asynchronously after the gesture ends.

let inv_cached = cache.view_matrix.inverse()?;
let cur_vm = self.camera.view_matrix();
Expand Down Expand Up @@ -1614,12 +1611,14 @@ impl Renderer {
let can_defer = !stable
&& self.backend.is_gpu()
&& (
// No content or camera change — overlay-only (marquee, hover)
(!camera_change.any_changed() && self.pan_image_cache.is_some())
// Pan cache will likely hit
|| (camera_change == CameraChangeKind::PanOnly && self.pan_image_cache.is_some())
// Zoom cache will likely hit
// Pan cache will likely hit (pan-only or overlay-only with pan cache)
(camera_change == CameraChangeKind::PanOnly && self.pan_image_cache.is_some())
// Zoom cache will likely hit (zoom change or no-change with zoom cache)
|| (camera_change.zoom_changed() && self.zoom_image_cache.is_some())
// No-change: prefer zoom cache (covers mid-zoom zero-delta frames),
// fall back to pan cache (covers overlay-only marquee/hover frames)
|| (camera_change == CameraChangeKind::None
&& (self.zoom_image_cache.is_some() || self.pan_image_cache.is_some()))
);

if can_defer {
Expand Down
8 changes: 7 additions & 1 deletion crates/grida-canvas/src/window/application.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1450,9 +1450,15 @@ impl UnknownTargetApplication {
// flushing so the picture cache is invalidated and the new content
// is re-recorded. In the frame() path this happens automatically;
// in the redraw() path (native host) we must do it explicitly.
//
// Use unstable (stable=false) when a camera change is active — this
// preserves the zoom/pan image caches and allows the fast blit paths.
// Without this, every zoom frame would nuke the zoom cache and force
// a full O(N) draw, causing ~3 FPS on large scenes.
{
let camera_change = self.renderer.camera.change_kind();
self.renderer.apply_changes(camera_change, true);
let stable = !camera_change.any_changed();
self.renderer.apply_changes(camera_change, stable);
Comment on lines +1453 to +1461
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python <<'PY'
from pathlib import Path
import re

redraw = re.compile(r'\.\s*redraw_requested\s*\(')
frame = re.compile(r'\.\s*frame\s*\(')

for path in Path('.').rglob('*.rs'):
    text = path.read_text(errors='ignore')
    if redraw.search(text):
        redraw_hits = [text.count('\n', 0, m.start()) + 1 for m in redraw.finditer(text)]
        frame_hits = [text.count('\n', 0, m.start()) + 1 for m in frame.finditer(text)]
        print(f'== {path} ==')
        print(f'  redraw_requested calls: {redraw_hits}')
        print(f'  frame calls: {frame_hits or "NONE"}')
        print()
PY

Repository: gridaco/grida

Length of output: 288

🏁 Script executed:

head -20 crates/grida-dev/src/platform/native_application.rs && echo "..." && sed -n '360,370p' crates/grida-dev/src/platform/native_application.rs

Repository: gridaco/grida

Length of output: 1130

🏁 Script executed:

# Check the broader context around line 364 in native_application.rs
sed -n '355,375p' crates/grida-dev/src/platform/native_application.rs

Repository: gridaco/grida

Length of output: 723

🏁 Script executed:

# Check if grida-dev is a dev/test crate or part of the main application
head -30 crates/grida-dev/Cargo.toml

Repository: gridaco/grida

Length of output: 785

Potential redraw-only regression in dev application: verify settle-frame expectation.

The script confirms that crates/grida-dev/src/platform/native_application.rs (lines 364) calls redraw_requested() without a corresponding frame() path. While this is development/test code (publish = false), line 1460's change to make stable depend on camera_change.any_changed() means an unstable frame could be rendered if no subsequent stable settle is guaranteed. The continuous winit event loop makes this less likely to manifest, but the architectural risk remains: any host that relies only on redraw_requested() → redraw() for interaction rendering will not trigger the full-quality settle recapture that the new zoom-cache optimization assumes.

Verify whether this dev integration path is expected to support the new cache behavior, or if this regression is accepted for test code only.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@crates/grida-canvas/src/window/application.rs` around lines 1453 - 1461, The
change makes the `stable` flag (computed from
`self.renderer.camera.change_kind().any_changed()`) drive
`self.renderer.apply_changes(camera_change, stable)`, which can leave dev-only
paths that call `redraw_requested()` without a corresponding full `frame()` in
an unstable state; verify and ensure the dev integration either triggers a
settle/full frame after camera changes or adjust the logic: either (A) ensure
the dev path in `crates/grida-dev/src/platform/native_application.rs` schedules
a follow-up `frame()`/settle when `camera_change.any_changed()` is true, or (B)
modify `apply_changes` call-site to treat redraw-only calls as stable for
recapture (or add a guard) so zoom/pan caches are recaptured—look for the
symbols `camera.change_kind()`, `camera_change.any_changed()`,
`self.renderer.apply_changes(...)`, and the `redraw_requested()` path and make
the chosen fix consistent across dev/test code paths.

}

let __frame_start = std::time::Instant::now();
Expand Down
Loading
Loading