From c81def7e435f10a44e6825c087e663c49aed4b5b Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 11:17:51 -0700 Subject: [PATCH 01/16] Avoid redundant property invalidation and attribute widget work --- draw/src/turtle.rs | 4 +- platform/src/draw_vars.rs | 91 +++++++++++++++++++++------ platform/src/event/finger.rs | 2 +- platform/src/perf_monitor.rs | 117 ++++++++++++++++++++++++++++++++++- widgets/src/image.rs | 3 + widgets/src/image_cache.rs | 2 +- widgets/src/text_input.rs | 12 ++++ widgets/src/view.rs | 3 + widgets/src/widget.rs | 74 +++++++++++++++++++++- 9 files changed, 281 insertions(+), 27 deletions(-) diff --git a/draw/src/turtle.rs b/draw/src/turtle.rs index c7242cb3da..14528ebd0c 100644 --- a/draw/src/turtle.rs +++ b/draw/src/turtle.rs @@ -88,7 +88,7 @@ pub enum AlignEntry { } /// Specifies how a turtle should walk. -#[derive(Copy, Clone, Default, Debug, Script, ScriptHook)] +#[derive(Copy, Clone, Default, Debug, PartialEq, Script, ScriptHook)] pub struct Walk { #[doc(hidden)] #[live] @@ -254,7 +254,7 @@ impl Walk { } } -#[derive(Copy, Clone, Debug, Script, ScriptHook)] +#[derive(Copy, Clone, Debug, PartialEq, Script, ScriptHook)] pub struct Metrics { #[live] pub descender: f64, diff --git a/platform/src/draw_vars.rs b/platform/src/draw_vars.rs index 030904edde..888447fe91 100644 --- a/platform/src/draw_vars.rs +++ b/platform/src/draw_vars.rs @@ -130,6 +130,35 @@ impl ScriptHook for DrawVars { } impl DrawVars { + /// Instance slots can carry packed integer bits, including NaN payloads. + /// Preserve signed zero and bit patterns while avoiding identical GPU writes. + fn copy_changed_slots( + target: &mut [f32], + value: &[f32], + perf: &mut crate::perf_monitor::PerfMonitor, + ) -> bool { + debug_assert_eq!(target.len(), value.len()); + let changed = !target + .iter() + .zip(value) + .all(|(a, b)| a.to_bits() == b.to_bits()); + if perf.enabled() { + perf.count_work( + if changed { + "shader.write.changed" + } else { + "shader.write.unchanged" + }, + "DrawVars", + 1, + ); + } + if changed { + target.copy_from_slice(value); + } + changed + } + /// Loud report for a failed draw-shader compile. A shader that fails /// must never fall back silently: the draw is skipped from then on and /// the only on-screen symptom is a flat clear-color region where the @@ -243,12 +272,13 @@ impl DrawVars { let key: ScriptValue = input.id.into(); if obj_map.contains_key(&key) { for j in 0..repeat { - for i in 0..input.slots { - instances[input.offset + i + j * stride] = - inst_slice[input.offset + i] - } + let offset = input.offset + j * stride; + any_updated |= Self::copy_changed_slots( + &mut instances[offset..offset + input.slots], + &inst_slice[input.offset..input.offset + input.slots], + &mut cx.perf_monitor, + ); } - any_updated = true; } } @@ -288,11 +318,12 @@ impl DrawVars { for input in &sh.mapping.dyn_uniforms.inputs { let key: ScriptValue = input.id.into(); if obj_map.contains_key(&key) { - for i in 0..input.slots { - draw_call.dyn_uniforms[input.offset + i] = - self.dyn_uniforms[input.offset + i] - } - any_updated = true; + let range = input.offset..input.offset + input.slots; + any_updated |= Self::copy_changed_slots( + &mut draw_call.dyn_uniforms[range.clone()], + &self.dyn_uniforms[range], + &mut cx.perf_monitor, + ); } } @@ -525,8 +556,12 @@ impl DrawVars { return; }; - for i in 0..slots { - draw_call.dyn_uniforms[offset + i] = value[i]; + if !Self::copy_changed_slots( + &mut draw_call.dyn_uniforms[offset..offset + slots], + &value[..slots], + &mut cx.perf_monitor, + ) { + return; } draw_call.mark_uniforms_dirty(uniforms_gen); if let Some(pass_id) = draw_list.draw_pass_id { @@ -570,7 +605,14 @@ impl DrawVars { if draw_call.draw_shader_id != draw_shader_id { return false; } - draw_call.dyn_uniforms[offset..offset + slots].copy_from_slice(&value[..slots]); + if !Self::copy_changed_slots( + &mut draw_call.dyn_uniforms[offset..offset + slots], + &value[..slots], + &mut cx.perf_monitor, + ) { + // Success means a valid target, including an already-current value. + return true; + } draw_call.mark_uniforms_dirty(uniforms_gen); if let Some(pass_id) = draw_list.draw_pass_id { cx.passes[pass_id].paint_dirty = true; @@ -677,8 +719,12 @@ impl DrawVars { if draw_call.draw_shader_id != draw_shader_id { continue; } - for i in 0..slots { - draw_call.dyn_uniforms[offset + i] = value[i]; + if !Self::copy_changed_slots( + &mut draw_call.dyn_uniforms[offset..offset + slots], + &value[..slots], + &mut cx.perf_monitor, + ) { + continue; } draw_call.mark_uniforms_dirty(Cx::next_uniform_gen_from(uniform_gen)); touched = true; @@ -728,13 +774,18 @@ impl DrawVars { let instances = &mut all_instances[inst.instance_offset..]; - // Update all instances in this area + let mut changed = false; for j in 0..inst.instance_count { - for i in 0..slots { - instances[input.offset + i + j * stride] = value[i]; - } + let offset = input.offset + j * stride; + changed |= Self::copy_changed_slots( + &mut instances[offset..offset + slots], + &value[..slots], + &mut cx.perf_monitor, + ); + } + if !changed { + return; } - draw_call.instance_dirty = true; if let Some(pass_id) = draw_list.draw_pass_id { cx.passes[pass_id].paint_dirty = true; diff --git a/platform/src/event/finger.rs b/platform/src/event/finger.rs index b2ec3073bc..d9de2fc60b 100644 --- a/platform/src/event/finger.rs +++ b/platform/src/event/finger.rs @@ -170,7 +170,7 @@ pub struct TouchUpdateEvent { /// Inset represents spacing values for all four edges (left, top, right, bottom). /// Used for both margin (outer spacing) and padding (inner spacing). -#[derive(Clone, Copy, Default, Debug, Script)] +#[derive(Clone, Copy, Default, Debug, PartialEq, Script)] pub struct Inset { /// The left inset. #[live] diff --git a/platform/src/perf_monitor.rs b/platform/src/perf_monitor.rs index 032c4fd83a..62d4eaa751 100644 --- a/platform/src/perf_monitor.rs +++ b/platform/src/perf_monitor.rs @@ -15,10 +15,32 @@ //! Collection is off until something (normally the PerfGraph widget) calls //! `set_enabled(true)`; disabled adds are a single branch. -use std::{cell::Cell, rc::Rc}; +use std::{cell::Cell, collections::HashMap, rc::Rc}; pub const PERF_MONITOR_HISTORY: usize = 240; pub const PERF_MONITOR_MAX_CHANNELS: usize = 12; +pub const PERF_MONITOR_MAX_WORK: usize = 256; + +/// Aggregate by operation and static component type, never by user content or +/// instance ID. Scrolling through new widgets cannot grow this table unbounded. +#[derive(Clone)] +pub struct PerfWorkSample { + pub operation: &'static str, + pub component: &'static str, + pub calls: u64, + pub total_ns: u64, + pub self_ns: u64, + pub max_ns: u64, +} + +/// A main-thread measurement. Finish descendants before their parents. +pub struct PerfWorkToken { + slot: usize, + started: f64, + completed_self_ns: u64, + generation: u64, + owner: Rc>, +} /// GPU completion handlers run off-thread; they park each presented frame's /// GPU time here and the next `frame_boundary` folds it into the ring. @@ -79,6 +101,11 @@ pub struct PerfMonitor { event_deduct: u32, /// Window repaints seen since enabling (see `frames_painted`). frames_painted: u64, + work: Vec, + work_index: HashMap<(&'static str, &'static str), usize>, + completed_self_ns: u64, + work_generation: u64, + work_overflow: u64, } impl Default for PerfMonitor { @@ -100,12 +127,20 @@ impl Default for PerfMonitor { event_depth: Rc::new(Cell::new(0)), event_deduct: 0, frames_painted: 0, + work: Vec::new(), + work_index: HashMap::new(), + completed_self_ns: 0, + work_generation: 0, + work_overflow: 0, } } } impl PerfMonitor { pub fn set_enabled(&mut self, on: bool) { + if self.enabled != on { + self.work_generation = self.work_generation.wrapping_add(1); + } self.enabled = on; GPU_COLLECT.store(on, std::sync::atomic::Ordering::Relaxed); if !on { @@ -120,6 +155,86 @@ impl PerfMonitor { self.enabled } + fn work_slot(&mut self, operation: &'static str, component: &'static str) -> Option { + if !self.enabled { + return None; + } + let key = (operation, component); + if let Some(slot) = self.work_index.get(&key) { + return Some(*slot); + } + if self.work.len() == PERF_MONITOR_MAX_WORK { + self.work_overflow = self.work_overflow.saturating_add(1); + return None; + } + let slot = self.work.len(); + self.work.push(PerfWorkSample { + operation, + component, + calls: 0, + total_ns: 0, + self_ns: 0, + max_ns: 0, + }); + self.work_index.insert(key, slot); + Some(slot) + } + + /// Disabled measurements neither read the clock nor allocate. Component + /// names should be static type names, not per-message identifiers. + pub fn begin_work( + &mut self, + operation: &'static str, + component: &'static str, + ) -> Option { + let slot = self.work_slot(operation, component)?; + Some(PerfWorkToken { + slot, + started: crate::cx::Cx::monotonic_now(), + completed_self_ns: self.completed_self_ns, + generation: self.work_generation, + owner: self.event_depth.clone(), + }) + } + + pub fn end_work(&mut self, token: Option) { + let Some(token) = token else { return }; + if !self.enabled + || token.generation != self.work_generation + || !Rc::ptr_eq(&token.owner, &self.event_depth) + { + return; + } + let elapsed = ((crate::cx::Cx::monotonic_now() - token.started).max(0.0) * 1e9) as u64; + // Descendant exclusive times sum to their whole subtree exactly once. + let own = elapsed.saturating_sub( + self.completed_self_ns + .saturating_sub(token.completed_self_ns), + ); + let sample = &mut self.work[token.slot]; + sample.calls = sample.calls.saturating_add(1); + sample.total_ns = sample.total_ns.saturating_add(elapsed); + sample.self_ns = sample.self_ns.saturating_add(own); + sample.max_ns = sample.max_ns.max(elapsed); + self.completed_self_ns = self.completed_self_ns.saturating_add(own); + } + + /// Count work without timing it. Counter rows have zero timing fields. + pub fn count_work(&mut self, operation: &'static str, component: &'static str, count: u64) { + if let Some(slot) = self.work_slot(operation, component) { + self.work[slot].calls = self.work[slot].calls.saturating_add(count); + } + } + + pub fn work(&self) -> &[PerfWorkSample] { + &self.work + } + + /// Measurements omitted because the bounded operation/type table is full. + pub fn work_overflow(&self) -> u64 { + self.work_overflow + } + /// Number of window repaints (frame boundaries) recorded while enabled. /// Lets a scripted driver pace itself to presented frames instead of /// queueing pass renders faster than the GPU retires them. diff --git a/widgets/src/image.rs b/widgets/src/image.rs index ad05583431..6699756ecf 100644 --- a/widgets/src/image.rs +++ b/widgets/src/image.rs @@ -229,6 +229,9 @@ impl Image { /// useful for widgets owned by an isolated VM: their typed Rust state can /// be changed safely even while the host VM is active. pub fn set_walk_and_fit(&mut self, cx: &mut Cx, walk: Walk, fit: ImageFit) { + if self.walk == walk && self.fit == fit { + return; + } self.walk = walk; self.fit = fit; self.redraw(cx); diff --git a/widgets/src/image_cache.rs b/widgets/src/image_cache.rs index ca3d2d5a9a..42025c5f84 100644 --- a/widgets/src/image_cache.rs +++ b/widgets/src/image_cache.rs @@ -1,6 +1,6 @@ use crate::makepad_draw::*; -#[derive(Clone, Copy, Debug, Default, Script, ScriptHook)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Script, ScriptHook)] pub enum ImageFit { /// Draws the image into the widget's requested bounds without preserving /// the image's aspect ratio. diff --git a/widgets/src/text_input.rs b/widgets/src/text_input.rs index ea38b01963..4090fbec3b 100644 --- a/widgets/src/text_input.rs +++ b/widgets/src/text_input.rs @@ -742,6 +742,9 @@ impl TextInput { } pub fn set_is_multiline(&mut self, cx: &mut Cx, is_multiline: bool) { + if self.is_multiline == is_multiline { + return; + } self.is_multiline = is_multiline; if !is_multiline { self.scroll_y = 0.0; @@ -816,6 +819,9 @@ impl TextInput { /// the wrong text. Overflow is clipped, so pick a whole number of lines or /// the last one is sliced through the middle of its glyphs. pub fn set_height(&mut self, cx: &mut Cx, height: Size) { + if self.walk.height == height { + return; + } self.walk.height = height; self.draw_bg.redraw(cx); } @@ -842,6 +848,9 @@ impl TextInput { } pub fn set_is_password(&mut self, cx: &mut Cx, is_password: bool) { + if self.is_password == is_password { + return; + } self.is_password = is_password; self.laidout_text = None; self.draw_bg.redraw(cx); @@ -877,6 +886,9 @@ impl TextInput { } pub fn set_is_numeric_only(&mut self, cx: &mut Cx, is_numeric_only: bool) { + if self.is_numeric_only == is_numeric_only { + return; + } self.is_numeric_only = is_numeric_only; self.laidout_text = None; self.draw_bg.redraw(cx); diff --git a/widgets/src/view.rs b/widgets/src/view.rs index ad333ec3e3..035fd508b6 100644 --- a/widgets/src/view.rs +++ b/widgets/src/view.rs @@ -373,6 +373,9 @@ impl ViewRef { /// Updates this view's typed walk without evaluating script. pub fn set_walk(&self, cx: &mut Cx, walk: Walk) { if let Some(mut inner) = self.borrow_mut() { + if inner.walk == walk { + return; + } inner.walk = walk; inner.redraw(cx); } diff --git a/widgets/src/widget.rs b/widgets/src/widget.rs index b6e496a1b1..f719026add 100644 --- a/widgets/src/widget.rs +++ b/widgets/src/widget.rs @@ -26,6 +26,10 @@ impl WidgetUid { } pub trait WidgetNode: ScriptApply { + /// Static component name for opt-in diagnostics; contains no UI content. + fn widget_type_name(&self) -> &'static str { + std::any::type_name::() + } fn widget_uid(&self) -> WidgetUid; /// Enumerate direct children for widget-tree indexing. fn children(&self, _visit: &mut dyn FnMut(LiveId, WidgetRef)) {} @@ -1120,7 +1124,14 @@ impl WidgetRef { let depth = cx.nesting_depth; cx.sploded_note_depth(uid.0, depth); } + let perf = if cx.perf_monitor.enabled() { + cx.perf_monitor + .begin_work("widget.draw", inner.widget.widget_type_name()) + } else { + None + }; let step = inner.widget.draw_walk(cx, scope, walk).step(); + cx.perf_monitor.end_work(perf); cx.exit_nesting_depth(); if let Some(nd) = step { if nd.is_empty() { @@ -1143,7 +1154,14 @@ impl WidgetRef { let depth = cx.nesting_depth; cx.sploded_note_depth(uid.0, depth); } + let perf = if cx.perf_monitor.enabled() { + cx.perf_monitor + .begin_work("widget.draw", inner.widget.widget_type_name()) + } else { + None + }; inner.widget.draw_walk_all(cx, scope, walk); + cx.perf_monitor.end_work(perf); cx.exit_nesting_depth(); } } @@ -1174,7 +1192,14 @@ impl WidgetRef { let depth = cx.nesting_depth; cx.sploded_note_depth(uid.0, depth); } + let perf = if cx.perf_monitor.enabled() { + cx.perf_monitor + .begin_work("widget.draw", inner.widget.widget_type_name()) + } else { + None + }; let step = inner.widget.draw(cx, scope).step(); + cx.perf_monitor.end_work(perf); cx.exit_nesting_depth(); if let Some(nd) = step { if nd.is_empty() { @@ -1194,7 +1219,14 @@ impl WidgetRef { let depth = cx.nesting_depth; cx.sploded_note_depth(uid.0, depth); } + let perf = if cx.perf_monitor.enabled() { + cx.perf_monitor + .begin_work("widget.draw", inner.widget.widget_type_name()) + } else { + None + }; let step = inner.widget.draw(cx, &mut Scope::empty()).step(); + cx.perf_monitor.end_work(perf); cx.exit_nesting_depth(); if let Some(nd) = step { if nd.is_empty() { @@ -1286,7 +1318,14 @@ impl WidgetRef { pub fn draw_all(&self, cx: &mut Cx2d, scope: &mut Scope) { if let Some(inner) = self.0.borrow_mut().as_mut() { - return inner.widget.draw_all(cx, scope); + let perf = if cx.perf_monitor.enabled() { + cx.perf_monitor + .begin_work("widget.draw", inner.widget.widget_type_name()) + } else { + None + }; + inner.widget.draw_all(cx, scope); + cx.perf_monitor.end_work(perf); } } @@ -1306,7 +1345,14 @@ impl WidgetRef { pub fn draw_all_unscoped(&self, cx: &mut Cx2d) { if let Some(inner) = self.0.borrow_mut().as_mut() { - return inner.widget.draw_all_unscoped(cx); + let perf = if cx.perf_monitor.enabled() { + cx.perf_monitor + .begin_work("widget.draw", inner.widget.widget_type_name()) + } else { + None + }; + inner.widget.draw_all_unscoped(cx); + cx.perf_monitor.end_work(perf); } } @@ -1372,6 +1418,30 @@ impl WidgetRef { apply: &Apply, scope: &mut Scope, value: ScriptValue, + ) { + let perf = if vm.cx_mut().perf_monitor.enabled() { + let component = self + .0 + .borrow() + .as_ref() + .map(|inner| inner.widget.widget_type_name()) + .unwrap_or("WidgetRef"); + vm.cx_mut() + .perf_monitor + .begin_work("widget.apply", component) + } else { + None + }; + self.script_apply_inner(vm, apply, scope, value); + vm.cx_mut().perf_monitor.end_work(perf); + } + + fn script_apply_inner( + &self, + vm: &mut ScriptVm, + apply: &Apply, + scope: &mut Scope, + value: ScriptValue, ) { let mut inner = self.0.borrow_mut(); let mut set_ui_after_apply = false; From ad41689f070e54ee5140972c795a8e21564864a1 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 12:26:02 -0700 Subject: [PATCH 02/16] Report Android GL frames and honor app drawable captures --- platform/src/os/linux/android/android.rs | 20 ++++++++++ platform/src/os/linux/opengl.rs | 50 ++++++++++++++++++++++++ platform/src/os/linux/opengl_cx.rs | 46 +++------------------- 3 files changed, 75 insertions(+), 41 deletions(-) diff --git a/platform/src/os/linux/android/android.rs b/platform/src/os/linux/android/android.rs index 96cec9751e..58cb5f5e27 100644 --- a/platform/src/os/linux/android/android.rs +++ b/platform/src/os/linux/android/android.rs @@ -1786,6 +1786,7 @@ impl Cx { display.surface, ); if swapped != 0 { + self.perf_monitor.frame_boundary(Cx::monotonic_now()); self.hide_android_surface_cover_after_first_present_if_needed(); self.request_android_surface_snapshot_refresh_after_present_if_needed(); } @@ -1813,6 +1814,7 @@ impl Cx { self.perf_monitor.add(crate::perf_monitor::PERF_CHANNEL_DRAWABLE_WAIT, us); } if swapped != 0 { + self.perf_monitor.frame_boundary(Cx::monotonic_now()); self.hide_android_surface_cover_after_first_present_if_needed(); self.request_android_surface_snapshot_refresh_after_present_if_needed(); } @@ -2623,6 +2625,24 @@ impl Cx { vertex_buffer_bytes: metrics.vertex_buffer_bytes, texture_bytes: metrics.texture_bytes, })); + // Android has its own GL window path, so it must also drain + // explicit app-frame captures before swapping the surface. + #[cfg(not(use_vulkan))] + if self.os.has_drawable_surface() { + self.opengl_capture_window( + Some(window_id.id()), + self.os.display_size.x as u32, + self.os.display_size.y as u32, + ); + } + #[cfg(use_vulkan)] + if self.os.vulkan.is_none() && self.os.has_drawable_surface() { + self.opengl_capture_window( + Some(window_id.id()), + self.os.display_size.x as u32, + self.os.display_size.y as u32, + ); + } self.present_window_for_active_backend(); } CxDrawPassParent::DrawPass(_) => { diff --git a/platform/src/os/linux/opengl.rs b/platform/src/os/linux/opengl.rs index 0905297aea..a7afddd351 100644 --- a/platform/src/os/linux/opengl.rs +++ b/platform/src/os/linux/opengl.rs @@ -356,6 +356,56 @@ mod skipped_draws { } impl Cx { + /// Capture only when explicitly requested, after the full window (including + /// popup overlays) has been drawn and before its backbuffer is swapped. + #[cfg(not(target_env = "ohos"))] + pub(crate) fn opengl_capture_window( + &mut self, + capture_window_id: Option, + w: u32, + h: u32, + ) { + // Studio screenshot readback: read framebuffer pixels before swap. + // The drain names this pass's window: every `--remote` grab targets a + // window, and a drain that names none never answers it. + let request_ids = self.take_studio_screenshot_request_ids_for_window(0, capture_window_id); + // A continuous capture sink (the ScreenCap recorder) is standing + // permission rather than a queued request, so it is asked separately. + let wants_capture = crate::screen_capture::capture_wants_window(capture_window_id); + if !request_ids.is_empty() || wants_capture { + let mut pixels = vec![0u8; (w * h * 4) as usize]; + unsafe { + let gl = self.os.gl(); + (gl.glReadPixels)( + 0, + 0, + w as i32, + h as i32, + gl_sys::RGBA, + gl_sys::UNSIGNED_BYTE, + pixels.as_mut_ptr() as *mut _, + ); + } + // OpenGL reads bottom-up; flip rows for top-down PNG. + let stride = (w * 4) as usize; + for y in 0..(h as usize / 2) { + let top = y * stride; + let bot = ((h as usize) - 1 - y) * stride; + for x in 0..stride { + pixels.swap(top + x, bot + x); + } + } + crate::screen_capture::deliver_capture_frame(capture_window_id, w, h, &pixels); + // Encode as PNG. + if !request_ids.is_empty() { + if let Ok(png) = Self::encode_rgba_as_png(w, h, &pixels) { + Self::send_studio_screenshot_response(request_ids, w, h, png); + } + } + } + } + + /// Renderer-owned texture capture (see the metal backend): not /// implemented here — callers fall back to `debug_read_render_texture` /// (GL commands on one context are ordered, so the sync path is safe). diff --git a/platform/src/os/linux/opengl_cx.rs b/platform/src/os/linux/opengl_cx.rs index fc60816a7b..17cc00de7a 100644 --- a/platform/src/os/linux/opengl_cx.rs +++ b/platform/src/os/linux/opengl_cx.rs @@ -452,47 +452,11 @@ impl Cx { } } - // Studio screenshot readback: read framebuffer pixels before swap. - // The drain names this pass's window: every `--remote` grab targets a - // window, and a drain that names none never answers it. - let capture_window_id = self.get_pass_window_id(draw_pass_id).map(|w| w.id()); - let request_ids = self.take_studio_screenshot_request_ids_for_window(0, capture_window_id); - // A continuous capture sink (the ScreenCap recorder) is standing - // permission rather than a queued request, so it is asked separately. - let wants_capture = crate::screen_capture::capture_wants_window(capture_window_id); - if !request_ids.is_empty() || wants_capture { - let w = pix_width.floor() as u32; - let h = pix_height.floor() as u32; - let mut pixels = vec![0u8; (w * h * 4) as usize]; - unsafe { - let gl = self.os.gl(); - (gl.glReadPixels)( - 0, - 0, - w as i32, - h as i32, - gl_sys::RGBA, - gl_sys::UNSIGNED_BYTE, - pixels.as_mut_ptr() as *mut _, - ); - } - // OpenGL reads bottom-up; flip rows for top-down PNG. - let stride = (w * 4) as usize; - for y in 0..(h as usize / 2) { - let top = y * stride; - let bot = ((h as usize) - 1 - y) * stride; - for x in 0..stride { - pixels.swap(top + x, bot + x); - } - } - crate::screen_capture::deliver_capture_frame(capture_window_id, w, h, &pixels); - // Encode as PNG. - if !request_ids.is_empty() { - if let Ok(png) = Self::encode_rgba_as_png(w, h, &pixels) { - Self::send_studio_screenshot_response(request_ids, w, h, png); - } - } - } + self.opengl_capture_window( + self.get_pass_window_id(draw_pass_id).map(|w| w.id()), + pix_width.floor() as u32, + pix_height.floor() as u32, + ); #[cfg(target_os = "android")] if let Some(request) = self.take_studio_run_view_frame_request(0) { From bd324a87fd294e5e9a2e547851adaddf5d6caf4d Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:21:17 -0700 Subject: [PATCH 03/16] Attribute opt-in widget event dispatch costs --- widgets/src/widget.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/widgets/src/widget.rs b/widgets/src/widget.rs index f719026add..33e0168c06 100644 --- a/widgets/src/widget.rs +++ b/widgets/src/widget.rs @@ -728,7 +728,21 @@ impl WidgetRef { pub fn handle_event(&self, cx: &mut Cx, event: &Event, scope: &mut Scope) { if let Some(inner) = self.0.borrow_mut().as_mut() { + let perf = if cx.perf_monitor.enabled() { + let operation = match event { + Event::Actions(_) => "widget.event.actions", + Event::NextFrame(_) => "widget.event.next_frame", + Event::TouchUpdate(_) | Event::MouseMove(_) | Event::Scroll(_) => { + "widget.event.pointer" + } + _ => "widget.event.other", + }; + cx.perf_monitor.begin_work(operation, inner.widget.widget_type_name()) + } else { + None + }; inner.widget.handle_event(cx, event, scope); + cx.perf_monitor.end_work(perf); } } From 2c1f6e1919dac18a86149db5a8d7a6f909c711d2 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:32:13 -0700 Subject: [PATCH 04/16] Avoid refreshing PortalList topology when retention removes no rows --- widgets/src/portal_list.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/widgets/src/portal_list.rs b/widgets/src/portal_list.rs index a64f60a8ae..d51d113c6d 100644 --- a/widgets/src/portal_list.rs +++ b/widgets/src/portal_list.rs @@ -1256,6 +1256,7 @@ impl PortalList { // When a selection is active (but drag finished), keep selected items alive // so their selection state persists when scrolled back into view. if !self.keep_invisible && !self.is_selecting { + let previous_item_count = self.items.len(); let selection_range = self.get_selection_range(); self.cached_item_order.truncate(self.cache_items); if self.reuse_items { @@ -1274,7 +1275,12 @@ impl PortalList { } else { self.items.retain_visible(); } - cx.widget_tree_mark_dirty(self.uid); + // Retaining existing rows does not change the widget tree. New or + // replaced rows are registered by item_with_existed; this pass can + // only remove rows, so refresh the tree only when it did so. + if self.items.len() != previous_item_count { + cx.widget_tree_mark_dirty(self.uid); + } } cx.end_turtle_with_area(&mut self.area); From 817a1fc60ed824e76c26468a1f780ac816863c15 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:55:03 -0700 Subject: [PATCH 05/16] Stop closed modals from repainting transparent fullscreen overlays --- widgets/src/modal.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/widgets/src/modal.rs b/widgets/src/modal.rs index e5c40bec1d..2861f2907d 100644 --- a/widgets/src/modal.rs +++ b/widgets/src/modal.rs @@ -176,6 +176,16 @@ impl Widget for Modal { /// sized by the pass, so its geometry comes from `Walk::fill()` against /// that root — never from the slot a parent thought it was handing over. fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, _walk: Walk) -> DrawStep { + if !self.is_open { + // Clear a previously open overlay even when its parent draw list + // is retained. A closed modal needs no hit target: drawing the + // transparent background here repainted the whole window for each + // closed modal on every frame, despite producing no visible ink. + let draw_list = self.draw_list.as_mut().unwrap(); + draw_list.begin_overlay_reuse(cx); + draw_list.end(cx); + return DrawStep::done(); + } let bounds = cx.global::().0; // Bounded to part of the pass, the overlay also takes the view transform // of the list it opens from: a host may draw the app shifted or scaled From e7c4a075e3d3dd62306782cf25e8fe3098d1677b Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:21:07 -0700 Subject: [PATCH 06/16] Skip transparent fullscreen drawing for closed tooltips --- widgets/src/tooltip.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/widgets/src/tooltip.rs b/widgets/src/tooltip.rs index 9f587655ec..0a70833ad6 100644 --- a/widgets/src/tooltip.rs +++ b/widgets/src/tooltip.rs @@ -122,6 +122,12 @@ impl Widget for Tooltip { fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, _walk: Walk) -> DrawStep { let draw_list = self.draw_list.as_mut().unwrap(); draw_list.begin_overlay_reuse(cx); + // Retire a previously visible tip without painting a transparent + // fullscreen background while the tooltip is closed. + if !self.opened { + draw_list.end(cx); + return DrawStep::done(); + } let size = cx.current_pass_size(); cx.begin_root_turtle(size, self.view.layout); From b2b518f4d36a076220b6a86d0138b890704552c6 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:47:22 -0700 Subject: [PATCH 07/16] Skip Android Studio GPU metrics when no capture consumes them --- platform/src/os/linux/android/android.rs | 39 +++++++++++++++--------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/platform/src/os/linux/android/android.rs b/platform/src/os/linux/android/android.rs index 58cb5f5e27..d222f31a60 100644 --- a/platform/src/os/linux/android/android.rs +++ b/platform/src/os/linux/android/android.rs @@ -2595,8 +2595,17 @@ impl Cx { if self.windows[window_id].is_popup { continue; } - let start = self.seconds_since_app_start(); - let metrics = self.collect_gpu_pass_metrics(*draw_pass_id); + // Collecting Studio statistics walks the full draw tree and + // allocates geometry/texture inventories. Normal rendering + // has no consumer for this work; local captures still opt in. + let profile = (Cx::has_studio_web_socket() + || Cx::local_profile_capture_enabled()) + .then(|| { + ( + self.seconds_since_app_start(), + self.collect_gpu_pass_metrics(*draw_pass_id), + ) + }); self.draw_pass_to_window_for_active_backend(*draw_pass_id); // Draw popup window passes as overlays on the same surface @@ -2613,18 +2622,20 @@ impl Cx { } } - let end = self.seconds_since_app_start(); - Cx::send_studio_message(AppToStudio::GPUSample(GPUSample { - start, - end, - draw_calls: metrics.draw_calls, - instances: metrics.instances, - vertices: metrics.vertices, - instance_bytes: metrics.instance_bytes, - uniform_bytes: metrics.uniform_bytes, - vertex_buffer_bytes: metrics.vertex_buffer_bytes, - texture_bytes: metrics.texture_bytes, - })); + if let Some((start, metrics)) = profile { + let end = self.seconds_since_app_start(); + Cx::send_studio_message(AppToStudio::GPUSample(GPUSample { + start, + end, + draw_calls: metrics.draw_calls, + instances: metrics.instances, + vertices: metrics.vertices, + instance_bytes: metrics.instance_bytes, + uniform_bytes: metrics.uniform_bytes, + vertex_buffer_bytes: metrics.vertex_buffer_bytes, + texture_bytes: metrics.texture_bytes, + })); + } // Android has its own GL window path, so it must also drain // explicit app-frame captures before swapping the surface. #[cfg(not(use_vulkan))] From afd9467ec3d42bccc5a90a0cf2650a152439b487 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:47:22 -0700 Subject: [PATCH 08/16] Schedule Android rendering after batched View input --- platform/src/os/linux/android/android_jni.rs | 105 ++++-------------- platform/src/os/linux/android/ndk_sys.rs | 35 +----- .../dev/makepad/android/MakepadActivity.java | 18 ++- .../dev/makepad/android/MakepadNative.java | 3 +- 4 files changed, 41 insertions(+), 120 deletions(-) diff --git a/platform/src/os/linux/android/android_jni.rs b/platform/src/os/linux/android/android_jni.rs index 9936b01b83..e0111d4646 100644 --- a/platform/src/os/linux/android/android_jni.rs +++ b/platform/src/os/linux/android/android_jni.rs @@ -1,4 +1,3 @@ -use crate::module_loader::ModuleLoader; use makepad_jni_sys as jni_sys; use { @@ -798,107 +797,40 @@ unsafe fn create_native_window_with_env( ndk_sys::ANativeWindow_fromSurface(env, surface) } -#[cfg(not(no_android_choreographer))] -static mut CHOREOGRAPHER: *mut ndk_sys::AChoreographer = std::ptr::null_mut(); - -/// Whether a render loop (a Choreographer callback chain or the paced -/// thread) is running. Every activity instance's `onCreate` calls -/// `initChoreographer`, and a second instance can be created while the -/// first, and the app behind it, live on (`MakepadActivity.sNativeActivity`); -/// one loop is enough, two would double the beat. A loop clears this when -/// it stops, which it does once the message channel is gone. +/// One render beat for the native app, including activity replacements. Java's +/// Choreographer shares the View input phase; an independent NDK Choreographer +/// can run first and paint before that frame's batched MotionEvents arrive. static RENDER_LOOP_ACTIVE: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); -#[cfg(not(no_android_choreographer))] -static mut CHOREOGRAPHER_POST_CALLBACK_FN: Option< - unsafe extern "C" fn( - *mut ndk_sys::AChoreographer, - Option< - unsafe extern "C" fn( - *mut ndk_sys::AChoreographerFrameCallbackData, - *mut std::ffi::c_void, - ), - >, - *mut std::ffi::c_void, - ) -> i32, -> = None; - -/// Initializes the render loop which used the Android Choreographer when available to ensure proper vsync. -/// If `no_android_choreographer` is present (e.g. OHOS with non-compatiblity), we fallback to a simple loop with frame pacing. -/// This will be replaced by proper a vsync mechanism once we firgure it out for that OHOS. -#[allow(unused)] +/// Returns true when Java should start its Choreographer callback chain. +/// The no-Choreographer compatibility path retains its paced native thread. #[no_mangle] pub unsafe extern "C" fn Java_dev_makepad_android_MakepadNative_initChoreographer( _: *mut jni_sys::JNIEnv, _: jni_sys::jclass, device_refresh_rate: jni_sys::jfloat, - sdk_version: jni_sys::jint, -) { + _sdk_version: jni_sys::jint, +) -> jni_sys::jboolean { if RENDER_LOOP_ACTIVE.swap(true, std::sync::atomic::Ordering::SeqCst) { - return; + return 0; } - // If the Choreographer is not available (e.g. OHOS), use a manual render loop #[cfg(no_android_choreographer)] { init_simple_render_loop(device_refresh_rate); - return; + 0 } - #[allow(unused)] #[cfg(not(no_android_choreographer))] { - // Otherwise use the actual Choreographer. - CHOREOGRAPHER = ndk_sys::AChoreographer_getInstance(); - // AChoreographer_postFrameCallback64 (API 29) and - // AChoreographer_postVsyncCallback (API 33) must be resolved via dlsym, - // never declared as `extern "C"` — see the note in ndk_sys.rs. On API - // 26-28 neither symbol exists, so the callback fn stays None and we - // fall back to the manual frame loop below. - if sdk_version >= 29 { - if let Ok(lib) = ModuleLoader::load("libandroid.so") { - // Prefer the newer vsync callback (API 33+); fall back to the - // API 29 frame callback when it isn't available. - let vsync: Option = if sdk_version >= 33 { - lib.get_symbol("AChoreographer_postVsyncCallback").ok() - } else { - None - }; - let frame_callback_64: Option = - lib.get_symbol("AChoreographer_postFrameCallback64").ok(); - CHOREOGRAPHER_POST_CALLBACK_FN = vsync.or(frame_callback_64); - } - } - match CHOREOGRAPHER_POST_CALLBACK_FN { - Some(_) => post_vsync_callback(), - None => init_simple_render_loop(device_refresh_rate), - } + let _ = device_refresh_rate; + 1 } } -#[cfg(not(no_android_choreographer))] -unsafe extern "C" fn vsync_callback( - _data: *mut ndk_sys::AChoreographerFrameCallbackData, - _user_data: *mut std::ffi::c_void, -) { - send_from_java_message(FromJavaMessage::RenderLoop); - post_vsync_callback(); -} - -#[cfg(not(no_android_choreographer))] -pub unsafe fn post_vsync_callback() { - if let Some(post_callback) = CHOREOGRAPHER_POST_CALLBACK_FN { - if !CHOREOGRAPHER.is_null() && from_java_messages_already_set() { - post_callback(CHOREOGRAPHER, Some(vsync_callback), std::ptr::null_mut()); - return; - } - } - // The chain ends here: the next `initChoreographer` starts a new one. - RENDER_LOOP_ACTIVE.store(false, std::sync::atomic::Ordering::SeqCst); -} - /// Fallback render loop used when the Android Choreographer isn't available -/// (API < 29, and `no_android_choreographer` builds such as OHOS). A dedicated +/// (`no_android_choreographer` builds such as OHOS). A dedicated /// thread paces frames manually, since there is no system vsync callback. +#[cfg(no_android_choreographer)] fn init_simple_render_loop(device_refresh_rate: f32) { std::thread::spawn(move || { let mut last_frame_time = std::time::Instant::now(); @@ -907,7 +839,7 @@ fn init_simple_render_loop(device_refresh_rate: f32) { // Exit the thread once the app has shut down and the Java->native // message channel has been torn down by `from_java_messages_clear()` // (called when the main event loop quits). This mirrors - // `post_vsync_callback`, which likewise stops re-arming the + // Java's frame callback, which likewise stops re-arming the // Choreographer once `from_java_messages_already_set()` is false. // // Without this, the thread spins forever after the activity is @@ -1291,9 +1223,14 @@ extern "C" fn Java_dev_makepad_android_MakepadNative_surfaceOnSafeAreaInsets( #[no_mangle] extern "C" fn Java_dev_makepad_android_MakepadNative_onRenderLoop( _: *mut jni_sys::JNIEnv, - _: jni_sys::jobject, -) { + _: jni_sys::jclass, +) -> jni_sys::jboolean { + if !from_java_messages_already_set() { + RENDER_LOOP_ACTIVE.store(false, std::sync::atomic::Ordering::SeqCst); + return 0; + } send_from_java_message(FromJavaMessage::RenderLoop); + 1 } #[no_mangle] diff --git a/platform/src/os/linux/android/ndk_sys.rs b/platform/src/os/linux/android/ndk_sys.rs index 6597ce3a68..3b536a049a 100644 --- a/platform/src/os/linux/android/ndk_sys.rs +++ b/platform/src/os/linux/android/ndk_sys.rs @@ -58,8 +58,7 @@ extern "C" { // It is API 30+; a plain `extern "C"` strong reference to it would break // `dlopen` of libmakepad.so on API 26-29 devices. If surface frame-rate // control is wanted, resolve it via `dlsym(libandroid.so, ...)` gated on - // `sdk_version >= 30`, the same way the Choreographer post-callbacks are - // handled below. + // `sdk_version >= 30`, rather than linking newer symbols at the minSdk floor. // AHardwareBuffer_acquire / _release are API 26 — safe at the minSdk floor. pub fn AHardwareBuffer_acquire(buffer: *mut AHardwareBuffer); @@ -70,38 +69,6 @@ pub const AHARDWAREBUFFER_USAGE_CPU_READ_RARELY: u64 = 2; pub const AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN: u64 = 3; pub const AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE: u64 = 1 << 8; -pub type AChoreographer = c_void; -pub type AChoreographerFrameCallbackData = c_void; - -pub type AChoreographer_vsyncCallback = - unsafe extern "C" fn(callbackData: *mut AChoreographerFrameCallbackData, data: *mut c_void); - -/// The function type for posting callbacks to the AChoreographer -pub type AChoreographerPostCallbackFn = unsafe extern "C" fn( - *mut AChoreographer, - Option, - *mut std::ffi::c_void, -) -> i32; - -#[cfg(not(no_android_choreographer))] -extern "C" { - // AChoreographer_getInstance was introduced in API 24, so it's safe to - // link directly at our minSdk floor (26). - pub fn AChoreographer_getInstance() -> *mut AChoreographer; -} - -// AChoreographer_postVsyncCallback (API 33) and AChoreographer_postFrameCallback64 -// (API 29) are deliberately NOT declared in an `extern "C"` block. -// -// A strong undefined reference to a symbol that doesn't exist on API 26-28 -// devices makes the entire `libmakepad.so` fail to `dlopen` at process start -// (`UnsatisfiedLinkError: cannot locate symbol ...`), even if the call site is -// runtime-gated behind an `sdk_version >= 29` check — the relocation is still -// emitted. Rust's `extern "C"` can't express weak linkage on stable, so these -// are instead resolved at runtime with `dlsym(libandroid.so, ...)` in -// android_jni.rs, typed via `AChoreographerPostCallbackFn`, only on devices -// where they actually exist. - #[repr(C)] pub struct ANativeActivity { pub callbacks: *mut ANativeActivityCallbacks, diff --git a/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadActivity.java b/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadActivity.java index c00c756a6e..ce2d4d217a 100644 --- a/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadActivity.java +++ b/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadActivity.java @@ -49,6 +49,7 @@ import android.os.SystemClock; import android.util.Log; import android.view.ActionMode; +import android.view.Choreographer; import android.view.Display; import android.view.InputDevice; import android.view.Menu; @@ -1126,6 +1127,19 @@ public class MakepadActivity // would background the app and its onDestroy would shut it down), and // it finishes. private static MakepadActivity sNativeActivity; + // Use the same Choreographer as ViewRootImpl: its batched touch input runs + // before our animation-phase callback sends the native render beat. A + // separate NDK Choreographer can beat Java input and miss a scrolling frame. + // Static ownership avoids retaining an Activity or starting a second chain + // when a replacement Activity takes over the same native app. + private static final Choreographer.FrameCallback sRenderLoop = new Choreographer.FrameCallback() { + @Override + public void doFrame(long frameTimeNanos) { + if (MakepadNative.onRenderLoop()) { + Choreographer.getInstance().postFrameCallback(this); + } + } + }; private boolean mSuperseded; private final Handler mHandler = new Handler(Looper.getMainLooper()); private InputManager mInputManager; @@ -1607,7 +1621,9 @@ public void onCreate(Bundle savedInstanceState) { setVolumeControlStream(AudioManager.STREAM_MUSIC); float refreshRate = getDeviceRefreshRate(); - MakepadNative.initChoreographer(refreshRate, sdkVersion); + if (MakepadNative.initChoreographer(refreshRate, sdkVersion)) { + Choreographer.getInstance().postFrameCallback(sRenderLoop); + } //% MAIN_ACTIVITY_ON_CREATE } diff --git a/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java b/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java index 866ef68dcc..6e1b1f34b6 100644 --- a/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java +++ b/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java @@ -15,7 +15,8 @@ public class MakepadNative { public static native void onAndroidParams(String cache_path, String data_path, float density, boolean isEmulator, String androidVersion, String buildNumber, String kernelVersion); - public native static void initChoreographer(float deviceRefreshRate, int sdkVersion); + public native static boolean initChoreographer(float deviceRefreshRate, int sdkVersion); + public native static boolean onRenderLoop(); public native static void onBackPressed(); public native static void onHomeIntent(); From 22c65c8296945df59e9b2b3452d421df3de3d563 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:55:19 -0700 Subject: [PATCH 09/16] Remove unused NDK import after scheduler migration --- platform/src/os/linux/android/ndk_sys.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/platform/src/os/linux/android/ndk_sys.rs b/platform/src/os/linux/android/ndk_sys.rs index 3b536a049a..bf627b1ba2 100644 --- a/platform/src/os/linux/android/ndk_sys.rs +++ b/platform/src/os/linux/android/ndk_sys.rs @@ -2,7 +2,6 @@ #![allow(non_snake_case)] use makepad_jni_sys as jni_sys; -use std::ffi::c_void; #[repr(C)] #[derive(Debug, Copy, Clone)] From 98aa8ea3c5dca22029fe1aca61f928426e9f6dfd Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:58:43 -0700 Subject: [PATCH 10/16] Keep legacy Android packagers working with the View frame scheduler --- platform/src/os/linux/android/android_jni.rs | 130 ++++++++++++++++-- platform/src/os/linux/android/ndk_sys.rs | 36 ++++- .../dev/makepad/android/MakepadActivity.java | 18 +-- .../dev/makepad/android/MakepadNative.java | 21 ++- 4 files changed, 171 insertions(+), 34 deletions(-) diff --git a/platform/src/os/linux/android/android_jni.rs b/platform/src/os/linux/android/android_jni.rs index e0111d4646..2d6fabe215 100644 --- a/platform/src/os/linux/android/android_jni.rs +++ b/platform/src/os/linux/android/android_jni.rs @@ -1,3 +1,4 @@ +use crate::module_loader::ModuleLoader; use makepad_jni_sys as jni_sys; use { @@ -797,40 +798,139 @@ unsafe fn create_native_window_with_env( ndk_sys::ANativeWindow_fromSurface(env, surface) } -/// One render beat for the native app, including activity replacements. Java's -/// Choreographer shares the View input phase; an independent NDK Choreographer -/// can run first and paint before that frame's batched MotionEvents arrive. +#[cfg(not(no_android_choreographer))] +static mut CHOREOGRAPHER: *mut ndk_sys::AChoreographer = std::ptr::null_mut(); + +/// Whether a render loop (a Choreographer callback chain or the paced +/// thread) is running. Every activity instance's `onCreate` calls +/// `initChoreographer`, and a second instance can be created while the +/// first, and the app behind it, live on (`MakepadActivity.sNativeActivity`); +/// one loop is enough, two would double the beat. A loop clears this when +/// it stops, which it does once the message channel is gone. static RENDER_LOOP_ACTIVE: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); -/// Returns true when Java should start its Choreographer callback chain. -/// The no-Choreographer compatibility path retains its paced native thread. +#[cfg(not(no_android_choreographer))] +static mut CHOREOGRAPHER_POST_CALLBACK_FN: Option< + unsafe extern "C" fn( + *mut ndk_sys::AChoreographer, + Option< + unsafe extern "C" fn( + *mut ndk_sys::AChoreographerFrameCallbackData, + *mut std::ffi::c_void, + ), + >, + *mut std::ffi::c_void, + ) -> i32, +> = None; + +/// Initializes the render loop which used the Android Choreographer when available to ensure proper vsync. +/// If `no_android_choreographer` is present (e.g. OHOS with non-compatiblity), we fallback to a simple loop with frame pacing. +/// This will be replaced by proper a vsync mechanism once we firgure it out for that OHOS. +#[allow(unused)] #[no_mangle] pub unsafe extern "C" fn Java_dev_makepad_android_MakepadNative_initChoreographer( - _: *mut jni_sys::JNIEnv, - _: jni_sys::jclass, + env: *mut jni_sys::JNIEnv, + class: jni_sys::jclass, device_refresh_rate: jni_sys::jfloat, - _sdk_version: jni_sys::jint, -) -> jni_sys::jboolean { + sdk_version: jni_sys::jint, +) { if RENDER_LOOP_ACTIVE.swap(true, std::sync::atomic::Ordering::SeqCst) { - return 0; + return; + } + // ViewRootImpl batches touch input on Java's Choreographer. Prefer its + // animation phase so the render beat follows that input in the same frame. + // Keep the existing JNI signature and NDK fallback for older packagers. + #[cfg(not(no_android_choreographer))] + if start_java_choreographer(env, class) { + return; } + // If the Choreographer is not available (e.g. OHOS), use a manual render loop #[cfg(no_android_choreographer)] { init_simple_render_loop(device_refresh_rate); - 0 + return; } + #[allow(unused)] #[cfg(not(no_android_choreographer))] { - let _ = device_refresh_rate; - 1 + // Otherwise use the actual Choreographer. + CHOREOGRAPHER = ndk_sys::AChoreographer_getInstance(); + // AChoreographer_postFrameCallback64 (API 29) and + // AChoreographer_postVsyncCallback (API 33) must be resolved via dlsym, + // never declared as `extern "C"` — see the note in ndk_sys.rs. On API + // 26-28 neither symbol exists, so the callback fn stays None and we + // fall back to the manual frame loop below. + if sdk_version >= 29 { + if let Ok(lib) = ModuleLoader::load("libandroid.so") { + // Prefer the newer vsync callback (API 33+); fall back to the + // API 29 frame callback when it isn't available. + let vsync: Option = if sdk_version >= 33 { + lib.get_symbol("AChoreographer_postVsyncCallback").ok() + } else { + None + }; + let frame_callback_64: Option = + lib.get_symbol("AChoreographer_postFrameCallback64").ok(); + CHOREOGRAPHER_POST_CALLBACK_FN = vsync.or(frame_callback_64); + } + } + match CHOREOGRAPHER_POST_CALLBACK_FN { + Some(_) => post_vsync_callback(), + None => init_simple_render_loop(device_refresh_rate), + } } } +#[cfg(not(no_android_choreographer))] +unsafe extern "C" fn vsync_callback( + _data: *mut ndk_sys::AChoreographerFrameCallbackData, + _user_data: *mut std::ffi::c_void, +) { + send_from_java_message(FromJavaMessage::RenderLoop); + post_vsync_callback(); +} + +#[cfg(not(no_android_choreographer))] +pub unsafe fn post_vsync_callback() { + if let Some(post_callback) = CHOREOGRAPHER_POST_CALLBACK_FN { + if !CHOREOGRAPHER.is_null() && from_java_messages_already_set() { + post_callback(CHOREOGRAPHER, Some(vsync_callback), std::ptr::null_mut()); + return; + } + } + // The chain ends here: the next `initChoreographer` starts a new one. + RENDER_LOOP_ACTIVE.store(false, std::sync::atomic::Ordering::SeqCst); +} + +/// Older cargo-makepad Java sources lack this bridge. Clear the lookup's +/// NoSuchMethodError and retain native scheduling rather than freezing their UI. +#[cfg(not(no_android_choreographer))] +unsafe fn start_java_choreographer( + env: *mut jni_sys::JNIEnv, + class: jni_sys::jclass, +) -> bool { + let method = ((**env).GetStaticMethodID.unwrap())( + env, + class, + c"startRenderLoop".as_ptr(), + c"()V".as_ptr(), + ); + if method.is_null() { + ((**env).ExceptionClear.unwrap())(env); + return false; + } + ((**env).CallStaticVoidMethod.unwrap())(env, class, method); + if ((**env).ExceptionCheck.unwrap())(env) != 0 { + ((**env).ExceptionClear.unwrap())(env); + return false; + } + true +} + /// Fallback render loop used when the Android Choreographer isn't available -/// (`no_android_choreographer` builds such as OHOS). A dedicated +/// (API < 29, and `no_android_choreographer` builds such as OHOS). A dedicated /// thread paces frames manually, since there is no system vsync callback. -#[cfg(no_android_choreographer)] fn init_simple_render_loop(device_refresh_rate: f32) { std::thread::spawn(move || { let mut last_frame_time = std::time::Instant::now(); diff --git a/platform/src/os/linux/android/ndk_sys.rs b/platform/src/os/linux/android/ndk_sys.rs index bf627b1ba2..6597ce3a68 100644 --- a/platform/src/os/linux/android/ndk_sys.rs +++ b/platform/src/os/linux/android/ndk_sys.rs @@ -2,6 +2,7 @@ #![allow(non_snake_case)] use makepad_jni_sys as jni_sys; +use std::ffi::c_void; #[repr(C)] #[derive(Debug, Copy, Clone)] @@ -57,7 +58,8 @@ extern "C" { // It is API 30+; a plain `extern "C"` strong reference to it would break // `dlopen` of libmakepad.so on API 26-29 devices. If surface frame-rate // control is wanted, resolve it via `dlsym(libandroid.so, ...)` gated on - // `sdk_version >= 30`, rather than linking newer symbols at the minSdk floor. + // `sdk_version >= 30`, the same way the Choreographer post-callbacks are + // handled below. // AHardwareBuffer_acquire / _release are API 26 — safe at the minSdk floor. pub fn AHardwareBuffer_acquire(buffer: *mut AHardwareBuffer); @@ -68,6 +70,38 @@ pub const AHARDWAREBUFFER_USAGE_CPU_READ_RARELY: u64 = 2; pub const AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN: u64 = 3; pub const AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE: u64 = 1 << 8; +pub type AChoreographer = c_void; +pub type AChoreographerFrameCallbackData = c_void; + +pub type AChoreographer_vsyncCallback = + unsafe extern "C" fn(callbackData: *mut AChoreographerFrameCallbackData, data: *mut c_void); + +/// The function type for posting callbacks to the AChoreographer +pub type AChoreographerPostCallbackFn = unsafe extern "C" fn( + *mut AChoreographer, + Option, + *mut std::ffi::c_void, +) -> i32; + +#[cfg(not(no_android_choreographer))] +extern "C" { + // AChoreographer_getInstance was introduced in API 24, so it's safe to + // link directly at our minSdk floor (26). + pub fn AChoreographer_getInstance() -> *mut AChoreographer; +} + +// AChoreographer_postVsyncCallback (API 33) and AChoreographer_postFrameCallback64 +// (API 29) are deliberately NOT declared in an `extern "C"` block. +// +// A strong undefined reference to a symbol that doesn't exist on API 26-28 +// devices makes the entire `libmakepad.so` fail to `dlopen` at process start +// (`UnsatisfiedLinkError: cannot locate symbol ...`), even if the call site is +// runtime-gated behind an `sdk_version >= 29` check — the relocation is still +// emitted. Rust's `extern "C"` can't express weak linkage on stable, so these +// are instead resolved at runtime with `dlsym(libandroid.so, ...)` in +// android_jni.rs, typed via `AChoreographerPostCallbackFn`, only on devices +// where they actually exist. + #[repr(C)] pub struct ANativeActivity { pub callbacks: *mut ANativeActivityCallbacks, diff --git a/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadActivity.java b/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadActivity.java index ce2d4d217a..c00c756a6e 100644 --- a/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadActivity.java +++ b/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadActivity.java @@ -49,7 +49,6 @@ import android.os.SystemClock; import android.util.Log; import android.view.ActionMode; -import android.view.Choreographer; import android.view.Display; import android.view.InputDevice; import android.view.Menu; @@ -1127,19 +1126,6 @@ public class MakepadActivity // would background the app and its onDestroy would shut it down), and // it finishes. private static MakepadActivity sNativeActivity; - // Use the same Choreographer as ViewRootImpl: its batched touch input runs - // before our animation-phase callback sends the native render beat. A - // separate NDK Choreographer can beat Java input and miss a scrolling frame. - // Static ownership avoids retaining an Activity or starting a second chain - // when a replacement Activity takes over the same native app. - private static final Choreographer.FrameCallback sRenderLoop = new Choreographer.FrameCallback() { - @Override - public void doFrame(long frameTimeNanos) { - if (MakepadNative.onRenderLoop()) { - Choreographer.getInstance().postFrameCallback(this); - } - } - }; private boolean mSuperseded; private final Handler mHandler = new Handler(Looper.getMainLooper()); private InputManager mInputManager; @@ -1621,9 +1607,7 @@ public void onCreate(Bundle savedInstanceState) { setVolumeControlStream(AudioManager.STREAM_MUSIC); float refreshRate = getDeviceRefreshRate(); - if (MakepadNative.initChoreographer(refreshRate, sdkVersion)) { - Choreographer.getInstance().postFrameCallback(sRenderLoop); - } + MakepadNative.initChoreographer(refreshRate, sdkVersion); //% MAIN_ACTIVITY_ON_CREATE } diff --git a/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java b/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java index 6e1b1f34b6..2f6dc2a8ab 100644 --- a/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java +++ b/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java @@ -1,6 +1,7 @@ package dev.makepad.android; import android.view.Surface; +import android.view.Choreographer; import android.view.MotionEvent; public class MakepadNative { @@ -15,7 +16,25 @@ public class MakepadNative { public static native void onAndroidParams(String cache_path, String data_path, float density, boolean isEmulator, String androidVersion, String buildNumber, String kernelVersion); - public native static boolean initChoreographer(float deviceRefreshRate, int sdkVersion); + public native static void initChoreographer(float deviceRefreshRate, int sdkVersion); + + // Called by the native bootstrap when this bridge is available. Share the + // View input scheduler so batched MotionEvents reach Rust before its beat. + // Static ownership avoids retaining an Activity or duplicating the callback + // chain when a replacement Activity takes over the same native app. + private static final Choreographer.FrameCallback renderLoop = new Choreographer.FrameCallback() { + @Override + public void doFrame(long frameTimeNanos) { + if (onRenderLoop()) { + Choreographer.getInstance().postFrameCallback(this); + } + } + }; + + private static void startRenderLoop() { + Choreographer.getInstance().postFrameCallback(renderLoop); + } + public native static boolean onRenderLoop(); public native static void onBackPressed(); From 2226f16c37a1025a2f5e7fa2140195bbe050f304 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:33:18 -0700 Subject: [PATCH 11/16] fix(widgets): advance the first fling frame --- widgets/src/scroll_motion.rs | 39 +++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/widgets/src/scroll_motion.rs b/widgets/src/scroll_motion.rs index 5002b45ea1..d41d42fad8 100644 --- a/widgets/src/scroll_motion.rs +++ b/widgets/src/scroll_motion.rs @@ -563,28 +563,39 @@ impl Fling { /// Advance the fling to wall-clock time `now` (the NextFrame event time). /// - /// Returns `None` on the first frame, which only establishes the time base. Afterwards - /// returns `Some(displacement)` in pixels, with `velocity` decayed for the next step. + /// Returns `Some(displacement)` in pixels, with `velocity` decayed for the next step. + /// Like the bounce clock, the first frame advances by a nominal frame interval. + /// Spending it only establishing a time base freezes the list for one refresh + /// at the drag-to-fling handoff. Subsequent frames use measured intervals. /// /// Frame delivery is not perfectly vsync-uniform (e.g. Windows `Present(1,0)` can return /// early or span more than one vblank), so the raw inter-frame dt jitters. We track an EMA /// of the interval and clamp the dt used to a tight band around it, so a single late or /// early frame can't produce a visible jump or stall. pub fn step(&mut self, now: f64) -> Option { - if self.last_time <= 0.0 { - self.last_time = now; - self.dt_ema = 0.0; - return None; - } - let raw_dt = (now - self.last_time).clamp(0.0, FLING_MAX_DT); + let first_step = self.last_time <= 0.0; + let raw_dt = if first_step { + // Input timestamps and NextFrame timestamps need not share an epoch + // (Android touch events use uptime). Seed from the animation clock, + // using the same first-step policy as a mid-motion bounce. + FRAME_CLOCK_FIRST_STEP + } else { + (now - self.last_time).clamp(0.0, FLING_MAX_DT) + }; self.last_time = now; - if self.dt_ema <= 0.0 { - self.dt_ema = raw_dt; + let dt = if first_step { + // Let the next measured interval seed the EMA, including on displays + // whose refresh rate differs from the nominal first step. + raw_dt } else { - self.dt_ema = - self.dt_ema * (1.0 - FLING_DT_EMA_ALPHA) + raw_dt * FLING_DT_EMA_ALPHA; - } - let dt = raw_dt.clamp(self.dt_ema * FLING_DT_BAND.0, self.dt_ema * FLING_DT_BAND.1); + if self.dt_ema <= 0.0 { + self.dt_ema = raw_dt; + } else { + self.dt_ema = + self.dt_ema * (1.0 - FLING_DT_EMA_ALPHA) + raw_dt * FLING_DT_EMA_ALPHA; + } + raw_dt.clamp(self.dt_ema * FLING_DT_BAND.0, self.dt_ema * FLING_DT_BAND.1) + }; if USE_ANDROID_FLING_SPLINE { self.age += dt; return Some(self.step_android_spline()); From c445425975f12d71fe2a2ed2f3a74fd00ea2adf6 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Thu, 8 Oct 2026 21:22:39 -0700 Subject: [PATCH 12/16] Add input-only PortalList capture routing setter --- widgets/src/portal_list.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/widgets/src/portal_list.rs b/widgets/src/portal_list.rs index d51d113c6d..075e92a70e 100644 --- a/widgets/src/portal_list.rs +++ b/widgets/src/portal_list.rs @@ -2011,6 +2011,13 @@ impl PortalList { self.tail_range = tail_range; } + /// Controls whether the list may take pointer capture from a child. + /// This only changes input routing: it does not redraw, reapply templates, + /// or reset the current scroll position and auto-tail state. + pub fn set_capture_overload(&mut self, capture_overload: bool) { + self.capture_overload = capture_overload; + } + /// Sets the flow direction, e.g. to switch a list between a vertical /// (`Flow::Down`) and horizontal (`Flow::right()`) layout at runtime. pub fn set_flow(&mut self, cx: &mut Cx, flow: Flow) { @@ -3712,6 +3719,13 @@ impl PortalListRef { } } + /// See [`PortalList::set_capture_overload`]. + pub fn set_capture_overload(&self, capture_overload: bool) { + if let Some(mut inner) = self.borrow_mut() { + inner.set_capture_overload(capture_overload); + } + } + /// See [`PortalList::set_flow`]. pub fn set_flow(&self, cx: &mut Cx, flow: Flow) { if let Some(mut inner) = self.borrow_mut() { From fca1478f09394d9391dbca7601190337be562eb8 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Fri, 9 Oct 2026 00:56:20 -0700 Subject: [PATCH 13/16] Backport upstream Android navigation and IME timing fixes Adapt makepad/makepad#1288 (711521dbf) to the widgets layout in this fork. Preserve bounded dynamic-view reuse and existing back handling; omit the newer upstream cancel-scope dependency. --- apps/notes/src/view.rs | 2 +- platform/src/cx_api.rs | 7 + platform/src/os/linux/android/android.rs | 28 +++- widgets/src/animator.rs | 55 +++++++- widgets/src/stack_navigation.rs | 158 ++++++++++------------- 5 files changed, 149 insertions(+), 101 deletions(-) diff --git a/apps/notes/src/view.rs b/apps/notes/src/view.rs index de466716d6..e824619a21 100644 --- a/apps/notes/src/view.rs +++ b/apps/notes/src/view.rs @@ -1850,7 +1850,7 @@ mod tests { let page_ref = nav.view_by_id(&mut cx, page); page_ref .as_stack_navigation_view() - .show_at_rest(&mut cx, size.x); + .show_at_rest(&mut cx); let actions = cx.capture_actions(|cx| { cx.widget_action( page_ref.widget_uid(), diff --git a/platform/src/cx_api.rs b/platform/src/cx_api.rs index 71c12d2a3b..363dc7eeb3 100644 --- a/platform/src/cx_api.rs +++ b/platform/src/cx_api.rs @@ -1530,6 +1530,13 @@ impl Cx { pub fn text_ime_was_dismissed(&mut self) { self.publish_hosted_ime(crate::ime::HostedImeState::default()); self.keyboard.set_text_ime_dismissed(); + // A focused TextInput re-pushes ShowTextIME on every draw, so a show op + // queued by a frame drawn just before this dismissal landed can still be + // in platform_ops. If it drains after the backend forgets its last-shown + // config (e.g. Android's ResizeTextIME-closed clearing last_ime_config), + // it re-opens the keyboard the user just closed. Drop the stale shows. + self.platform_ops + .retain(|op| !matches!(op, CxOsOp::ShowTextIME(..))); self.platform_ops.push_back(CxOsOp::HideTextIME); } diff --git a/platform/src/os/linux/android/android.rs b/platform/src/os/linux/android/android.rs index d222f31a60..35e9e10c1d 100644 --- a/platform/src/os/linux/android/android.rs +++ b/platform/src/os/linux/android/android.rs @@ -478,6 +478,15 @@ impl Cx { // but immediately torn down again (rotation race). self.os.needs_first_draw = true; } + // A hide still pending after this draw really hides the keyboard, + // unless the draw queued a `ShowTextIME` for an input that took focus. + if self.os.pending_ime_hide + && !self.platform_ops.iter().any(|op| matches!(op, CxOsOp::ShowTextIME(..))) + { + self.os.pending_ime_hide = false; + unsafe { android_jni::to_java_show_keyboard(false); } + self.os.last_ime_config = None; + } } Ok(message) => { self.handle_message(message); @@ -2771,6 +2780,9 @@ impl Cx { self.os.timers.timers.remove(&timer_id); } CxOsOp::ShowTextIME(_area, _pos, config) => unsafe { + // An input asking for the keyboard right after a hide (e.g., it just took + // focus from another input) keeps the keyboard up. + self.os.pending_ime_hide = false; // A focused `TextInput` re-issues `ShowTextIME` on every // draw. Calling into Java each time thrashes the IME: // `configure_keyboard` can restart the input connection, @@ -2780,19 +2792,19 @@ impl Cx { // — a loop that flickers the soft keyboard open then shut. // Only touch Java when the requested config actually // changes; `last_ime_config` is cleared whenever the - // keyboard goes down (here or via `ResizeTextIME`). + // keyboard goes down (once a pending hide goes through, + // or via `ResizeTextIME`). if self.os.last_ime_config != Some(config) { android_jni::to_java_configure_keyboard(&config); android_jni::to_java_show_keyboard(true); self.os.last_ime_config = Some(config); } }, - CxOsOp::HideTextIME => unsafe { + CxOsOp::HideTextIME => { // Unconditional on purpose: unlike `ShowTextIME` this is not // issued per-frame, so there is no thrash to dedup — and a // skipped hide would leave the soft keyboard stuck open. - android_jni::to_java_show_keyboard(false); - self.os.last_ime_config = None; + self.os.pending_ime_hide = true; }, CxOsOp::SyncImeState { text, @@ -3746,6 +3758,7 @@ impl Default for CxOs { last_ime_height: 0.0, last_ime_visible: false, last_ime_config: None, + pending_ime_hide: false, media: CxAndroidMedia::default(), display: None, surface_alive: false, @@ -3888,9 +3901,12 @@ pub struct CxOs { /// or `None` while the keyboard is requested-hidden. A focused `TextInput` /// re-issues `ShowTextIME` every draw; this dedups those so the JNI IME /// calls (and the inset-driven redraw loop they trigger) fire only on a - /// real change. Reset to `None` on `HideTextIME` and when Java reports the - /// keyboard closed. + /// real change. Reset to `None` once a `HideTextIME` goes through and when + /// Java reports the keyboard closed. pub last_ime_config: Option, + /// Set by `HideTextIME`, which waits until after the next draw, so that moving focus + /// from one text input to another keeps the keyboard up instead of hiding it. + pub pending_ime_hide: bool, pub frame_time: i64, pub quit: bool, pub fullscreen: bool, diff --git a/widgets/src/animator.rs b/widgets/src/animator.rs index 5c190169d9..f1e07ef8cd 100644 --- a/widgets/src/animator.rs +++ b/widgets/src/animator.rs @@ -1517,6 +1517,10 @@ pub enum Ease { cp2: f64, cp3: f64, }, + /// Material's emphasized easing, i.e., Android's `fast_out_extra_slow_in`, + /// which Android uses for its activity transitions. + #[live] + Emphasized, } impl Ease { @@ -1793,7 +1797,8 @@ impl Ease { return t; } - let epsilon = 1.0 / 200.0 * t; + // Tight enough that steep curves stay smooth instead of jittering back and forth. + const EPSILON: f64 = 1e-7; let cx = 3.0 * cp0; let bx = 3.0 * (cp2 - cp0) - cx; let ax = 1.0 - cx - bx; @@ -1802,9 +1807,9 @@ impl Ease { let ay = 1.0 - cy - by; let mut u = t; - for _i in 0..6 { + for _i in 0..8 { let x = ((ax * u + bx) * u + cx) * u - t; - if x.abs() < epsilon { + if x.abs() < EPSILON { return ((ay * u + by) * u + cy) * u; } let d = (3.0 * ax * u + 2.0 * bx) * u + cx; @@ -1824,9 +1829,9 @@ impl Ease { let mut w = 0.0; let mut v = 1.0; u = t; - for _i in 0..8 { + for _i in 0..30 { let x = ((ax * u + bx) * u + cx) * u; - if (x - t).abs() < epsilon { + if (x - t).abs() < EPSILON { return ((ay * u + by) * u + cy) * u; } @@ -1840,6 +1845,46 @@ impl Ease { return ((ay * u + by) * u + cy) * u; } + Self::Emphasized => { + // Android's path is the emphasized-accelerate curve up to 40% at a sixth of + // the time, then the emphasized-decelerate curve for the rest. + const SPLIT_TIME: f64 = 1.0 / 6.0; + const SPLIT_PROGRESS: f64 = 0.4; + if t <= 0. { + return 0.; + } + if t >= 1. { + return 1.; + } + if t < SPLIT_TIME { + let accelerate = Self::Bezier { cp0: 0.3, cp1: 0.0, cp2: 0.8, cp3: 0.15 }; + return SPLIT_PROGRESS * accelerate.map(t / SPLIT_TIME); + } + let decelerate = Self::Bezier { cp0: 0.05, cp1: 0.7, cp2: 0.1, cp3: 1.0 }; + let t = (t - SPLIT_TIME) / (1. - SPLIT_TIME); + return SPLIT_PROGRESS + (1. - SPLIT_PROGRESS) * decelerate.map(t); + } + } + } +} + +#[cfg(test)] +mod ease_tests { + use super::*; + + #[test] + fn emphasized_matches_androids_fast_out_extra_slow_in() { + // Sampled from the path data of Android's `fast_out_extra_slow_in` interpolator. + let samples = [(0.1, 0.0935), (1.0 / 6.0, 0.4), (0.25, 0.7728), (0.5, 0.9506), (0.75, 0.9914)]; + for (t, expected) in samples { + assert!((Ease::Emphasized.map(t) - expected).abs() < 1e-4, "t = {t}"); + } + let mut prev = 0.0; + for i in 0..=1000 { + let t = i as f64 / 1000.0; + let mix = Ease::Emphasized.map(t); + assert!(mix >= prev, "steps backwards at t = {t}"); + prev = mix; } } } diff --git a/widgets/src/stack_navigation.rs b/widgets/src/stack_navigation.rs index 5b15c26ef6..3f168d8d2f 100644 --- a/widgets/src/stack_navigation.rs +++ b/widgets/src/stack_navigation.rs @@ -80,23 +80,23 @@ script_mod! { header := mod.widgets.StackViewHeader{} - offset: 4000.0 + slide: 1.0 animator: Animator{ slide: { default: @hide hide: AnimatorState{ redraw: true - ease: Ease.ExpDecay{d1: 0.80 d2: 0.97} - from: {all: Play.Forward{duration: 5.0}} - apply: {offset: 4000.0} + ease: Ease.Emphasized + from: {all: Play.Forward{duration: 0.45}} + apply: {slide: 1.0} } show: AnimatorState{ redraw: true - ease: Ease.ExpDecay{d1: 0.82 d2: 0.95} - from: {all: Play.Forward{duration: 0.5}} - apply: {offset: 0.0} + ease: Ease.Emphasized + from: {all: Play.Forward{duration: 0.45}} + apply: {slide: 0.0} } } } @@ -150,17 +150,22 @@ pub struct StackNavigationView { #[deref] view: View, - /// The offset of the stack view from the left edge of the parent view. + /// How far this view has slid out to the right, as a fraction of its width: + /// 0 when it's fully shown and 1 when it's fully hidden. #[live] - offset: f64, - - /// The offset of the stack view from the left edge of the parent view when it is fully hidden. - #[rust(10000.0)] - offset_to_hide: f64, + slide: f64, #[apply_default] animator: Animator, + /// The slide to play on the next frame, so that a slow first draw of the view + /// it reveals happens before the slide's clock starts, rather than making it jump. + #[rust] + pending_slide: Option<[LiveId; 2]>, + + #[rust] + next_frame: NextFrame, + /// The state of the stack view. #[rust] state: StackNavigationViewState, @@ -253,6 +258,12 @@ impl StackNavigationView { impl Widget for StackNavigationView { fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) { + if let Some(slide_state) = self.pending_slide { + if self.next_frame.is_event(event).is_some() { + self.pending_slide = None; + self.animator_play(cx, &slide_state); + } + } if self.animator_handle_event(cx, event).must_redraw() { self.view.redraw(cx); } @@ -266,7 +277,7 @@ impl Widget for StackNavigationView { fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep { let parent_rect = cx.peek_walk_turtle(walk); let abs_pos = Vec2d { - x: parent_rect.pos.x + self.offset, + x: parent_rect.pos.x + self.slide * parent_rect.size.x, y: parent_rect.pos.y, }; @@ -276,7 +287,7 @@ impl Widget for StackNavigationView { impl StackNavigationView { fn hide_stack_view(&mut self, cx: &mut Cx) { - self.animator_play(cx, ids!(slide.hide)); + self.schedule_slide(cx, ids!(slide.hide)); cx.widget_action( self.widget_uid(), @@ -307,61 +318,57 @@ impl StackNavigationView { fn finish_closure_animation_if_done(&mut self, cx: &mut Cx) { if self.state == StackNavigationViewState::Active && self.animator.in_state(cx, ids!(slide.hide)) + && !self.is_animating() { - if self.offset > self.offset_to_hide { - self.view.visible = false; - self.redraw(cx); + self.view.visible = false; + self.redraw(cx); + self.state = StackNavigationViewState::Inactive; - // Dispatch HideEnd with the parent navigation's UID - let hide_end_action = if let Some(parent_uid) = self.parent_navigation_uid { - StackNavigationTransitionAction::HideEnd(parent_uid) - } else { - error!( - "No parent navigation UID found for stack view {:?}", - self.widget_uid() - ); - return; - }; - - cx.widget_action(self.widget_uid(), hide_end_action); + // Dispatch HideEnd with the parent navigation's UID + let hide_end_action = if let Some(parent_uid) = self.parent_navigation_uid { + StackNavigationTransitionAction::HideEnd(parent_uid) + } else { + error!( + "No parent navigation UID found for stack view {:?}", + self.widget_uid() + ); + return; + }; - self.animator_cut(cx, ids!(slide.hide)); - self.state = StackNavigationViewState::Inactive; - } + cx.widget_action(self.widget_uid(), hide_end_action); } } fn trigger_action_post_opening_if_done(&mut self, cx: &mut Cx) { if self.state == StackNavigationViewState::Inactive && self.animator.in_state(cx, ids!(slide.show)) + && !self.is_animating() { - const OPENING_OFFSET_THRESHOLD: f64 = 0.5; - if self.offset < OPENING_OFFSET_THRESHOLD { - cx.widget_action(self.widget_uid(), StackNavigationTransitionAction::ShowDone); - self.state = StackNavigationViewState::Active; - } + cx.widget_action(self.widget_uid(), StackNavigationTransitionAction::ShowDone); + self.state = StackNavigationViewState::Active; } } + fn schedule_slide(&mut self, cx: &mut Cx, slide_state: &[LiveId; 2]) { + self.pending_slide = Some(*slide_state); + self.next_frame = cx.new_next_frame(); + } + fn is_animating(&self) -> bool { - self.animator.is_track_animating(live_id!(slide)) + self.pending_slide.is_some() || self.animator.is_track_animating(live_id!(slide)) } } impl StackNavigationViewRef { - pub fn show(&self, cx: &mut Cx, view_width: f64) { + pub fn show(&self, cx: &mut Cx) { if let Some(mut inner) = self.borrow_mut() { inner.view.visible = true; - inner.offset_to_hide = view_width; + inner.slide = 1.0; inner.state = StackNavigationViewState::Inactive; - // Force-reset the animator by cutting to show (offset=0) first, - // then cutting to hide, then playing show. This ensures the animator - // always sees a state change regardless of its current state. - inner.animator_cut(cx, ids!(slide.show)); // force to show state - inner.animator_cut(cx, ids!(slide.hide)); // then to hide state - inner.offset = view_width; // set actual start offset - inner.animator_play(cx, ids!(slide.show)); // now animate hide -> show + // Always slide in from fully hidden, wherever an earlier slide left off. + inner.animator_cut(cx, ids!(slide.hide)); + inner.schedule_slide(cx, ids!(slide.show)); inner.redraw(cx); } } @@ -382,33 +389,27 @@ impl StackNavigationViewRef { } } - pub fn set_offset_to_hide(&self, offset_to_hide: f64) { - if let Some(mut inner) = self.borrow_mut() { - inner.offset_to_hide = offset_to_hide; - } - } - pub fn hide(&self, cx: &mut Cx) { if let Some(mut inner) = self.borrow_mut() { inner.hide_stack_view(cx); } } - pub fn show_at_rest(&self, cx: &mut Cx, view_width: f64) { + pub fn show_at_rest(&self, cx: &mut Cx) { if let Some(mut inner) = self.borrow_mut() { inner.view.visible = true; - inner.offset_to_hide = view_width; - inner.offset = 0.0; + inner.slide = 0.0; + inner.pending_slide = None; inner.state = StackNavigationViewState::Active; inner.animator_cut(cx, ids!(slide.show)); inner.redraw(cx); } } - pub fn hide_immediately(&self, cx: &mut Cx, view_width: f64) { + pub fn hide_immediately(&self, cx: &mut Cx) { if let Some(mut inner) = self.borrow_mut() { - inner.offset_to_hide = view_width; - inner.offset = view_width; + inner.slide = 1.0; + inner.pending_slide = None; inner.view.visible = false; inner.state = StackNavigationViewState::Inactive; inner.animator_cut(cx, ids!(slide.hide)); @@ -447,9 +448,6 @@ pub struct StackNavigation { #[live] stack_templates: ScriptObjectRef, - #[rust] - view_width: f64, - #[rust] current_view: Option, @@ -540,9 +538,6 @@ impl Widget for StackNavigation { } fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep { - let parent_rect = cx.peek_walk_turtle(walk); - self.update_view_width(cx.cx, parent_rect.size.x); - for (_id, widget_ref) in self.get_visible_views(cx.cx).iter() { widget_ref.draw_walk(cx, scope, walk)?; } @@ -611,18 +606,6 @@ impl WidgetMatchEvent for StackNavigation { } impl StackNavigation { - fn update_view_width(&mut self, cx: &mut Cx, view_width: f64) { - if view_width <= 0.0 || (self.view_width - view_width).abs() <= f64::EPSILON { - return; - } - - self.view_width = view_width; - for view_id in self.visible_view_ids() { - let stack_view_ref = self.stack_view_ref(cx, view_id); - stack_view_ref.set_offset_to_hide(view_width); - } - } - fn collect_stack_templates(&mut self, vm: &mut ScriptVm) { if self.stack_templates.is_zero() { return; @@ -783,7 +766,7 @@ impl StackNavigation { fn release_view(&mut self, cx: &mut Cx, view_id: LiveId) { let stack_view_ref = self.stack_view_ref(cx, view_id); - stack_view_ref.hide_immediately(cx, self.view_width); + stack_view_ref.hide_immediately(cx); cx.widget_action( self.widget_uid(), StackNavigationTransitionAction::ViewReleased(view_id), @@ -811,15 +794,14 @@ impl StackNavigation { for view_id in dynamic_view_ids { let stack_view_ref = self.stack_view_ref(cx, view_id); if Some(view_id) == active { - stack_view_ref.show_at_rest(cx, self.view_width); + stack_view_ref.show_at_rest(cx); } else { - stack_view_ref.hide_immediately(cx, self.view_width); + stack_view_ref.hide_immediately(cx); } } if let Some(view_id) = active { if !self.dynamic_views.contains_key(&view_id) { - self.stack_view_ref(cx, view_id) - .show_at_rest(cx, self.view_width); + self.stack_view_ref(cx, view_id).show_at_rest(cx); } } } @@ -837,7 +819,7 @@ impl StackNavigation { let stack_view_ref = self.stack_view_ref(cx, view_id); stack_view_ref.set_parent_navigation_uid(self.widget_uid()); - stack_view_ref.show(cx, self.view_width); + stack_view_ref.show(cx); cx.widget_action( stack_view_ref.widget_uid(), @@ -861,7 +843,7 @@ impl StackNavigation { if let Some(incoming) = view_id { let incoming_view_ref = self.stack_view_ref(cx, incoming); incoming_view_ref.set_parent_navigation_uid(self.widget_uid()); - incoming_view_ref.show_at_rest(cx, self.view_width); + incoming_view_ref.show_at_rest(cx); } self.transition = Some(StackNavigationTransition::Pop { @@ -890,8 +872,7 @@ impl StackNavigation { self.transition = None; self.current_view = Some(incoming); - self.stack_view_ref(cx, incoming) - .show_at_rest(cx, self.view_width); + self.stack_view_ref(cx, incoming).show_at_rest(cx); if let Some(outgoing) = outgoing { self.release_view(cx, outgoing); } @@ -917,8 +898,7 @@ impl StackNavigation { self.current_view = incoming; self.release_view(cx, outgoing); if let Some(incoming) = incoming { - self.stack_view_ref(cx, incoming) - .show_at_rest(cx, self.view_width); + self.stack_view_ref(cx, incoming).show_at_rest(cx); } self.redraw(cx); } From 7d8470ac359456c17b3a99f106dc96818ab6090c Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Fri, 9 Oct 2026 00:56:20 -0700 Subject: [PATCH 14/16] Backport upstream image animation clocks and sized decoding Adapt makepad/makepad#1296 (95e0ad5a4) to this fork. Preserve async eviction recovery and error reporting; omit slice-image options absent in this widget API. --- draw/src/image_cache.rs | 820 ++++++++++++++++++++++++++++++------- draw/src/lib.rs | 4 +- platform/src/texture.rs | 12 + widgets/src/image.rs | 398 ++++++++++++++++-- widgets/src/image_cache.rs | 5 +- 5 files changed, 1056 insertions(+), 183 deletions(-) diff --git a/draw/src/image_cache.rs b/draw/src/image_cache.rs index 1243a14a65..e0d9932d95 100644 --- a/draw/src/image_cache.rs +++ b/draw/src/image_cache.rs @@ -5,10 +5,10 @@ use makepad_zune_bmp::BmpDecoder; use makepad_zune_jpeg::JpegDecoder; use makepad_zune_png::makepad_zune_core::bytestream::ZCursor; use makepad_zune_png::makepad_zune_core::options::DecoderOptions; -use makepad_zune_png::{post_process_image, PngDecoder}; +use makepad_zune_png::{BlendOp, DisposeOp, PngDecoder}; use makepad_zune_qoi::QoiDecoder; use std::cell::RefCell; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::error::Error; use std::fmt; use std::io::Read; @@ -33,6 +33,8 @@ pub struct ImageBuffer { /// highest level `n`) instead of level 0 only. Set by /// [`ImageBuffer::build_cpu_mip_chain_if_uploaded`]. max_level: Option, + /// The size of the image (or of each animation frame) before it was shrunk, if it was. + natural_size: Option<(usize, usize)>, } /// Alpha-weighted average of up to four `0xAARRGGBB` texels (premultiplying RGB by alpha so @@ -106,6 +108,90 @@ fn backend_uploads_cpu_mip_chain() -> bool { cfg!(any(target_os = "macos", target_os = "ios", target_os = "tvos")) } +/// Halves an image's size in both directions, rounding up. +fn halve_size((width, height): (usize, usize)) -> (usize, usize) { + (width.div_ceil(2), height.div_ceil(2)) +} + +/// Returns how many times an image that's `natural_size` pixels big can be halved +/// and still have enough pixels to be drawn `drawn_size` pixels big. +fn count_halvings(natural_size: (usize, usize), drawn_size: (usize, usize)) -> usize { + let mut size = natural_size; + let mut halvings = 0; + while size != (1, 1) && halve_size(size).0 >= drawn_size.0 && halve_size(size).1 >= drawn_size.1 { + size = halve_size(size); + halvings += 1; + } + halvings +} + +/// Returns how many pixels an image that's `natural_size` pixels big gets decoded into +/// to be drawn `drawn_size` pixels big: halved as often as it can be, like a mip level. +fn get_decoded_size(natural_size: (usize, usize), drawn_size: (usize, usize)) -> (usize, usize) { + (0..count_halvings(natural_size, drawn_size)).fold(natural_size, |size, _| halve_size(size)) +} + +/// Halves the `width` x `height` texels at the start of `src` (whose rows are `stride` texels +/// apart) into a new buffer, averaging each 2x2 block the same way the mip chain does. +fn halve_texels(src: &[u32], stride: usize, (width, height): (usize, usize)) -> Vec { + let (half_width, half_height) = halve_size((width, height)); + let mut half = Vec::with_capacity(half_width * half_height); + for y in 0..half_height { + let row0 = y * 2 * stride; + let row1 = (y * 2 + 1).min(height - 1) * stride; + for x in 0..half_width { + let (x0, x1) = (x * 2, (x * 2 + 1).min(width - 1)); + half.push(box_average_bgra(&[ + src[row0 + x0], + src[row0 + x1], + src[row1 + x0], + src[row1 + x1], + ])); + } + } + half +} + +/// Returns whether `texture` has enough pixels to be drawn `drawn_size` pixels big, +/// or to be drawn with all of the image's pixels if that's `None`. +pub fn has_enough_pixels(cx: &mut Cx, texture: &Texture, drawn_size: Option<(usize, usize)>) -> bool { + let Some(natural_size) = texture.natural_size(cx) else { + return true; + }; + let Some(drawn_size) = drawn_size else { + return false; + }; + let decoded_size = match texture.animation(cx) { + Some(animation) => (animation.width, animation.height), + None => texture.get_format(cx).vec_width_height().unwrap_or_default(), + }; + let needed_size = get_decoded_size(natural_size, drawn_size); + decoded_size.0 >= needed_size.0 && decoded_size.1 >= needed_size.1 +} + +/// A decode that's on its way, with enough pixels for an image that's `natural_size` pixels big +/// to be drawn at `drawn_size`, or with all of its pixels if that's `None`. +#[derive(Clone, Copy)] +struct PendingDecode { + natural_size: (usize, usize), + drawn_size: Option<(usize, usize)>, +} + +impl PendingDecode { + /// Returns whether this decode will have enough pixels for the image to be drawn at `drawn_size` too. + fn is_big_enough_for(&self, drawn_size: Option<(usize, usize)>) -> bool { + match (self.drawn_size, drawn_size) { + (None, _) => true, + (Some(_), None) => false, + (Some(own_drawn_size), Some(drawn_size)) => { + let own_decoded_size = get_decoded_size(self.natural_size, own_drawn_size); + let decoded_size = get_decoded_size(self.natural_size, drawn_size); + own_decoded_size.0 >= decoded_size.0 && own_decoded_size.1 >= decoded_size.1 + } + } + } +} + /// Hard upper bound on a decoded image's width or height (per side). Matches /// zune's default cap; lets us reject decompression-bomb headers before /// allocating anything. @@ -188,7 +274,10 @@ fn animation_atlas_layout_with_limit( if frame_count == 0 || frame_count > MAX_IMAGE_FRAMES { return Err(ImageError::DimensionsTooLarge { width, height }); } - let fits_horizontal = (Cx::max_texture_width() / width).max(1); + // Use as few rows as the frames fit in, then as few columns as fill those rows, + // so the atlas (which gets uploaded all at once) is barely bigger than its frames. + let max_columns = (Cx::max_texture_width() / width).max(1); + let fits_horizontal = frame_count.div_ceil(frame_count.div_ceil(max_columns)); let total_width = fits_horizontal * width; let rows = frame_count .checked_add(fits_horizontal - 1) @@ -265,6 +354,7 @@ impl ImageBuffer { data: out, animation: None, max_level: None, + natural_size: None, }) } @@ -311,6 +401,7 @@ impl ImageBuffer { }; let texture = Texture::new_with_format(cx, format); texture.set_animation(cx, self.animation); + texture.set_natural_size(cx, self.natural_size); texture } @@ -382,6 +473,8 @@ impl ImageBuffer { Ok(orient(buffer, decoder.info().and_then(|i| i.exif.as_deref()))) } + /// Decodes an animated PNG by drawing each frame onto a canvas the way browsers do + /// (blending it, then disposing of it), and packs those into an atlas like GIF/WebP. fn decode_animated_png( decoder: &mut PngDecoder, ) -> Result { @@ -397,100 +490,82 @@ impl ImageBuffer { .ok_or(ImageError::PngDecode(PngDecodeErrors::GenericStatic( "Failed to get animated PNG image dimensions", )))?; - let actl_info = - decoder - .actl_info() - .ok_or(ImageError::PngDecode(PngDecodeErrors::GenericStatic( - "Failed to get animated PNG actl info", - )))?; - let num_components = colorspace.num_components(); let frame_pixels = checked_pixel_count(width, height)?; - let output_len = frame_pixels - .checked_mul(num_components) - .filter(|&len| len <= MAX_IMAGE_DECODED_BYTES) - .ok_or(ImageError::DimensionsTooLarge { width, height })?; - let mut output = vec![0; output_len]; - // The atlas is sized from the declared `num_frames`; the decode loop below - // is driven by the actual fcTL chunks in the stream, which a malicious PNG - // can make exceed the declaration. Cap the loop to the allocated count so a - // frame can never be written past the atlas bounds. - let num_frames = actl_info.num_frames as usize; - let (total_width, total_height) = animation_atlas_layout(num_frames, width, height)?; - let mut final_buffer = ImageBuffer::default(); - final_buffer.data.resize(total_width * total_height, 0); - final_buffer.width = total_width; - final_buffer.height = total_height; - let mut cx = 0; - let mut cy = 0; - final_buffer.animation = Some(TextureAnimation { - width, - height, - num_frames, - frame_delays: Vec::new(), - }); - let mut previous_frame = None; - let mut frame_index = 0; + let max_frames = (MAX_IMAGE_PIXELS / frame_pixels).max(1).min(MAX_IMAGE_FRAMES); + let mut canvas = vec![0u8; frame_pixels * IMAGE_RGBA_BYTES_PER_PIXEL]; + let mut frames = Vec::new(); + let mut frame_delays = Vec::new(); + while decoder.more_frames() { - if frame_index >= num_frames { - break; - } - frame_index += 1; decoder.decode_headers()?; - let frame = decoder.frame_info().expect("to have already been decoded"); - let pix = decoder.decode_raw()?; - let info = - decoder - .info() - .ok_or(ImageError::PngDecode(PngDecodeErrors::GenericStatic( - "Failed to get animated PNG image info", - )))?; - post_process_image( - info, - colorspace, - &frame, - &pix, - previous_frame.as_deref(), - &mut output, - None, - )?; - previous_frame = Some(pix); - match num_components { - 4 => { - for y in 0..height { - for x in 0..width { - let r = output[y * width * 4 + x * 4]; - let g = output[y * width * 4 + x * 4 + 1]; - let b = output[y * width * 4 + x * 4 + 2]; - let a = output[y * width * 4 + x * 4 + 3]; - final_buffer.data[(y + cy) * total_width + (x + cx)] = ((a as u32) - << 24) - | ((r as u32) << 16) - | ((g as u32) << 8) - | (b as u32); + let frame = decoder.frame_info().ok_or(ImageError::PngDecode( + PngDecodeErrors::GenericStatic("Failed to get animated PNG frame info"), + ))?; + let pixels = decoder.decode_raw()?; + // A default image that isn't part of the animation is only there for viewers without APNG support. + if !frame.is_part_of_seq { + continue; + } + if frames.len() >= max_frames { + return Err(ImageError::DimensionsTooLarge { width, height }); + } + if frame.width == 0 + || frame.height == 0 + || frame.x_offset + frame.width > width + || frame.y_offset + frame.height > height + || pixels.len() < frame.width * frame.height * num_components + { + return Err(ImageError::PngDecode(PngDecodeErrors::GenericStatic( + "Animated PNG frame doesn't fit within the image", + ))); + } + let frame_rgba = match num_components { + 4 => pixels, + 3 => rgb_to_rgba(&pixels), + 2 => pixels.chunks_exact(2).flat_map(|la| [la[0], la[0], la[0], la[1]]).collect(), + 1 => pixels.iter().flat_map(|&l| [l, l, l, 0xFF]).collect(), + unsupported => return Err(ImageError::InvalidPixelAlignment(unsupported)), + }; + // A first frame that asks to be disposed to the "previous" frame is disposed to the background instead. + let previous_canvas = (frame.dispose_op == DisposeOp::Previous && !frames.is_empty()) + .then(|| canvas.clone()); + for (y, src_row) in frame_rgba.chunks_exact(frame.width * 4).take(frame.height).enumerate() { + let dst_start = ((frame.y_offset + y) * width + frame.x_offset) * 4; + let dst_row = &mut canvas[dst_start..dst_start + frame.width * 4]; + match frame.blend_op { + BlendOp::Source => dst_row.copy_from_slice(src_row), + BlendOp::Over => { + for (dst, src) in dst_row.chunks_exact_mut(4).zip(src_row.chunks_exact(4)) { + blend_rgba_over(dst, src); } } } - 3 => { - for y in 0..height { - for x in 0..width { - let r = output[y * width * 3 + x * 3]; - let g = output[y * width * 3 + x * 3 + 1]; - let b = output[y * width * 3 + x * 3 + 2]; - final_buffer.data[(y + cy) * total_width + (x + cx)] = - 0xff000000 | ((r as u32) << 16) | ((g as u32) << 8) | (b as u32); - } + } + frames.push(canvas.clone()); + // A delay denominator of 0 means hundredths of a second. + let delay_denom = if frame.delay_denom == 0 { 100 } else { frame.delay_denom }; + frame_delays.push(f64::from(frame.delay_num) / f64::from(delay_denom)); + + match (frame.dispose_op, previous_canvas) { + (DisposeOp::None, _) => {} + (DisposeOp::Previous, Some(previous_canvas)) => canvas = previous_canvas, + (DisposeOp::Background | DisposeOp::Previous, _) => { + for y in frame.y_offset..frame.y_offset + frame.height { + let row_start = (y * width + frame.x_offset) * 4; + canvas[row_start..row_start + frame.width * 4].fill(0); } } - _ => return Err(ImageError::InvalidPixelAlignment(num_components)), - } - cx += width; - if cx >= total_width { - cy += height; - cx = 0; } } - Ok(final_buffer) + + match frames.len() { + 0 => Err(ImageError::PngDecode(PngDecodeErrors::GenericStatic( + "Animated PNG had no frames", + ))), + 1 => Self::new(&frames[0], width, height), + _ => Self::pack_animation_atlas(frames, frame_delays, width, height), + } } pub fn from_webp(data: &[u8]) -> Result { @@ -523,6 +598,11 @@ impl ImageBuffer { if num_frames > max_frames { return Err(ImageError::DimensionsTooLarge { width, height }); } + // Browsers ignore the file's background color and clear disposed frames to transparent. + // Without a background color, the decoder never clears them at all. + decoder + .set_background_color([0, 0, 0, 0]) + .map_err(ImageError::WebpDecode)?; decoder.reset_animation(); let mut frames: Vec> = Vec::new(); let mut frame_delays: Vec = Vec::new(); @@ -677,6 +757,65 @@ impl ImageBuffer { Ok(final_buffer) } + /// Shrinks this image (or each frame of this animation) to as few pixels as it + /// needs to be drawn `drawn_size` pixels big, by halving it like a mip level. + fn shrink_to_drawn_size(self, drawn_size: (usize, usize)) -> Result { + let frame_size = self.animation.as_ref() + .map_or((self.width, self.height), |animation| (animation.width, animation.height)); + let halvings = count_halvings(frame_size, drawn_size); + if halvings == 0 { + return Ok(self); + } + let shrunk_size = get_decoded_size(frame_size, drawn_size); + let shrink_frame = |first_texel: usize| { + let mut texels = halve_texels(&self.data[first_texel..], self.width, frame_size); + let mut size = halve_size(frame_size); + for _ in 1..halvings { + texels = halve_texels(&texels, size.0, size); + size = halve_size(size); + } + texels + }; + let Some(animation) = &self.animation else { + return Ok(ImageBuffer { + width: shrunk_size.0, + height: shrunk_size.1, + data: shrink_frame(0), + animation: None, + max_level: None, + natural_size: Some(frame_size), + }); + }; + + // Pack the shrunk frames into a smaller atlas, in the same order. + let (atlas_width, atlas_height) = + animation_atlas_layout(animation.num_frames, shrunk_size.0, shrunk_size.1)?; + let columns = self.width / frame_size.0; + let shrunk_columns = atlas_width / shrunk_size.0; + let mut data = vec![0; atlas_width * atlas_height]; + for frame in 0..animation.num_frames { + let first_texel = (frame / columns) * frame_size.1 * self.width + (frame % columns) * frame_size.0; + let shrunk_first_texel = (frame / shrunk_columns) * shrunk_size.1 * atlas_width + + (frame % shrunk_columns) * shrunk_size.0; + for (y, row) in shrink_frame(first_texel).chunks_exact(shrunk_size.0).enumerate() { + let start = shrunk_first_texel + y * atlas_width; + data[start..start + shrunk_size.0].copy_from_slice(row); + } + } + Ok(ImageBuffer { + width: atlas_width, + height: atlas_height, + data, + animation: Some(TextureAnimation { + width: shrunk_size.0, + height: shrunk_size.1, + ..animation.clone() + }), + max_level: None, + natural_size: Some(frame_size), + }) + } + pub fn from_jpg(data: &[u8]) -> Result { let cursor = ZCursor::new(data); let mut decoder = JpegDecoder::new_with_options(cursor, decoder_options()); @@ -749,6 +888,12 @@ pub struct ImageCache { /// lane; a re-request of the same path replaces the staged decode. pub decode_queue: TaskQueue, pub pending_http_requests: HashMap, + /// The decode that's on its way for each image being decoded, + /// so a request it has enough pixels for just waits for it. + pending_decodes: HashMap, + /// Images evicted while a decode of them was on its way. That decode still gets cached when + /// it lands (for anything waiting on it), and then evicted when the next decode lands. + evicted_while_decoding: HashSet, } impl Default for ImageCache { @@ -767,6 +912,8 @@ impl ImageCache { map: HashMap::new(), decode_queue: TaskQueue::new(Lane::Light, MAX_POOL_WORKERS, 1024), pending_http_requests: HashMap::new(), + pending_decodes: HashMap::new(), + evicted_while_decoding: HashSet::new(), } } @@ -785,6 +932,9 @@ impl ImageCache { /// cap that may never be reached. Prefer [`evict_image_from_cache`], /// which also releases the pixels behind the entry. pub fn evict(&mut self, image_path: &Path) -> Option { + if self.pending_decodes.contains_key(image_path) { + self.evicted_while_decoding.insert(image_path.into()); + } self.map.remove(image_path) } @@ -1047,7 +1197,7 @@ fn detect_image_format_from_path_and_data(image_path: &Path, data: &[u8]) -> Opt /// Decodes an image of any format makepad supports into an [`ImageBuffer`], /// auto-detecting the format from the encoded `data`'s magic bytes. pub fn decode_image_from_data(data: &[u8]) -> Result { - decode_image_buffer(Path::new(""), data) + decode_image_buffer(Path::new(""), data, None) } /// Returns true if `data` looks like an SVG document (vs. a raster image). @@ -1077,7 +1227,91 @@ fn rgb_to_rgba(rgb: &[u8]) -> Vec { out } -fn decode_image_buffer(image_path: &Path, data: &[u8]) -> Result { +/// Draws the RGBA pixel `src` over the RGBA pixel `dst`, neither of them premultiplied. +fn blend_rgba_over(dst: &mut [u8], src: &[u8]) { + let src_alpha = u32::from(src[3]); + if src_alpha == 0xFF { + dst.copy_from_slice(src); + return; + } + if src_alpha == 0 { + return; + } + let dst_alpha = u32::from(dst[3]) * (0xFF - src_alpha) / 0xFF; + let out_alpha = src_alpha + dst_alpha; + for (dst_channel, &src_channel) in dst.iter_mut().zip(src).take(3) { + let blended = u32::from(src_channel) * src_alpha + u32::from(*dst_channel) * dst_alpha; + *dst_channel = ((blended + out_alpha / 2) / out_alpha) as u8; + } + dst[3] = out_alpha as u8; +} + +/// Returns true if `data` is an animated GIF, WebP or PNG, by looking at its headers only. +pub fn is_animated_image(data: &[u8]) -> bool { + match detect_image_format(data) { + Some("gif") => gif_has_multiple_frames(data).unwrap_or(false), + // An animated WebP starts with a VP8X chunk whose flags have the animation bit set. + Some("webp") => data.get(12..16) == Some(&b"VP8X"[..]) && data.get(20).is_some_and(|flags| flags & 0x02 != 0), + Some("png") => png_has_multiple_frames(data).unwrap_or(false), + _ => false, + } +} + +/// Returns whether a GIF has at least two frames, skipping over all the blocks before its second one. +fn gif_has_multiple_frames(data: &[u8]) -> Option { + let color_table_len = |flags: u8| if flags & 0x80 != 0 { 3 << ((flags & 0x07) + 1) } else { 0 }; + let skip_sub_blocks = |mut pos: usize| -> Option { + loop { + let len = *data.get(pos)? as usize; + pos += 1 + len; + if len == 0 { + return Some(pos); + } + } + }; + // The header and logical screen descriptor are 13 bytes, followed by an optional global color table. + let mut pos = 13 + color_table_len(*data.get(10)?); + let mut num_frames = 0; + loop { + match *data.get(pos)? { + // An extension: its introducer, its label, then its data sub-blocks. + 0x21 => pos = skip_sub_blocks(pos + 2)?, + // An image descriptor, an optional local color table, the LZW code size, then sub-blocks. + 0x2C => { + num_frames += 1; + if num_frames > 1 { + return Some(true); + } + pos = skip_sub_blocks(pos + 10 + color_table_len(*data.get(pos + 9)?) + 1)?; + } + _ => return Some(false), + } + } +} + +/// Returns whether a PNG has an `acTL` chunk for more than one frame before its image data. +fn png_has_multiple_frames(data: &[u8]) -> Option { + let mut pos = 8; + loop { + let len = u32::from_be_bytes(data.get(pos..pos + 4)?.try_into().ok()?) as usize; + match data.get(pos + 4..pos + 8)? { + b"acTL" => { + let num_frames = u32::from_be_bytes(data.get(pos + 8..pos + 12)?.try_into().ok()?); + return Some(num_frames > 1); + } + b"IDAT" | b"IEND" => return Some(false), + _ => pos = pos.checked_add(12)?.checked_add(len)?, + } + } +} + +/// Decodes `data` with just enough pixels to be drawn `drawn_size` pixels big, +/// or with all of them if that's `None`. +fn decode_image_buffer( + image_path: &Path, + data: &[u8], + drawn_size: Option<(usize, usize)>, +) -> Result { if data.len() > MAX_IMAGE_DECODED_BYTES { return Err(ImageError::DataTooLarge { bytes: data.len(), @@ -1086,7 +1320,7 @@ fn decode_image_buffer(image_path: &Path, data: &[u8]) -> Result ImageBuffer::from_jpg(data), "png" => ImageBuffer::from_png(data), "webp" => ImageBuffer::from_webp(data), @@ -1095,6 +1329,10 @@ fn decode_image_buffer(image_path: &Path, data: &[u8]) -> Result ImageBuffer::from_qoi(data), "ico" => ImageBuffer::from_ico(data), _ => Err(ImageError::UnsupportedFormat), + }?; + match drawn_size { + Some(drawn_size) => buffer.shrink_to_drawn_size(drawn_size), + None => Ok(buffer), } } @@ -1105,6 +1343,14 @@ fn orient(buffer: ImageBuffer, exif: Option<&[u8]>) -> ImageBuffer { apply_exif_orientation(buffer, exif.and_then(tiff_orientation).unwrap_or(1)) } +/// Returns the size of an image that's `width` x `height` once its EXIF orientation is applied. +fn get_oriented_size((width, height): (usize, usize), exif: Option<&[u8]>) -> (usize, usize) { + match exif.and_then(tiff_orientation) { + Some(5..=8) => (height, width), + _ => (width, height), + } +} + /// Reads the Orientation tag (0x0112) from a TIFF/EXIF block that begins at the /// byte-order marker ("II" little-endian or "MM" big-endian). Returns the value /// only when it's a valid 1-8. @@ -1181,11 +1427,11 @@ fn apply_exif_orientation(buf: ImageBuffer, orientation: u16) -> ImageBuffer { dst[dy * dw + dx] = buf.data[y * w + x]; } } - ImageBuffer { width: dw, height: dh, data: dst, animation: buf.animation, max_level: None } + ImageBuffer { width: dw, height: dh, data: dst, animation: buf.animation, max_level: None, natural_size: None } } -/// Returns the `(width, height)` in pixels of an encoded image, auto-detecting -/// the format from the data's magic bytes (falling back to the path's extension). +/// Returns the `(width, height)` in pixels of an encoded image as it's shown (after its EXIF orientation), +/// auto-detecting the format from the data's magic bytes (falling back to the path's extension). /// Reads only headers for most formats; does not fully decode. pub fn image_size_by_data(data: &[u8], image_path: &Path) -> Result<(usize, usize), ImageError> { let format = detect_image_format_from_path_and_data(image_path, data) @@ -1201,7 +1447,7 @@ pub fn image_size_by_data(data: &[u8], image_path: &Path) -> Result<(usize, usiz )) })?; checked_pixel_count(width, height)?; - Ok((width, height)) + Ok(get_oriented_size((width, height), decoder.exif().map(Vec::as_slice))) } "png" => { let cursor = ZCursor::new(data); @@ -1211,16 +1457,19 @@ pub fn image_size_by_data(data: &[u8], image_path: &Path) -> Result<(usize, usiz PngDecodeErrors::GenericStatic("Failed to get PNG image dimensions"), ))?; checked_pixel_count(width, height)?; - Ok((width, height)) + // Animations are never rotated, see `apply_exif_orientation`. + let exif = if decoder.is_animated() { None } else { decoder.info().and_then(|i| i.exif.as_deref()) }; + Ok(get_oriented_size((width, height), exif)) } "webp" => { let cursor = std::io::Cursor::new(data); - let decoder = WebPDecoder::new(std::io::BufReader::new(cursor)) + let mut decoder = WebPDecoder::new(std::io::BufReader::new(cursor)) .map_err(ImageError::WebpDecode)?; let (width, height) = decoder.dimensions(); let (width, height) = (width as usize, height as usize); checked_pixel_count(width, height)?; - Ok((width, height)) + let exif = if decoder.is_animated() { None } else { decoder.exif_metadata().ok().flatten() }; + Ok(get_oriented_size((width, height), exif.as_deref())) } "gif" => { let decoder = DecodeOptions::new() @@ -1584,8 +1833,8 @@ mod tests { .frame_delays .iter() .all(|delay| (*delay - 0.05).abs() < f64::EPSILON)); - assert!(image.width >= 2 * 4); - assert!(image.height >= 2); + // Four frames fit in one row, so the atlas is no wider than that. + assert_eq!((image.width, image.height), (2 * 4, 2)); } #[test] @@ -1602,10 +1851,29 @@ mod tests { } #[test] - fn test_from_png_animated_does_not_populate_frame_delays() { + fn test_from_png_animated_reads_frame_delays() { let image = ImageBuffer::from_png(&animated_png()).unwrap(); let animation = image.animation.as_ref().unwrap(); - assert!(animation.frame_delays.is_empty()); + assert_eq!(animation.num_frames, 4); + assert_eq!(animation.frame_delays, vec![0.05; 4]); + } + + #[test] + fn test_animation_atlas_layout_splits_frames_evenly_across_rows() { + let max_columns = Cx::max_texture_width() / 10; + let layout = animation_atlas_layout(max_columns + 2, 10, 10).unwrap(); + assert_eq!(layout, ((max_columns + 2).div_ceil(2) * 10, 2 * 10)); + } + + #[test] + fn test_is_animated_image() { + assert!(is_animated_image(&animated_gif())); + assert!(!is_animated_image(&single_frame_gif())); + assert!(is_animated_image(&animated_png())); + let mut webp = b"RIFF\0\0\0\0WEBPVP8X\x0a\0\0\0\0\0\0\0".to_vec(); + assert!(!is_animated_image(&webp)); + webp[20] = 0x02; + assert!(is_animated_image(&webp)); } #[test] @@ -1619,7 +1887,7 @@ mod tests { #[test] fn test_decode_image_buffer_rejects_random_bytes_as_unsupported() { assert!(matches!( - decode_image_buffer(Path::new("sticker"), &[0; 16]), + decode_image_buffer(Path::new("sticker"), &[0; 16], None), Err(ImageError::UnsupportedFormat) )); } @@ -1769,7 +2037,7 @@ mod tests { qoi_solid_2x2(10, 20, 30), ico_wrap(&ico_dib_2x2_32bit(10, 20, 30), 2, 2, 32), ] { - let image = decode_image_buffer(Path::new("img"), &data).unwrap(); + let image = decode_image_buffer(Path::new("img"), &data, None).unwrap(); assert_eq!((image.width, image.height), (2, 2)); } } @@ -1789,9 +2057,166 @@ mod tests { Err(ImageError::UnsupportedFormat) )); } + + #[test] + fn decoded_size_halves_while_it_still_has_enough_pixels() { + assert_eq!(get_decoded_size((4032, 3024), (800, 600)), (1008, 756)); + assert_eq!(get_decoded_size((4032, 3024), (1009, 600)), (2016, 1512)); + assert_eq!(get_decoded_size((400, 300), (800, 600)), (400, 300)); + // Odd sizes round up, so no pixel row or column gets dropped. + assert_eq!(get_decoded_size((5, 3), (1, 1)), (1, 1)); + assert_eq!(get_decoded_size((5, 3), (2, 2)), (3, 2)); + assert_eq!(get_decoded_size((1, 1), (1, 1)), (1, 1)); + } + + #[test] + fn shrinking_averages_by_alpha_and_keeps_the_natural_size() { + let red = [255, 0, 0, 255]; + let blue = [0, 0, 255, 255]; + let clear = [0, 0, 0, 0]; + let rows = [[red, red, blue, blue], [red, red, blue, blue], [clear, red, clear, clear]]; + let data: Vec = rows.iter().flatten().flatten().copied().collect(); + let image = ImageBuffer::new(&data, 4, 3).unwrap().shrink_to_drawn_size((2, 2)).unwrap(); + assert_eq!((image.width, image.height, image.natural_size), (2, 2, Some((4, 3)))); + // A transparent pixel lowers the alpha but doesn't darken the color. + assert_eq!(image.data, vec![0xffff0000, 0xff0000ff, 0x7fff0000, 0x00000000]); + + let image = ImageBuffer::new(&data, 4, 3).unwrap().shrink_to_drawn_size((3, 3)).unwrap(); + assert_eq!((image.width, image.height, image.natural_size), (4, 3, None)); + } + + #[test] + fn shrinking_an_animation_shrinks_each_frame() { + let image = decode_image_buffer(Path::new("a.gif"), &animated_gif(), Some((1, 1))).unwrap(); + let animation = image.animation.as_ref().unwrap(); + assert_eq!((animation.width, animation.height, animation.num_frames), (1, 1, 4)); + assert_eq!(animation.frame_delays, vec![0.05; 4]); + assert_eq!(image.natural_size, Some((2, 2))); + assert_eq!((image.width, image.height), (4, 1)); + // Each frame's one pixel averages two pixels of one color and two of the next one. + assert_eq!(image.data, vec![0xff7f0000, 0xff7f7f00, 0xff007f7f, 0xff00007f]); + } + + #[test] + fn a_pending_decode_covers_requests_that_decode_to_the_same_size() { + let pending_decode = PendingDecode { natural_size: (4032, 3024), drawn_size: Some((1000, 700)) }; + assert!(pending_decode.is_big_enough_for(Some((1005, 755)))); + assert!(!pending_decode.is_big_enough_for(Some((1009, 757)))); + assert!(!pending_decode.is_big_enough_for(None)); + let pending_decode = PendingDecode { natural_size: (4032, 3024), drawn_size: None }; + assert!(pending_decode.is_big_enough_for(Some((4000, 3000)))); + assert!(pending_decode.is_big_enough_for(None)); + } + + #[test] + fn oriented_size_swaps_for_turning_exif_orientations() { + let exif = |orientation: u8| { + let mut tiff = b"II\x2a\0\x08\0\0\0\x01\0".to_vec(); + tiff.extend_from_slice(&[0x12, 0x01, 0x03, 0x00, 0x01, 0x00, 0x00, 0x00, orientation, 0, 0, 0]); + tiff + }; + assert_eq!(get_oriented_size((4, 3), Some(&exif(6))), (3, 4)); + assert_eq!(get_oriented_size((4, 3), Some(&exif(8))), (3, 4)); + assert_eq!(get_oriented_size((4, 3), Some(&exif(3))), (4, 3)); + assert_eq!(get_oriented_size((4, 3), None), (4, 3)); + } + + #[test] + fn only_a_decode_with_enough_pixels_counts_as_cached() { + let mut cx = Cx::new(Box::new(|_, _| {})); + ensure_image_cache(&mut cx); + let is_loaded = |result: Option| matches!(result, Some(AsyncLoadResult::Loaded)); + let loading_size = |result: Option| match result { + Some(AsyncLoadResult::Loading(w, h)) => Some((w, h)), + _ => None, + }; + + // An image that's still being fetched gets all of its pixels. + let fetched = Path::new("https://example.com/photo.png"); + cx.get_global::().map.insert(fetched.into(), ImageCacheEntry::Loading(1, 1)); + assert_eq!(loading_size(get_cached_load_result(&mut cx, fetched, None)), Some((1, 1))); + + let path = Path::new("photo.png"); + let mut shrunk = ImageBuffer::new(&[0; 2 * 2 * 4], 2, 2).unwrap(); + shrunk.natural_size = Some((8, 8)); + let texture = shrunk.into_new_texture(&mut cx); + cx.get_global::().insert_loaded(path.into(), texture); + assert!(is_loaded(get_cached_load_result(&mut cx, path, Some((2, 2))))); + assert!(get_cached_load_result(&mut cx, path, Some((3, 3))).is_none()); + assert!(get_cached_load_result(&mut cx, path, None).is_none()); + + // A request that a decode on its way covers waits for it. + cx.get_global::() + .pending_decodes + .insert(path.into(), PendingDecode { natural_size: (8, 8), drawn_size: Some((4, 4)) }); + assert_eq!(loading_size(get_cached_load_result(&mut cx, path, Some((3, 3)))), Some((8, 8))); + assert!(get_cached_load_result(&mut cx, path, Some((5, 5))).is_none()); + } + + #[test] + fn an_image_evicted_while_decoding_goes_once_its_decode_lands() { + let mut cx = Cx::new(Box::new(|_, _| {})); + ensure_image_cache(&mut cx); + let (path, other_path) = (Path::new("photo.png"), Path::new("other.png")); + let decoded = || ImageBuffer::new(&[0; 2 * 2 * 4], 2, 2).unwrap(); + for path in [path, other_path] { + let cache = cx.get_global::(); + cache.map.insert(path.into(), ImageCacheEntry::Loading(2, 2)); + cache.pending_decodes.insert(path.into(), PendingDecode { natural_size: (2, 2), drawn_size: None }); + } + assert!(evict_image_from_cache(&mut cx, path)); + // Its decode still lands in the cache, for anything that's waiting on it. + process_async_image_load(&mut cx, path, Ok(decoded())); + assert!(load_image_from_cache(&mut cx, path).is_some()); + process_async_image_load(&mut cx, other_path, Ok(decoded())); + assert!(load_image_from_cache(&mut cx, path).is_none()); + assert!(load_image_from_cache(&mut cx, other_path).is_some()); + } + + #[test] + fn a_cached_image_with_more_pixels_is_kept() { + let mut cx = Cx::new(Box::new(|_, _| {})); + ensure_image_cache(&mut cx); + let path = Path::new("photo.png"); + let shrunk = |pixels: usize| { + let mut image = ImageBuffer::new(&vec![0; pixels * pixels * 4], pixels, pixels).unwrap(); + image.natural_size = Some((8, 8)); + image + }; + let cached_size = |cx: &mut Cx| { + load_image_from_cache(cx, path).and_then(|texture| texture.get_format(cx).vec_width_height()) + }; + cx.get_global::() + .pending_decodes + .insert(path.into(), PendingDecode { natural_size: (8, 8), drawn_size: Some((4, 4)) }); + process_async_image_load(&mut cx, path, Ok(shrunk(2))); + assert_eq!(cached_size(&mut cx), Some((2, 2))); + // A bigger decode is still on its way. + assert!(is_decoding_image(&mut cx, path, Some((4, 4)))); + + process_async_image_load(&mut cx, path, Ok(shrunk(4))); + assert_eq!(cached_size(&mut cx), Some((4, 4))); + assert!(!is_decoding_image(&mut cx, path, Some((4, 4)))); + let texture = load_image_from_cache(&mut cx, path).unwrap(); + assert!(has_enough_pixels(&mut cx, &texture, Some((4, 4)))); + assert!(!has_enough_pixels(&mut cx, &texture, Some((5, 5)))); + assert!(!has_enough_pixels(&mut cx, &texture, None)); + + // One with fewer pixels that lands later doesn't replace it. + process_async_image_load(&mut cx, path, Ok(shrunk(2))); + assert_eq!(cached_size(&mut cx), Some((4, 4))); + // A failed decode doesn't throw the cached one out either. + process_async_image_load(&mut cx, path, Err(ImageError::EmptyData)); + assert_eq!(cached_size(&mut cx), Some((4, 4))); + } } -fn spawn_decode_job(cx: &mut Cx, image_path: PathBuf, data: Arc) +fn spawn_decode_job( + cx: &mut Cx, + image_path: PathBuf, + data: Arc, + drawn_size: Option<(usize, usize)>, +) where D: AsRef<[u8]> + Send + Sync + ?Sized + 'static, { @@ -1807,7 +2232,7 @@ where image_size_bytes ); } - let result = decode_image_buffer(&image_path, (*data).as_ref()).map(|mut buffer| { + let result = decode_image_buffer(&image_path, (*data).as_ref(), drawn_size).map(|mut buffer| { // Mip-chain generation belongs with the decode: doing it here keeps the // box-filter cost off the UI thread when the texture is committed. buffer.build_cpu_mip_chain_if_uploaded(); @@ -1868,34 +2293,63 @@ pub fn process_async_image_load( // A finished decode frees a slot: hand the next staged one over. let pool = cx.task_pool(); cx.get_global::().decode_queue.pump(&pool); + // Images that were evicted while they were decoding go now if their decode has landed. + let cache = cx.get_global::(); + let landed_paths: Vec = cache.evicted_while_decoding.iter() + .filter(|path| !cache.pending_decodes.contains_key(path.as_path())) + .cloned() + .collect(); + for path in landed_paths { + cx.get_global::().evicted_while_decoding.remove(&path); + evict_image_from_cache(cx, &path); + } if let Ok(data) = result { let width = data.width; let height = data.height; - let upload_start = decode_timing_start(); - let texture = data.into_new_texture(cx); - if image_decode_debug_enabled() { - if let Some(upload_start) = upload_start { - log!( - "ImageCache: gpu_commit key={} elapsed_ms={:.1} size={}x{}", - image_path.display(), - (Cx::monotonic_now() - upload_start) * 1000.0, - width, - height - ); - } else { - log!( - "ImageCache: gpu_commit key={} size={}x{}", - image_path.display(), - width, - height - ); + let cached_texture = load_image_from_cache(cx, image_path); + // A smaller decode that lands after a bigger one doesn't replace it. + let cached_size = cached_texture.as_ref().and_then(|texture| texture.get_format(cx).vec_width_height()); + let texture = match cached_texture { + Some(cached_texture) if cached_size.is_some_and(|(w, h)| w * h >= width * height) => cached_texture, + _ => { + let upload_start = decode_timing_start(); + let texture = data.into_new_texture(cx); + if image_decode_debug_enabled() { + if let Some(upload_start) = upload_start { + log!( + "ImageCache: gpu_commit key={} elapsed_ms={:.1} size={}x{}", + image_path.display(), + (Cx::monotonic_now() - upload_start) * 1000.0, + width, + height + ); + } else { + log!( + "ImageCache: gpu_commit key={} size={}x{}", + image_path.display(), + width, + height + ); + } + } + cx.get_global::() + .insert_loaded(image_path.into(), texture.clone()); + texture } + }; + // It's done decoding, unless a decode with more pixels is still on its way. + let pending_decode = cx.get_global::().pending_decodes.get(image_path).copied(); + if pending_decode.is_some_and(|pending_decode| has_enough_pixels(cx, &texture, pending_decode.drawn_size)) { + cx.get_global::().pending_decodes.remove(image_path); } - cx.get_global::() - .insert_loaded(image_path.into(), texture); } else if let Err(error) = result { error!("ImageCache: decode failed for {}: {}", image_path.display(), error); - cx.get_global::().map.remove(image_path); + let cache = cx.get_global::(); + cache.pending_decodes.remove(image_path); + // An image that's already decoded (with fewer pixels) stays cached. + if matches!(cache.map.get(image_path), Some(ImageCacheEntry::Loading(..))) { + cache.map.remove(image_path); + } } } @@ -1921,6 +2375,8 @@ pub fn process_async_image_load( /// is what the process is still *holding*: the next decode reuses those /// pages instead of asking for more, so a viewer dialing through a folder /// stops climbing. +/// +/// Its GPU copy is released too, unless something else still has a handle to it. pub fn evict_image_from_cache(cx: &mut Cx, image_path: &Path) -> bool { if !cx.has_global::() { return false; @@ -1929,14 +2385,21 @@ pub fn evict_image_from_cache(cx: &mut Cx, image_path: &Path) -> bool { return false; }; if let ImageCacheEntry::Loaded(texture) = entry { + let is_last_handle = texture.readers() == 1; // The decoded pixels; the slot itself goes when `texture` drops. match texture.get_format(cx) { - TextureFormat::VecBGRAu8_32 { data, .. } - | TextureFormat::VecMipBGRAu8_32 { data, .. } => { + // Pixels that something showing them still has to upload are kept. + TextureFormat::VecBGRAu8_32 { data, updated, .. } + | TextureFormat::VecMipBGRAu8_32 { data, updated, .. } + if is_last_handle || updated.is_empty() => + { *data = None; } _ => {} } + if is_last_handle { + texture.release(cx); + } } true } @@ -1978,16 +2441,42 @@ pub fn load_image_from_data_async( image_path: &Path, data: Arc, ) -> Result +where + D: AsRef<[u8]> + Send + Sync + ?Sized + 'static, +{ + load_image_from_data_async_at_size(cx, image_path, data, None) +} + +/// Like [`load_image_from_data_async`], but decodes the image with just enough pixels to be drawn +/// `drawn_size` pixels big (or all of them if `None`), unless it's cached or decoding with that many. +pub fn load_image_from_data_async_at_size( + cx: &mut Cx, + image_path: &Path, + data: Arc, + drawn_size: Option<(usize, usize)>, +) -> Result where D: AsRef<[u8]> + Send + Sync + ?Sized + 'static, { ensure_image_cache_inner(cx); - match cx.get_global::().map.get(image_path) { - Some(ImageCacheEntry::Loaded(_)) => return Ok(AsyncLoadResult::Loaded), - Some(ImageCacheEntry::Loading(w, h)) => return Ok(AsyncLoadResult::Loading(*w, *h)), - None => {} + // It's wanted again, so it stays cached once its decode lands. + cx.get_global::().evicted_while_decoding.remove(image_path); + if let Some(result) = get_cached_load_result(cx, image_path, drawn_size) { + return Ok(result); } let bytes: &[u8] = (*data).as_ref(); + // Decode it once with enough pixels for both this and the decode that's on its way. + let pending_drawn_size = cx.get_global::() + .pending_decodes + .get(image_path) + .and_then(|pending_decode| pending_decode.drawn_size); + let drawn_size = match (pending_drawn_size, drawn_size) { + (Some(pending_drawn_size), Some(drawn_size)) => Some(( + pending_drawn_size.0.max(drawn_size.0), + pending_drawn_size.1.max(drawn_size.1), + )), + _ => drawn_size, + }; if bytes.len() > MAX_IMAGE_DECODED_BYTES { return Err(ImageError::DataTooLarge { bytes: bytes.len(), @@ -2004,7 +2493,7 @@ where let force_sync = gpusim_mode_enabled(); if force_sync { - let image = decode_image_buffer(image_path, bytes)?; + let image = decode_image_buffer(image_path, bytes, drawn_size)?; let texture = image.into_new_texture(cx); cx.get_global::() .insert_loaded(image_path.into(), texture); @@ -2021,35 +2510,66 @@ where h ); } - cx.get_global::() - .map - .insert(image_path.into(), ImageCacheEntry::Loading(w, h)); - spawn_decode_job(cx, image_path.to_path_buf(), data); + let cache = cx.get_global::(); + cache.pending_decodes.insert(image_path.into(), PendingDecode { natural_size: (w, h), drawn_size }); + // An image that's cached with fewer pixels stays cached until this decode lands. + cache.map.entry(image_path.into()).or_insert(ImageCacheEntry::Loading(w, h)); + spawn_decode_job(cx, image_path.to_path_buf(), data, drawn_size); Ok(AsyncLoadResult::Loading(w, h)) } +/// Returns how loading the image at `image_path` with enough pixels to be drawn at `drawn_size` +/// (or with all of them if `None`) goes if the cache already has it or is getting it. +fn get_cached_load_result(cx: &mut Cx, image_path: &Path, drawn_size: Option<(usize, usize)>) -> Option { + let cache = cx.get_global::(); + let pending_decode = cache.pending_decodes.get(image_path).copied(); + let cached_texture = match cache.map.get(image_path) { + Some(ImageCacheEntry::Loaded(texture)) => Some(texture.clone()), + // It's still being fetched, so it'll be decoded with all of its pixels. + Some(ImageCacheEntry::Loading(w, h)) if pending_decode.is_none() => { + return Some(AsyncLoadResult::Loading(*w, *h)); + } + _ => None, + }; + if cached_texture.is_some_and(|texture| has_enough_pixels(cx, &texture, drawn_size)) { + return Some(AsyncLoadResult::Loaded); + } + // A decode with enough pixels that's on its way just needs waiting for. + let (w, h) = pending_decode + .filter(|pending_decode| pending_decode.is_big_enough_for(drawn_size))? + .natural_size; + Some(AsyncLoadResult::Loading(w, h)) +} + +/// Returns whether the image at `image_path` is being decoded with enough pixels +/// to be drawn `drawn_size` pixels big, or with all of them if that's `None`. +pub fn is_decoding_image(cx: &mut Cx, image_path: &Path, drawn_size: Option<(usize, usize)>) -> bool { + cx.has_global::() + && cx.get_global::().pending_decodes.get(image_path) + .is_some_and(|pending_decode| pending_decode.is_big_enough_for(drawn_size)) +} + pub fn load_image_file_by_path_async( cx: &mut Cx, image_path: &Path, ) -> Result { ensure_image_cache_inner(cx); - match cx.get_global::().map.get(image_path) { - Some(ImageCacheEntry::Loaded(_)) => Ok(AsyncLoadResult::Loaded), - Some(ImageCacheEntry::Loading(w, h)) => Ok(AsyncLoadResult::Loading(*w, *h)), - None => match read_image_file_limited(image_path) { - Ok(data) => load_image_from_data_async(cx, image_path, Arc::new(data)), - Err(err) => Err(err), - }, + // It's wanted again, so it stays cached once its decode lands. + cx.get_global::().evicted_while_decoding.remove(image_path); + if let Some(result) = get_cached_load_result(cx, image_path, None) { + return Ok(result); } + let data = read_image_file_limited(image_path)?; + load_image_from_data_async(cx, image_path, Arc::new(data)) } pub fn load_image_http_by_url_async(cx: &mut Cx, url: &str) -> Result { ensure_image_cache_inner(cx); let image_path = PathBuf::from(url); - match cx.get_global::().map.get(&image_path) { - Some(ImageCacheEntry::Loaded(_)) => return Ok(AsyncLoadResult::Loaded), - Some(ImageCacheEntry::Loading(w, h)) => return Ok(AsyncLoadResult::Loading(*w, *h)), - None => {} + // It's wanted again, so it stays cached once its decode lands. + cx.get_global::().evicted_while_decoding.remove(&image_path); + if let Some(result) = get_cached_load_result(cx, &image_path, None) { + return Ok(result); } let request_id = LiveId::unique(); @@ -2300,7 +2820,7 @@ pub trait ImageCacheImpl { id: usize, image_path: &Path, ) -> Result<(), ImageError> { - let image = decode_image_buffer(image_path, data)?; + let image = decode_image_buffer(image_path, data, None)?; let texture = image.into_new_texture(cx); ensure_image_cache(cx); cx.get_global::() @@ -2315,7 +2835,7 @@ pub trait ImageCacheImpl { image_path: &Path, id: usize, ) -> Result<(), ImageError> { - if let Some(texture) = load_image_from_cache(cx, image_path) { + if let Some(texture) = load_image_from_cache(cx, image_path).filter(|texture| has_enough_pixels(cx, texture, None)) { self.set_texture(Some(texture), id); return Ok(()); } @@ -2330,7 +2850,7 @@ pub trait ImageCacheImpl { id: usize, ) -> Result<(), ImageError> { let p_image_path = Path::new(image_path); - if let Some(texture) = load_image_from_cache(cx, p_image_path) { + if let Some(texture) = load_image_from_cache(cx, p_image_path).filter(|texture| has_enough_pixels(cx, texture, None)) { self.set_texture(Some(texture), id); return Ok(()); } diff --git a/draw/src/lib.rs b/draw/src/lib.rs index e7f9688b36..e9e87b6dd3 100644 --- a/draw/src/lib.rs +++ b/draw/src/lib.rs @@ -28,9 +28,9 @@ pub use crate::{ draw_list_2d::{DrawList2d, DrawListExt, ManyInstances, Redrawing, RedrawingApi}, image_cache::{ decode_image_from_data, evict_image_from_cache, handle_image_cache_network_responses, - image_size_by_data, + has_enough_pixels, image_size_by_data, is_animated_image, is_decoding_image, looks_like_svg, load_image_file_by_path_async, load_image_from_cache, - load_image_from_data_async, + load_image_from_data_async, load_image_from_data_async_at_size, load_image_http_by_url_async, process_async_image_load, AsyncImageLoad, AsyncLoadResult, ImageBuffer, ImageCache, ImageCacheImpl, ImageError, JpgDecodeErrors, PngDecodeErrors, }, diff --git a/platform/src/texture.rs b/platform/src/texture.rs index 74640497be..cdf0cb87e3 100644 --- a/platform/src/texture.rs +++ b/platform/src/texture.rs @@ -1456,6 +1456,17 @@ impl Texture { cx.textures[self.texture_id()].animation = animation; } + /// Records that this texture holds a shrunk copy of an image (or of each frame of an + /// animation) that's `natural_size` pixels big, so it can still be laid out at that size. + pub fn set_natural_size(&self, cx: &mut Cx, natural_size: Option<(usize, usize)>) { + cx.textures[self.texture_id()].natural_size = natural_size; + } + + /// The size of the image this texture holds a shrunk copy of, if it was shrunk. + pub fn natural_size(&self, cx: &Cx) -> Option<(usize, usize)> { + cx.textures[self.texture_id()].natural_size + } + /// Mark a render target as an application-held cache (see /// `CxTexture::retained_render_target`). pub fn set_retained_render_target(&self, cx: &mut Cx, retained: bool) { @@ -1594,6 +1605,7 @@ pub struct CxTexture { pub(crate) allocation_generation: u64, pub(crate) producer_serial: u64, pub(crate) animation: Option, + pub(crate) natural_size: Option<(usize, usize)>, /// A render target the application keeps as a cache: a software backend /// never releases its framebuffer for idleness or budget while the /// handle lives (GPU backends keep every target anyway). diff --git a/widgets/src/image.rs b/widgets/src/image.rs index 6699756ecf..f359fcedd0 100644 --- a/widgets/src/image.rs +++ b/widgets/src/image.rs @@ -43,7 +43,8 @@ script_mod! { let cr = vec2(c.x * cos_a - c.y * sin_a, c.x * sin_a + c.y * cos_a) let iuv = cr / vec2(self.image_dim_w, self.image_dim_h) + vec2(0.5, 0.5) let uv = iuv * scale + pan - if uv.x < 0.0 || uv.x > 1.0 || uv.y < 0.0 || uv.y > 1.0 { + // Also check iuv, since an animated texture's other frames sit right next to this one. + if iuv.x < 0.0 || iuv.x > 1.0 || iuv.y < 0.0 || iuv.y > 1.0 || uv.x < 0.0 || uv.x > 1.0 || uv.y < 0.0 || uv.y > 1.0 { return vec4(0.0, 0.0, 0.0, 0.0) } return self.image_texture.sample_as_bgra(uv) @@ -121,6 +122,9 @@ pub enum ImageAnimation { LoopFps(f64), #[live(60.0)] BounceFps(f64), + /// Shows each frame for as long as the image says to, looping forever, + /// the way browsers play GIFs. It only ticks while the image is on screen. + Natural, } #[derive(Script, ScriptHook, Widget, Animator)] @@ -142,12 +146,18 @@ pub struct Image { placeholder_height: u64, #[live(1.0)] width_scale: f64, - #[live(ImageAnimation::BounceFps(25.0))] + #[live(ImageAnimation::Natural)] animation: ImageAnimation, #[rust] last_time: Option, #[rust] animation_frame: f64, + /// When a `Natural` animation shows its next frame, on the `AnimationClock`'s timeline. + /// `None` while it's stopped, e.g., because it went off screen. + #[rust] + next_frame_time: Option, + #[rust] + shown_animation: Option, #[visible] #[live(true)] visible: bool, @@ -155,6 +165,11 @@ pub struct Image { next_frame: NextFrame, #[live] fit: ImageFit, + /// Decodes images loaded from data with just enough pixels for the size they're drawn at, + /// once they're drawn, and again if they're later drawn bigger. So an image that isn't drawn + /// until it has loaded (e.g., `Media`'s picture) never loads with this on. + #[live] + downscale_to_drawn_size: bool, /// HTTP/file resource handle for loading image data (set via `http_resource()` or `crate_resource()`) #[live] src: Option, @@ -164,6 +179,9 @@ pub struct Image { async_image_path: Option, #[rust] async_image_size: Option<(usize, usize)>, + /// The image being loaded or shown, if `downscale_to_drawn_size` might need to decode it again. + #[rust] + encoded_image: Option, #[rust] texture: Option, /// The async-load key that produced `texture`, when it came from a completed @@ -189,6 +207,42 @@ pub struct Image { svg_time: f64, } +/// The animated texture an `Image` is drawing, plus the `image_scale` and `image_pan` +/// it had before we pointed them at one frame of it, so a static texture gets them back. +struct ShownAnimation { + texture_id: TextureId, + orig_image_scale: Vec2f, + orig_image_pan: Vec2f, +} + +/// An image that's decoded with just enough pixels for the size it's drawn at. +struct EncodedImage { + image_path: PathBuf, + /// Loads it with enough pixels to be drawn at the given size. + load_at_size: Box Result>, + /// The biggest size we've loaded it to be drawn at. + drawn_size: Option<(usize, usize)>, +} + +/// Browsers show a frame that asks for 10ms or less for this long instead, +/// and GIFs made for the web are made with that in mind. +const DEFAULT_FRAME_DELAY_SECS: f64 = 0.1; + +/// Frames due within this long after a clock tick change on that tick, since the screen +/// only updates once per display frame anyway. This lets more of them share a repaint. +const ANIMATION_TICK_SLACK_SECS: f64 = 0.008; + +/// One clock that steps every `Natural` animation, so all the frames that are due +/// at about the same time change together, in one timer event and one repaint. +#[derive(Default)] +struct AnimationClock { + /// The timer for the next tick, and when it'll fire. + next_tick: Timer, + next_tick_time: f64, + /// The tick that's being handled right now. + current_tick: Timer, +} + impl ImageCacheImpl for Image { fn get_texture(&self, _id: usize) -> &Option { &self.texture @@ -369,6 +423,7 @@ impl Widget for Image { if self.async_image_size.is_some() && self.async_image_path.as_deref() == Some(image_path.as_path()) { + let loading_size = self.async_image_size; // see if we can load from cache if !self.load_image_from_cache(cx, image_path, 0) && self.src.is_some() { // Another completion in this Actions batch may @@ -381,7 +436,27 @@ impl Widget for Image { // Record which async load produced the texture, so a later // load for a different key knows it is stale. self.texture_async_source = Some(image_path.to_path_buf()); - self.animator_play(cx, ids!(async_load.off)); + // A decode with fewer pixels (for another image showing this one) can land + // before ours does, so we show that one while we keep waiting for ours. + let drawn_size = self.encoded_image.as_ref() + .filter(|encoded_image| encoded_image.image_path == *image_path) + .map(|encoded_image| encoded_image.drawn_size); + let is_waiting_for_more_pixels = match drawn_size { + // It hasn't been drawn yet, so any decode of it will do for now. + Some(None) => false, + // Without `downscale_to_drawn_size`, it wants all of its pixels. + drawn_size => { + let drawn_size = drawn_size.flatten(); + is_decoding_image(cx, image_path, drawn_size) + && !self.texture.as_ref().is_some_and(|texture| has_enough_pixels(cx, texture, drawn_size)) + } + }; + if is_waiting_for_more_pixels { + self.async_image_size = loading_size; + self.async_image_path = Some(image_path.to_path_buf()); + } else { + self.animator_play(cx, ids!(async_load.off)); + } self.redraw(cx); } } @@ -458,6 +533,8 @@ impl Widget for Image { } self.next_frame = cx.new_next_frame(); } + // Natural animations step on the `AnimationClock` instead. + ImageAnimation::Natural => {} } // alright now lets turn animation_frame into the right image_pan let last_pan = self.draw_bg.image_pan; @@ -468,12 +545,11 @@ impl Widget for Image { self.animation_frame } as usize; - let horizontal_frames = texture_width / animation.width; - let xpos = ((frame % horizontal_frames) * animation.width) as f32 - / texture_width as f32; - let ypos = ((frame / horizontal_frames) * animation.height) as f32 - / texture_height as f32; - self.draw_bg.image_pan = vec2(xpos, ypos); + self.draw_bg.image_pan = get_frame_pan( + frame, + (animation.width, animation.height), + (texture_width, texture_height), + ); if self.draw_bg.image_pan != last_pan { // patch it into the area self.draw_bg.update_instance_area_value(cx, ids!(image_pan)) @@ -481,6 +557,15 @@ impl Widget for Image { } } } + if let Some(next_frame_time) = self.next_frame_time { + if is_animation_tick(cx, event) { + if next_frame_time <= Cx::monotonic_now() + ANIMATION_TICK_SLACK_SECS { + self.show_next_animation_frame(cx, next_frame_time); + } else { + request_animation_tick(cx, next_frame_time); + } + } + } } fn draw_walk(&mut self, cx: &mut Cx2d, _scope: &mut Scope, walk: Walk) -> DrawStep { @@ -535,9 +620,47 @@ impl Image { .svg_size() .map(|sz| (sz.x as usize, sz.y as usize)); } - self.texture - .as_ref() - .and_then(|t| t.get_format(cx).vec_width_height()) + let texture = self.texture.as_ref()?; + if let Some(natural_size) = texture.natural_size(cx) { + return Some(natural_size); + } + // An animated texture holds all of its frames, so its own size isn't the image's size. + if let Some(animation) = texture.animation(cx) { + return Some((animation.width, animation.height)); + } + texture.get_format(cx).vec_width_height() + } + + /// Shows the next frame of a `Natural` animation, which was due at `frame_time`, + /// then waits as long as that frame asks. + /// + /// This stops once the image is no longer on screen, and drawing it again restarts it. + fn show_next_animation_frame(&mut self, cx: &mut Cx, frame_time: f64) { + self.next_frame_time = None; + let Some(texture) = self.texture.clone() else { return }; + let Some(texture_size) = texture.get_format(cx).vec_width_height() else { return }; + let Some((frame_width, frame_height, num_frames)) = texture.animation(cx).as_ref() + .map(|animation| (animation.width, animation.height, animation.num_frames)) + else { + return; + }; + if num_frames < 2 || !is_area_on_screen(cx, self.draw_bg.area()) { + return; + } + let frame = (self.animation_frame as usize + 1) % num_frames; + self.animation_frame = frame as f64; + self.draw_bg.image_pan = get_frame_pan(frame, (frame_width, frame_height), texture_size); + self.draw_bg.update_instance_area_value(cx, ids!(image_pan)); + let frame_delay = texture.animation(cx).as_ref() + .map_or(DEFAULT_FRAME_DELAY_SECS, |animation| get_frame_delay(animation, frame)); + // If we fell behind (e.g., the app was busy), get back on the clock's timeline + // instead of rushing through frames to catch up. + let now = Cx::monotonic_now(); + let next_frame_time = Some(frame_time + frame_delay) + .filter(|&time| time >= now) + .unwrap_or_else(|| get_aligned_frame_time(now, frame_delay)); + self.next_frame_time = Some(next_frame_time); + request_animation_tick(cx, next_frame_time); } /// True if a texture has been set on this `Image`. @@ -643,7 +766,26 @@ impl Image { let rect = cx.peek_walk_turtle(walk); let dpi = cx.current_dpi_factor(); - let (width, height) = if let Some((w, h)) = &self.async_image_size { + // A decode that landed while this wasn't getting events (e.g., in a closed modal) is picked up now. + let missed_decode = self.async_image_path.as_deref().and_then(|image_path| { + let drawn_size = self.encoded_image.as_ref() + .filter(|encoded_image| encoded_image.image_path == image_path) + .and_then(|encoded_image| encoded_image.drawn_size); + if is_decoding_image(cx, image_path, drawn_size) { + return None; + } + load_image_from_cache(cx, image_path).map(|texture| (image_path.to_path_buf(), texture)) + }); + if let Some((image_path, texture)) = missed_decode { + self.set_texture(Some(texture), 0); + self.finish_async_load(cx, &image_path); + } + // A decode of the image that's loading (just with fewer pixels) is drawn like any other, + // so an animation keeps playing while it waits for a bigger decode. + let is_showing_loading_image = self.async_image_path.is_some() + && self.texture.is_some() + && self.texture_async_source == self.async_image_path; + let (width, height) = if let Some((w, h)) = self.async_image_size.filter(|_| !is_showing_loading_image) { // Still loading. Any texture present here is legitimate current content // (the occupant's own placeholder, or a previous load of this same // source; begin_async_load already cleared stale ones), so keep showing @@ -654,7 +796,7 @@ impl Image { } else { self.draw_bg.draw_vars.empty_texture(0); } - (*w as f64, *h as f64) + (w as f64, h as f64) } else if let Some(image_texture) = &self.texture { self.draw_bg.draw_vars.set_texture(0, image_texture); let (width, height) = image_texture @@ -665,14 +807,56 @@ impl Image { // min_* (usually zero) and the picture silently vanished. .or_else(|| image_texture.get_format(cx).render_fixed_width_height()) .unwrap_or((self.placeholder_width as usize, self.placeholder_height as usize)); - if let Some(animation) = image_texture.animation(cx) { - let (w, h) = (animation.width as f64, animation.height as f64); - self.next_frame = cx.new_next_frame(); + let texture_id = image_texture.texture_id(); + // A texture with fewer pixels than its image is still laid out at the image's size. + let natural_size = image_texture.natural_size(cx); + let frame_info = image_texture.animation(cx).as_ref() + .map(|animation| (animation.width, animation.height, animation.num_frames)); + if frame_info.is_none() { + if let Some(shown) = self.shown_animation.take() { + self.draw_bg.image_scale = shown.orig_image_scale; + self.draw_bg.image_pan = shown.orig_image_pan; + self.next_frame_time = None; + } + } + if let Some((frame_width, frame_height, num_frames)) = frame_info { + if self.shown_animation.as_ref().is_none_or(|shown| shown.texture_id != texture_id) { + // A newly shown animation starts over from its first frame. + let (orig_image_scale, orig_image_pan) = match self.shown_animation.take() { + Some(shown) => (shown.orig_image_scale, shown.orig_image_pan), + None => (self.draw_bg.image_scale, self.draw_bg.image_pan), + }; + self.shown_animation = Some(ShownAnimation { texture_id, orig_image_scale, orig_image_pan }); + self.animation_frame = 0.0; + self.last_time = None; + self.draw_bg.image_pan = vec2(0.0, 0.0); + self.next_frame_time = None; + } + if !matches!(self.animation, ImageAnimation::Natural) { + self.next_frame = cx.new_next_frame(); + } else if num_frames > 1 { + // Drawing a natural animation (re)starts it, e.g., after it was off screen, + // or after something it's in kept the clock's tick from reaching it. + let now = Cx::monotonic_now(); + let next_frame_time = match self.next_frame_time { + Some(time) if time + ANIMATION_TICK_SLACK_SECS >= now => time, + _ => { + let frame_delay = image_texture.animation(cx).as_ref().map_or( + DEFAULT_FRAME_DELAY_SECS, + |animation| get_frame_delay(animation, self.animation_frame as usize), + ); + get_aligned_frame_time(now, frame_delay) + } + }; + self.next_frame_time = Some(next_frame_time); + request_animation_tick(cx, next_frame_time); + } // we have an animation. lets compute the scale and zoom for a certain frame - let scale_x = w as f32 / width as f32; - let scale_y = h as f32 / height as f32; + let scale_x = frame_width as f32 / width as f32; + let scale_y = frame_height as f32 / height as f32; self.draw_bg.image_scale = vec2(scale_x, scale_y); - (w, h) + let (frame_width, frame_height) = natural_size.unwrap_or((frame_width, frame_height)); + (frame_width as f64, frame_height as f64) } else if image_texture.get_format(cx).is_render() { // Render targets are stored top-left on EVERY backend now // (GL renders offscreen through a Y-inverted projection), @@ -680,6 +864,7 @@ impl Image { // unconditional flip here showed them upside down. (width as f64 * self.width_scale, height as f64) } else { + let (width, height) = natural_size.unwrap_or((width, height)); (width as f64 * self.width_scale, height as f64) } } else { @@ -744,15 +929,83 @@ impl Image { self.draw_bg.draw_walk(cx, walk); + // Load the image this is loading or showing again if it's now drawn bigger + // than it was ever loaded for, unless it already has (or is getting) enough pixels. + if !self.downscale_to_drawn_size { + return DrawStep::done(); + } + let is_current = self.encoded_image.as_ref() + .is_some_and(|encoded_image| self.is_loading_or_showing(&encoded_image.image_path)); + if !is_current { + // It now shows something else, e.g., a texture that was set directly. + self.encoded_image = None; + return DrawStep::done(); + } + let (drawn_width, drawn_height) = { + // When it's cropped to fill its rect, the whole image is drawn bigger than that rect. + let rect = self.draw_bg.area().rect(cx); + ( + (rect.size.x * dpi / self.draw_bg.fit_scale.x as f64).ceil(), + (rect.size.y * dpi / self.draw_bg.fit_scale.y as f64).ceil(), + ) + }; + if !(drawn_width >= 1.0 && drawn_height >= 1.0) { + return DrawStep::done(); + } + let (drawn_width, drawn_height) = (drawn_width as usize, drawn_height as usize); + let Some(encoded_image) = self.encoded_image.as_mut() else { return DrawStep::done() }; + let drawn_size = match encoded_image.drawn_size { + Some((width, height)) if width >= drawn_width && height >= drawn_height => return DrawStep::done(), + Some((width, height)) => (width.max(drawn_width), height.max(drawn_height)), + None => (drawn_width, drawn_height), + }; + encoded_image.drawn_size = Some(drawn_size); + if self.async_image_path.is_none() + && self.texture.as_ref().is_some_and(|texture| has_enough_pixels(cx, texture, Some(drawn_size))) + { + return DrawStep::done(); + } + let image_path = encoded_image.image_path.clone(); + match (encoded_image.load_at_size)(cx, drawn_size) { + Ok(AsyncLoadResult::Loaded) => { + // The cache had enough pixels already, e.g., from another image showing this one. + if self.load_image_from_cache(cx, &image_path, 0) { + self.finish_async_load(cx, &image_path); + cx.redraw_area_in_draw(self.draw_bg.area()); + } + } + Ok(AsyncLoadResult::Loading(width, height)) => { + if self.async_image_path.is_none() { + // Keep showing the one with fewer pixels, at the same size, until this one lands. + let natural_size = self.texture.as_ref().and_then(|texture| texture.natural_size(cx)); + self.begin_async_load(cx, &image_path, natural_size.unwrap_or((width, height))); + } + } + Err(_) => self.cancel_async_load(cx), + } DrawStep::done() } + /// Returns whether this image is already loading or showing the image at `image_path`. + /// + /// Loading it again then changes nothing, which matters for widgets that re-issue + /// their loads on every draw (e.g., list items), since a redraw would just loop. + fn is_loading_or_showing(&self, image_path: &Path) -> bool { + match self.async_image_path.as_deref() { + Some(loading_path) => loading_path == image_path, + None => self.texture.is_some() && self.texture_async_source.as_deref() == Some(image_path), + } + } + /// Loads the image at the given `image_path` on disk into this `ImageRef`. pub fn load_image_file_by_path_async( &mut self, cx: &mut Cx, image_path: &Path, ) -> Result<(), ImageError> { + if self.is_loading_or_showing(image_path) { + return Ok(()); + } self.lazy_create_image_cache(cx); match self.load_image_file_by_path_async_impl(cx, image_path, 0) { Ok(AsyncLoadResult::Loading(w, h)) => { @@ -777,7 +1030,34 @@ impl Image { where D: AsRef<[u8]> + Send + Sync + ?Sized + 'static, { + if self.is_loading_or_showing(image_path) { + return Ok(()); + } self.lazy_create_image_cache(cx); + if self.downscale_to_drawn_size { + // Show any cached decode of it right away, since drawing it + // then decodes it again if it needs more pixels. + if self.load_image_from_cache(cx, image_path, 0) { + self.finish_async_load(cx, image_path); + } else { + match image_size_by_data((*data).as_ref(), image_path) { + Ok(size) => self.begin_async_load(cx, image_path, size), + Err(_) => { + self.cancel_async_load(cx); + return Ok(()); + } + } + } + let path = image_path.to_path_buf(); + self.encoded_image = Some(EncodedImage { + image_path: image_path.to_path_buf(), + load_at_size: Box::new(move |cx, drawn_size| { + load_image_from_data_async_at_size(cx, &path, data.clone(), Some(drawn_size)) + }), + drawn_size: None, + }); + return Ok(()); + } match self.load_image_from_data_async_impl(cx, image_path, data, 0) { Ok(AsyncLoadResult::Loading(w, h)) => { self.begin_async_load(cx, image_path, (w, h)); @@ -797,6 +1077,9 @@ impl Image { cx: &mut Cx, url: &str, ) -> Result<(), ImageError> { + if self.is_loading_or_showing(Path::new(url)) { + return Ok(()); + } self.lazy_create_image_cache(cx); match self.load_image_http_by_url_async_impl(cx, url, 0) { Ok(AsyncLoadResult::Loading(w, h)) => { @@ -841,15 +1124,6 @@ impl Image { /// The requested image was already cached and its texture has been set: clear /// the pending-load state so the draw path binds the texture directly. fn finish_async_load(&mut self, cx: &mut Cx, image_path: &Path) { - // Re-loading the image that is already bound changes nothing; skip the - // animator and redraw so widgets that re-issue loads on every draw - // (e.g. list items repopulated per frame) don't dirty themselves into - // an endless redraw loop. - if self.async_image_path.is_none() - && self.texture_async_source.as_deref() == Some(image_path) - { - return; - } self.async_image_size = None; self.async_image_path = None; // Record which load produced the texture, just like the decode-completion @@ -877,6 +1151,72 @@ impl Image { } } +/// Returns the `image_pan` that shows the given frame of an animated texture, +/// which holds its frames left to right in rows. +fn get_frame_pan(frame: usize, frame_size: (usize, usize), texture_size: (usize, usize)) -> Vec2f { + let columns = (texture_size.0 / frame_size.0.max(1)).max(1); + vec2( + ((frame % columns) * frame_size.0) as f32 / texture_size.0 as f32, + ((frame / columns) * frame_size.1) as f32 / texture_size.1 as f32, + ) +} + +/// Returns when a frame shown from `now` should change: the first multiple of its delay +/// that's at least a delay away, so animations with the same frame delay change together. +fn get_aligned_frame_time(now: f64, frame_delay: f64) -> f64 { + ((now + frame_delay) / frame_delay).ceil() * frame_delay +} + +/// Asks the shared `AnimationClock` to tick at the given time, unless it'll already tick by then. +fn request_animation_tick(cx: &mut Cx, time: f64) { + if !cx.has_global::() { + cx.set_global(AnimationClock::default()); + } + let now = Cx::monotonic_now(); + let clock = cx.get_global::(); + // A tick that's past due is never trusted, since its event may have gone unhandled. + if !clock.next_tick.is_empty() && clock.next_tick_time <= time && clock.next_tick_time >= now { + return; + } + let later_tick = std::mem::replace(&mut clock.next_tick, Timer::empty()); + cx.stop_timer(later_tick); + let next_tick = cx.start_timeout((time - now).max(0.0)); + let clock = cx.get_global::(); + clock.next_tick = next_tick; + clock.next_tick_time = time; +} + +/// Returns whether the given event is a tick of the shared `AnimationClock`. +fn is_animation_tick(cx: &mut Cx, event: &Event) -> bool { + let Event::Timer(timer_event) = event else { return false }; + if !cx.has_global::() { + return false; + } + let clock = cx.get_global::(); + // The first image to see a tick frees the clock up to be scheduled again. + if !clock.next_tick.is_empty() && timer_event.timer_id == clock.next_tick.0 { + clock.current_tick = std::mem::replace(&mut clock.next_tick, Timer::empty()); + } + !clock.current_tick.is_empty() && timer_event.timer_id == clock.current_tick.0 +} + +/// Returns how long the given frame of an animation should be shown, in seconds. +fn get_frame_delay(animation: &TextureAnimation, frame: usize) -> f64 { + animation.frame_delays.get(frame) + .copied() + .filter(|&delay| delay > 0.0101) + .unwrap_or(DEFAULT_FRAME_DELAY_SECS) +} + +/// Returns whether the given area is on screen right now, unlike one in a hidden +/// dock tab or one whose draw list has since been redrawn without it. +fn is_area_on_screen(cx: &Cx, area: Area) -> bool { + area.is_valid(cx) && area.draw_list_id() + .and_then(|draw_list_id| cx.draw_lists[draw_list_id].draw_pass_id) + .is_some_and(|pass_id| !cx.pass_attachment_is_stale(pass_id) + && area.is_attached(cx, &cx.attached_draw_lists(pass_id))) +} + pub enum AsyncLoad { Yes, No, diff --git a/widgets/src/image_cache.rs b/widgets/src/image_cache.rs index 42025c5f84..ac156f9351 100644 --- a/widgets/src/image_cache.rs +++ b/widgets/src/image_cache.rs @@ -27,8 +27,9 @@ pub enum ImageFit { } pub use makepad_draw::{ - decode_image_from_data, handle_image_cache_network_responses, image_size_by_data, - looks_like_svg, load_image_file_by_path_async, load_image_from_cache, load_image_from_data_async, + decode_image_from_data, handle_image_cache_network_responses, has_enough_pixels, + image_size_by_data, is_decoding_image, looks_like_svg, load_image_file_by_path_async, + load_image_from_cache, load_image_from_data_async, load_image_from_data_async_at_size, load_image_http_by_url_async, process_async_image_load, AsyncImageLoad, AsyncLoadResult, ImageBuffer, ImageCache, ImageCacheImpl, ImageError, JpgDecodeErrors, PngDecodeErrors, }; From ace2e50337cc52f85e67554ad21a2f2069fd730e Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Fri, 9 Oct 2026 00:56:20 -0700 Subject: [PATCH 15/16] Avoid intermediate caret fade frames and retain full widget diagnostics --- platform/src/perf_monitor.rs | 5 ++++- widgets/src/text_input.rs | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/platform/src/perf_monitor.rs b/platform/src/perf_monitor.rs index 62d4eaa751..13a0e8cf58 100644 --- a/platform/src/perf_monitor.rs +++ b/platform/src/perf_monitor.rs @@ -19,7 +19,10 @@ use std::{cell::Cell, collections::HashMap, rc::Rc}; pub const PERF_MONITOR_HISTORY: usize = 240; pub const PERF_MONITOR_MAX_CHANNELS: usize = 12; -pub const PERF_MONITOR_MAX_WORK: usize = 256; +// A full application has more than 256 operation/type pairs even though +// repeated row instances share entries. Keep captures bounded without losing +// most of their attribution before the first conversation is opened. +pub const PERF_MONITOR_MAX_WORK: usize = 1024; /// Aggregate by operation and static component type, never by user content or /// instance ID. Scrolling through new widgets cannot grow this table unbounded. diff --git a/widgets/src/text_input.rs b/widgets/src/text_input.rs index 4090fbec3b..849fbc3e8d 100644 --- a/widgets/src/text_input.rs +++ b/widgets/src/text_input.rs @@ -378,13 +378,13 @@ script_mod! { blink: { default: @off off: AnimatorState{ - from: {all: Forward {duration: 0.05}} + from: {all: Snap} apply: { draw_cursor: {blink: 0.0} } } on: AnimatorState{ - from: {all: Forward {duration: 0.05}} + from: {all: Snap} apply: { draw_cursor: {blink: 1.0} } From 07825feab684d21f69b4afb1ec60f623c13f9183 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Fri, 9 Oct 2026 02:01:26 -0700 Subject: [PATCH 16/16] Retire PortalList drags when a child takes pointer capture --- widgets/src/portal_list.rs | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/widgets/src/portal_list.rs b/widgets/src/portal_list.rs index 075e92a70e..3d56f7d169 100644 --- a/widgets/src/portal_list.rs +++ b/widgets/src/portal_list.rs @@ -1845,6 +1845,45 @@ impl PortalList { self.caught_fling = None; } + /// A child can take the finger (for example, a long-press text selection) + /// without this list receiving FingerUp. Retire that drag immediately so its + /// samples cannot classify the next press as a catch and suppress child moves. + fn cancel_drag_without_capture(&mut self, cx: &mut Cx) { + if !matches!(self.scroll_state, ScrollState::Drag { .. }) + || cx.fingers.is_area_captured(self.area) + { + return; + } + self.was_scrolling = false; + self.suppress_child_events = false; + self.stretching = false; + self.caught_fling = None; + self.momentum = MomentumStream::Idle; + self.scroll_state = if self.bounce_at_start + && self.first_id == self.range_start && self.first_scroll > 0.0 + { + ScrollState::Pulldown { + next_frame: cx.new_next_frame(), + x0: self.first_scroll, + v0: 0.0, + clock: FrameClock::default(), + at_start: true, + touch: true, + } + } else if self.bounce_at_end && self.bounce_overshoot > 0.0 { + ScrollState::Pulldown { + next_frame: cx.new_next_frame(), + x0: self.bounce_overshoot, + v0: 0.0, + clock: FrameClock::default(), + at_start: false, + touch: true, + } + } else { + ScrollState::Stopped + }; + } + /// Whether a press at `time` belongs to a touch that just stopped live motion. fn press_is_catch(&self, time: f64) -> bool { self.touch_caught_motion_at @@ -2652,6 +2691,7 @@ impl WidgetNode for PortalList { impl Widget for PortalList { fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) { let uid = self.widget_uid(); + self.cancel_drag_without_capture(cx); // Selection autoscroll is driven by next-frame ticks. If pointer-up happens outside // hit testing, selection can get "stuck" and keep scheduling frames. Clear it on @@ -2866,6 +2906,8 @@ impl Widget for PortalList { } self.event_item_ids = item_ids; } + // Capture may have moved to a child during the dispatch above. + self.cancel_drag_without_capture(cx); // Handle auto-scroll during selection if let Some(mut scroll_state) = self.select_scroll_state.take() {