diff --git a/crates/app/src/ui/canvas/readout.rs b/crates/app/src/ui/canvas/readout.rs index 5de4750..594c587 100644 --- a/crates/app/src/ui/canvas/readout.rs +++ b/crates/app/src/ui/canvas/readout.rs @@ -6,12 +6,13 @@ //! keys move is visible where the keys are pressed. //! //! It is a label and nothing more: it reads a cached value through -//! [`PlotxApp::contour_base_readout`] and never resolves, measures or queues +//! [`PlotxApp::property_readout`] and never resolves, measures or queues //! anything. A plot whose estimate has not arrived says so. use super::{object_screen_rect, plot_rect}; use crate::ui::properties; use egui::{Align2, Color32, FontId}; +use plotx_core::properties::PropertyReadout; use plotx_core::state::PlotxApp; const READOUT_FONT_PT: f32 = 11.0; @@ -42,13 +43,13 @@ pub(crate) fn paint_property_readouts( // level while the keys moved the contour that does — or, with several // contours, present one of them as the whole plot's threshold. let targets = app.series_targets(ci, object); - let readouts: Vec<_> = app + let readouts: Vec = app .resolve_property_set(property, &targets) .applicable_targets .iter() - .filter_map(|address| app.contour_base_readout(&address.target)) + .filter_map(|address| app.property_readout(address).ok()) .collect(); - let Some(text) = properties::readout::aggregate_summary(&readouts) else { + let Some(text) = properties::readout::aggregate_property_summary(&readouts) else { continue; }; let Some(frame) = object_screen_rect(app.session.board, canvas, object, rect) else { diff --git a/crates/app/src/ui/command_palette.rs b/crates/app/src/ui/command_palette.rs index be54d0e..39eea51 100644 --- a/crates/app/src/ui/command_palette.rs +++ b/crates/app/src/ui/command_palette.rs @@ -12,7 +12,7 @@ use super::properties::{self, PanelRoute}; use super::*; use egui::{Align2, FontId, Key, TextEdit, vec2}; use plotx_core::properties::PropertyId; -use plotx_core::state::{ObjectId, PropertyFocus}; +use plotx_core::state::{ObjectId, PropertyFocus, ToolGroup}; const PANEL_WIDTH: f32 = 540.0; const LIST_HEIGHT: f32 = 320.0; @@ -175,11 +175,8 @@ pub(super) fn search_set(app: &PlotxApp) -> Vec { .map(PaletteItem::from_command) .collect(); - // Resolved once for the whole property half of the set: applicability is a - // question about the current selection, and every hit asks it of the same - // selection the panel and the other discovery channels read. - let targets = properties::discovery::selection_targets(app); for hit in properties::property_hits() { + let targets = properties::discovery::targets_for_property(app, hit.id); let unavailable = property_unavailable_reason(app, hit.id, &targets); items.push(PaletteItem { haystack: hit.terms.join(" "), @@ -254,11 +251,22 @@ fn property_unavailable_reason( ) -> Option { let resolved = app.resolve_property_set(property, targets); if !resolved.applicable_targets.is_empty() { + // The setting applies. Whether it can be *changed* is a separate + // question with its own answer: a locked plot is still described, still + // read out on the canvas, and still findable by name here. + let editable = properties::discovery::editable_targets_for_property(app, property); + if app + .resolve_property_set(property, &editable) + .applicable_targets + .is_empty() + { + return Some(properties::discovery::LOCKED_REASON.to_owned()); + } return None; } // Nothing was even a candidate: the group already declares the sentence // that names the fix, and the catalog has nothing more specific to add. - let Some((_, reason)) = resolved.skipped_targets.first() else { + let Some(skip) = resolved.skipped_targets.first() else { return Some( properties::presentation(property) .and_then(|entry| properties::discovery::group(entry.home_route.section)) @@ -267,7 +275,10 @@ fn property_unavailable_reason( .to_owned(), ); }; - Some(format!("Select a series this setting applies to: {reason}")) + Some(format!( + "Select a series this setting applies to: {}", + skip.message + )) } fn activate( @@ -308,10 +319,50 @@ pub(super) fn reveal_property(app: &mut PlotxApp, property: PropertyId, now: f64 } match route.panel { PanelRoute::SecondarySidebar => app.session.secondary_sidebar_visible = true, + PanelRoute::Processing => { + app.session.secondary_sidebar_visible = true; + app.session.ui.requested_tool_group = Some(ToolGroup::Processing); + } + } + // A property owned by an owner-local component needs that component opened, + // or the row it names is not on screen to scroll to. The step is chosen here + // — once, from the user's activation — rather than recomputed every frame + // while the panel renders. + if route.panel == PanelRoute::Processing + && let Some(step) = revealed_step(app, property) + { + app.session.ui.proc_expanded_step = Some(step); } app.session.ui.property_focus = Some(PropertyFocus::request(property, now)); } +/// The first processing step that actually carries `property`, addressed the way +/// the catalog addresses it. Applicability is the catalog's answer, so a step +/// whose current settings do not expose the property is passed over rather than +/// opened onto a row that is not there. +fn revealed_step( + app: &PlotxApp, + property: PropertyId, +) -> Option<(plotx_core::state::DatasetId, plotx_processing::StepId)> { + use plotx_core::automation::ComponentRef; + use plotx_core::properties::PropertyAddress; + + properties::discovery::targets_for_property(app, property) + .into_iter() + .find(|target| { + app.resolve_property(&PropertyAddress::new(target.clone(), property)) + .is_ok() + }) + .and_then(|target| match target.component { + Some(ComponentRef::ProcessingStep(step)) => { + plotx_core::state::DatasetId::try_from(&target.resource) + .ok() + .map(|dataset| (dataset, step)) + } + _ => None, + }) +} + fn filter(items: &[PaletteItem], query: &str) -> Vec { let terms: Vec = query.split_whitespace().map(str::to_lowercase).collect(); items diff --git a/crates/app/src/ui/command_palette_tests.rs b/crates/app/src/ui/command_palette_tests.rs index 9836b24..9e2b33f 100644 --- a/crates/app/src/ui/command_palette_tests.rs +++ b/crates/app/src/ui/command_palette_tests.rs @@ -217,3 +217,70 @@ fn empty_state_is_constructible() { let state = plotx_core::state::CommandPaletteState::default(); assert!(state.query.is_empty()); } + +/// The palette disables a setting it cannot reveal, and the reason has to name +/// the state that blocks it. A locked plot used to disappear from the selection +/// entirely, so the palette reported "Select a plot whose series draws +/// contours…" about a contour plot the user had selected. +#[test] +fn a_locked_selection_disables_a_setting_with_the_reason_that_actually_applies() { + use crate::ui::properties::fixture; + + let (mut app, ids) = fixture::contour_page(1); + let targets = properties::discovery::targets_for_property(&app, contour::BASE_MAGNITUDE); + assert!(property_unavailable_reason(&app, contour::BASE_MAGNITUDE, &targets).is_none()); + + if let Some(object) = app.doc.canvases[0].object_mut(ids[0]) { + object.locked = true; + } + let targets = properties::discovery::targets_for_property(&app, contour::BASE_MAGNITUDE); + let reason = property_unavailable_reason(&app, contour::BASE_MAGNITUDE, &targets) + .expect("a locked plot cannot receive the write"); + assert_eq!(reason, properties::discovery::LOCKED_REASON); + assert!( + !reason.contains("draws contours"), + "the plot does draw contours; saying otherwise sends the user to fix the wrong thing" + ); +} + +/// Revealing a setting that lives on a processing step has to open that step, +/// and has to do it by moving the panel's own expansion state. +/// +/// The previous arrangement derived the expansion from the property focus while +/// rendering, so the focus's ~800 ms highlight timer collapsed the row again +/// with no user action behind it — the crate's layout-stability rule forbids +/// exactly that — and, because a focus names a property rather than a step, it +/// opened every step that could carry the setting at once. +#[test] +fn revealing_a_step_setting_opens_that_step_and_leaves_it_open() { + use plotx_core::properties::apodization; + + let mut app = PlotxApp::new_with_settings(plotx_core::settings::Settings::default()); + app.doc + .datasets + .push(crate::ui::properties::fixture::time_domain_2d()); + app.set_active_dataset(Some(0)); + + let expected = properties::discovery::targets_for_property(&app, apodization::KIND) + .into_iter() + .find_map(|target| match target.component { + Some(plotx_core::automation::ComponentRef::ProcessingStep(step)) => Some(step), + _ => None, + }) + .expect("the time-domain factory recipe has an apodization step"); + + reveal_property(&mut app, apodization::KIND, 10.0); + assert_eq!( + app.session.ui.proc_expanded_step.map(|(_, step)| step), + Some(expected), + "the reveal opens the step that carries the setting" + ); + + // The highlight expires on a timer. The expansion may not follow it. + app.session.ui.property_focus = None; + assert_eq!( + app.session.ui.proc_expanded_step.map(|(_, step)| step), + Some(expected), + "a timer must never collapse a row the user asked to see" + ); +} diff --git a/crates/app/src/ui/figure_typography.rs b/crates/app/src/ui/figure_typography.rs index 0fc9d5e..5af29dc 100644 --- a/crates/app/src/ui/figure_typography.rs +++ b/crates/app/src/ui/figure_typography.rs @@ -4,6 +4,7 @@ //! same contract as the canvas-size fields. use super::*; +use plotx_core::properties::FloatBounds; use plotx_figure::FigureTypography; pub(super) fn figure_typography_window(app: &mut PlotxApp, ctx: &egui::Context) { @@ -29,11 +30,23 @@ pub(super) fn figure_typography_window(app: &mut PlotxApp, ctx: &egui::Context) .num_columns(2) .spacing([12.0, 6.0]) .show(ui, |ui| { - size_row(app, ui, "Tick labels", |t| &mut t.tick_pt); + // The tick size is a catalog property, so its range comes + // from the definition the catalog control and the write path + // are both built from. A literal here would be a second copy + // of the rule, and it was: this window clamped to 24 pt while + // the catalog admitted 72, so any interaction here silently + // pulled a 40 pt figure back down. + size_row(app, ui, "Tick labels", tick_bounds(), |t| &mut t.tick_pt); ui.end_row(); - size_row(app, ui, "Axis titles", |t| &mut t.label_pt); + // The other two are not catalog properties yet and keep the + // range this window has always applied to them. + size_row(app, ui, "Axis titles", UNREGISTERED_BOUNDS, |t| { + &mut t.label_pt + }); ui.end_row(); - size_row(app, ui, "Figure title", |t| &mut t.title_pt); + size_row(app, ui, "Figure title", UNREGISTERED_BOUNDS, |t| { + &mut t.title_pt + }); ui.end_row(); }); ui.add_space(8.0); @@ -54,10 +67,21 @@ pub(super) fn figure_typography_window(app: &mut PlotxApp, ctx: &egui::Context) /// One labelled pt-size drag. Live-applies while dragging and commits a single /// undoable action per gesture (or per typed edit), mirroring /// `handle_canvas_dimension_response`. +/// The range this window applies to the two sizes that have no catalog entry. +const UNREGISTERED_BOUNDS: FloatBounds = FloatBounds::inclusive(4.0, 24.0); + +/// The declared range of the tick-label size, read from its definition. +fn tick_bounds() -> FloatBounds { + plotx_core::properties::definition(plotx_core::properties::typography::TICK_PT) + .and_then(|definition| definition.value_schema.float_bounds()) + .unwrap_or(UNREGISTERED_BOUNDS) +} + fn size_row( app: &mut PlotxApp, ui: &mut Ui, label: &str, + bounds: FloatBounds, field: impl Fn(&mut FigureTypography) -> &mut f32, ) { ui.label(label); @@ -69,7 +93,7 @@ fn size_row( let resp = ui.add( egui::DragValue::new(&mut value) .speed(0.25) - .range(4.0..=24.0) + .range(bounds.lowest()..=bounds.max) .max_decimals(1) .suffix(" pt"), ); @@ -96,3 +120,26 @@ fn size_row( app.execute_action(Action::set_figure_typography(frame_before, after)); } } + +#[cfg(test)] +mod tests { + use super::*; + + /// One definition of the range. The window used to stop at 24 pt while the + /// catalog admitted 72, so a size set through the catalog was silently + /// clamped the next time this window was touched. + #[test] + fn the_tick_row_takes_its_range_from_the_catalog() { + let declared = + plotx_core::properties::definition(plotx_core::properties::typography::TICK_PT) + .and_then(|definition| definition.value_schema.float_bounds()) + .expect("the tick-label size is a registered float property"); + let row = tick_bounds(); + assert_eq!(row.max, declared.max); + assert_eq!(row.lowest(), declared.lowest()); + assert!( + row.admits(40.0), + "a size the catalog accepts must survive a visit to this window" + ); + } +} diff --git a/crates/app/src/ui/object_inspector.rs b/crates/app/src/ui/object_inspector.rs index 0fe8f84..082c0e7 100644 --- a/crates/app/src/ui/object_inspector.rs +++ b/crates/app/src/ui/object_inspector.rs @@ -31,9 +31,11 @@ pub(crate) fn render(app: &mut PlotxApp, ui: &mut Ui) { }); commit_if_target_changed(app, axis_target); let Some(ci) = app.session.active_canvas else { + crate::ui::properties::panel::typography_section(app, ui); return; }; if ci >= app.doc.canvases.len() { + crate::ui::properties::panel::typography_section(app, ui); return; } if ids.is_empty() { @@ -107,9 +109,18 @@ pub(crate) fn render(app: &mut PlotxApp, ui: &mut Ui) { /// cross-target `Mixed` aggregate out of reach of the interface entirely. The /// section renders nothing at all when no resolved series has an applicable /// encoding. +/// The typography section is deliberately unconditional: it is a *document* +/// property, so it applies whenever a document is open, whatever is selected. +/// Gating it on the selection would hide an always-applicable control for a +/// transient reason, which the crate's hide-vs-disable rule forbids. fn property_sections(app: &mut PlotxApp, ci: usize, ui: &mut Ui) { - let objects = crate::ui::properties::discovery::selection_objects(app); + // The write side of the shared selection: these sections carry controls, so + // they take the editable subset. The lock lives in one place rather than + // being re-derived here. + let objects = crate::ui::properties::discovery::editable_objects(app); crate::ui::properties::panel::contour_section(app, ci, &objects, ui); + crate::ui::properties::panel::line_section(app, ci, &objects, ui); + crate::ui::properties::panel::typography_section(app, ui); } fn geometry_section(app: &mut PlotxApp, ci: usize, ids: &[ObjectId], ui: &mut Ui) { @@ -768,4 +779,19 @@ mod tests { let ctx = egui::Context::default(); let _ = ctx.run_ui(egui::RawInput::default(), |ui| render(&mut app, ui)); } + + #[test] + fn property_target_filter_excludes_locked_plot_objects() { + let (mut app, ids) = crate::ui::properties::fixture::contour_page(1); + let object = ids[0]; + app.doc.canvases[0] + .object_mut(object) + .expect("the fixture plot exists") + .locked = true; + let targets = kind_targets(&app, 0, &ids, |candidate| candidate.plot().is_some()); + assert!( + targets.is_empty(), + "a locked plot must be excluded before catalog targets are built" + ); + } } diff --git a/crates/app/src/ui/properties/discovery.rs b/crates/app/src/ui/properties/discovery.rs index d44ea1b..fba3880 100644 --- a/crates/app/src/ui/properties/discovery.rs +++ b/crates/app/src/ui/properties/discovery.rs @@ -15,8 +15,10 @@ use super::{PRESENTATIONS, PropertyGroup, PropertyPresentation}; use egui::Ui; -use plotx_core::automation::TargetRef; -use plotx_core::properties::{PropertyId, PropertyStep, Tier}; +use plotx_core::automation::{ResourceRef, TargetRef}; +use plotx_core::properties::{ + ComponentKind, PropertyId, PropertyStep, ScopeKind, Tier, definition, +}; use plotx_core::state::{ObjectId, PlotxApp}; /// The declared groups, in Ribbon order. @@ -70,12 +72,21 @@ pub(crate) fn steppable_in( presentations.iter().find(|entry| entry.canvas_step) } -/// The plot objects the discovery channels act on: the current selection, or +/// What to say when the only plots a setting applies to are locked. It names +/// the state and the way out, per the crate's hide-vs-disable rule. +pub(crate) const LOCKED_REASON: &str = + "Unlock this plot to change its settings; it can still be read while locked."; + +/// The plot objects the discovery channels *refer* to: the current selection, or /// the page's active plot when nothing is selected. /// /// Every channel shares this, so the Ribbon button, the context menu, the /// gesture and the on-canvas readout can never disagree about what they refer -/// to. +/// to. A lock is deliberately not applied here. Locking a plot means "do not +/// change this", not "stop telling me about this": the corner readout, the +/// palette's applicability answer and the context menu all describe a plot +/// rather than edit it, and filtering here made them vanish with no explanation +/// on a plot that was plainly selected. pub(crate) fn selection_objects(app: &PlotxApp) -> Vec { let Some(canvas) = app .session @@ -84,7 +95,7 @@ pub(crate) fn selection_objects(app: &PlotxApp) -> Vec { else { return Vec::new(); }; - let selected: Vec = app + let selected: Vec<_> = app .session .ui .selection @@ -103,26 +114,90 @@ pub(crate) fn selection_objects(app: &PlotxApp) -> Vec { canvas.active_plot_object_id().into_iter().collect() } +/// The subset of [`selection_objects`] a write may land on. This is the one +/// place the lock is applied, so no editing path can forget it and no reading +/// path can accidentally inherit it. +pub(crate) fn editable_objects(app: &PlotxApp) -> Vec { + let Some(canvas) = app + .session + .active_canvas + .and_then(|index| app.doc.canvases.get(index)) + else { + return Vec::new(); + }; + selection_objects(app) + .into_iter() + .filter(|&id| canvas.object(id).is_some_and(|object| !object.locked)) + .collect() +} + /// Every series target of [`selection_objects`], in binding order. pub(crate) fn selection_targets(app: &PlotxApp) -> Vec { + targets_of(app, selection_objects(app)) +} + +/// Every series target a write may land on. +pub(crate) fn editable_targets(app: &PlotxApp) -> Vec { + targets_of(app, editable_objects(app)) +} + +/// The subset of [`targets_for_property`] a write may land on. Only object-owned +/// properties can be locked out; a document or dataset setting has no plot to +/// lock, so it passes through unchanged. +pub(crate) fn editable_targets_for_property( + app: &PlotxApp, + property: PropertyId, +) -> Vec { + match definition(property).map(|definition| definition.applicability.component) { + Some(ComponentKind::Series) => editable_targets(app), + _ => targets_for_property(app, property), + } +} + +fn targets_of(app: &PlotxApp, objects: Vec) -> Vec { let Some(canvas) = app.session.active_canvas else { return Vec::new(); }; - selection_objects(app) + objects .into_iter() .flat_map(|object| app.series_targets(canvas, object)) .collect() } +/// Derive the targets for one property from its catalog shape. This is target +/// discovery only: providers still decide how a target maps to typed storage. +/// Keeping this here lets document, object and processing-step presentations +/// share the same search and Ribbon applicability checks without inventing a +/// separate registry for each scope. +pub(crate) fn targets_for_property(app: &PlotxApp, property: PropertyId) -> Vec { + let Some(definition) = definition(property) else { + return Vec::new(); + }; + match definition.applicability.component { + ComponentKind::None if definition.scope_kind == ScopeKind::Document => { + vec![app.document_target()] + } + ComponentKind::None => Vec::new(), + ComponentKind::Series => selection_targets(app), + ComponentKind::ProcessingStep => { + let Some(dataset) = app + .active_dataset() + .and_then(|index| app.doc.datasets.get(index)) + else { + return Vec::new(); + }; + let resource = ResourceRef::from(dataset.resource_id()); + app.resource_property_targets(&resource, definition) + } + } +} + /// Whether any member of a group currently applies to the selection. This is /// the group's own applicability, derived from its members' definitions — /// capability and encoding gates included — not a second rule written here. pub(crate) fn group_applies(app: &PlotxApp, section: &str) -> bool { - let targets = selection_targets(app); - if targets.is_empty() { - return false; - } members_of(section, PRESENTATIONS).into_iter().any(|entry| { + let targets = targets_for_property(app, entry.id); !app.resolve_property_set(entry.id, &targets) .applicable_targets .is_empty() @@ -145,9 +220,21 @@ pub(crate) fn step_target(app: &PlotxApp) -> Option<(PropertyId, Vec) /// the catalog decides what a step is, validates it and compiles it into the /// same atomic action the panel produces, so the two entry points cannot drift. pub(crate) fn step_selection(app: &mut PlotxApp, step: PropertyStep) { - let Some((property, targets)) = step_target(app) else { + let Some((property, _)) = step_target(app) else { return; }; + // The readout above the gesture describes every selected plot; the gesture + // itself may only move the ones that are not locked, and says so when that + // leaves it nothing to move. + let targets = editable_targets_for_property(app, property); + if app + .resolve_property_set(property, &targets) + .applicable_targets + .is_empty() + { + app.session.status = LOCKED_REASON.to_owned(); + return; + } match app.plan_property_step(property, &targets, step) { Ok(commit) => { let skipped = commit.skipped.len(); diff --git a/crates/app/src/ui/properties/discovery_tests.rs b/crates/app/src/ui/properties/discovery_tests.rs index 0a19b3e..f700340 100644 --- a/crates/app/src/ui/properties/discovery_tests.rs +++ b/crates/app/src/ui/properties/discovery_tests.rs @@ -87,11 +87,11 @@ fn one_registration_joins_its_group_without_a_second_entry() { ); assert_eq!( members.len(), - PRESENTATIONS.len() + 1, + discovery::members_of(panel::CONTOUR_SECTION, PRESENTATIONS).len() + 1, "membership is derived, so it grows with the table and nothing else" ); // The group table itself is untouched: the newcomer contributed no entry. - assert_eq!(GROUPS.len(), 1); + assert_eq!(GROUPS.len(), 4); } /// Channel 3: the gesture picks up whichever property declared itself @@ -197,3 +197,45 @@ fn the_gesture_is_a_catalog_command_with_a_reason_when_it_cannot_run() { assert!(!descriptor.label.is_empty()); } } + +/// Locking a plot means "do not change this", not "stop telling me about this". +/// The read-only channels — the canvas corner readout, the palette's +/// applicability answer, the context menu — all resolve through +/// `selection_objects`, and filtering the lock in there made them vanish from a +/// plot that was plainly selected, with nothing on screen to explain it. +#[test] +fn a_locked_plot_is_still_read_and_only_refuses_the_write() { + use crate::ui::properties::fixture; + + let (mut app, ids) = fixture::contour_page(2); + assert_eq!(discovery::selection_objects(&app).len(), 2); + assert_eq!(discovery::editable_objects(&app).len(), 2); + + if let Some(object) = app.doc.canvases[0].object_mut(ids[0]) { + object.locked = true; + } + assert_eq!( + discovery::selection_objects(&app).len(), + 2, + "a locked plot stays in the set every read-only channel resolves" + ); + assert_eq!(discovery::editable_objects(&app), vec![ids[1]]); + assert!( + !discovery::selection_targets(&app).is_empty(), + "the readout still has a series to describe" + ); + + if let Some(object) = app.doc.canvases[0].object_mut(ids[1]) { + object.locked = true; + } + assert_eq!(discovery::selection_objects(&app).len(), 2); + assert!(discovery::editable_objects(&app).is_empty()); + assert!( + !discovery::selection_targets(&app).is_empty(), + "every plot being locked silences the write, never the reading" + ); + + // And the gesture says why rather than doing nothing. + discovery::step_selection(&mut app, PropertyStep::Raise); + assert_eq!(app.session.status, discovery::LOCKED_REASON); +} diff --git a/crates/app/src/ui/properties/fixture.rs b/crates/app/src/ui/properties/fixture.rs index b8032bb..5920fad 100644 --- a/crates/app/src/ui/properties/fixture.rs +++ b/crates/app/src/ui/properties/fixture.rs @@ -129,3 +129,11 @@ pub(crate) fn set_lowest_level(app: &mut PlotxApp, object: ObjectId, multiplier: .expect("the fixture writes a valid multiplier"); app.commit_property(commit); } + +/// A time-domain 2D acquisition, whose factory recipe carries the apodization +/// step the processing-panel tests address. +pub(crate) fn time_domain_2d() -> Dataset { + let mut data = nmr2d("time domain"); + data.domain = plotx_io::Domain::Time; + Dataset::Nmr2D(Box::new(Nmr2DDataset::load(data))) +} diff --git a/crates/app/src/ui/properties/mod.rs b/crates/app/src/ui/properties/mod.rs index d6d860a..f0f7e36 100644 --- a/crates/app/src/ui/properties/mod.rs +++ b/crates/app/src/ui/properties/mod.rs @@ -17,7 +17,9 @@ pub(crate) mod fixture; pub(crate) use search::property_hits; -use plotx_core::properties::{PropertyDefinition, PropertyId, Tier, contour, definition}; +use plotx_core::properties::{ + PropertyDefinition, PropertyId, Tier, apodization, contour, definition, line, typography, +}; use plotx_core::state::WorkflowTab; /// A user-facing string in the active locale. PlotX ships one locale today; the @@ -36,6 +38,7 @@ impl LocalizedText { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum PanelRoute { SecondarySidebar, + Processing, } impl PanelRoute { @@ -44,13 +47,19 @@ impl PanelRoute { /// test checks. pub const fn sections(self) -> &'static [&'static str] { match self { - Self::SecondarySidebar => &[panel::CONTOUR_SECTION], + Self::SecondarySidebar => &[ + panel::CONTOUR_SECTION, + panel::LINE_SECTION, + panel::TYPOGRAPHY_SECTION, + ], + Self::Processing => &[panel::APODIZATION_SECTION], } } pub const fn title(self) -> &'static str { match self { Self::SecondarySidebar => "Object inspector", + Self::Processing => "Processing tools", } } } @@ -106,17 +115,52 @@ pub struct PropertyGroup { pub unavailable_reason: &'static str, } -pub const GROUPS: &[PropertyGroup] = &[PropertyGroup { - section: panel::CONTOUR_SECTION, - label: LocalizedText("Contour"), - icon: egui_phosphor::regular::CHART_POLAR, - ribbon: RibbonSpot { - tab: WorkflowTab::Figure, - group: "Style", - priority: 2, +pub const GROUPS: &[PropertyGroup] = &[ + PropertyGroup { + section: panel::CONTOUR_SECTION, + label: LocalizedText("Contour"), + icon: egui_phosphor::regular::CHART_POLAR, + ribbon: RibbonSpot { + tab: WorkflowTab::Figure, + group: "Style", + priority: 2, + }, + unavailable_reason: "Select a plot whose series draws contours before changing contour levels.", + }, + PropertyGroup { + section: panel::LINE_SECTION, + label: LocalizedText("Line"), + icon: egui_phosphor::regular::LINE_SEGMENT, + ribbon: RibbonSpot { + tab: WorkflowTab::Figure, + group: "Style", + priority: 3, + }, + unavailable_reason: "Select a plot whose series draws lines before changing line style.", + }, + PropertyGroup { + section: panel::TYPOGRAPHY_SECTION, + label: LocalizedText("Figure typography"), + icon: egui_phosphor::regular::TEXT_T, + ribbon: RibbonSpot { + tab: WorkflowTab::Figure, + group: "Style", + priority: 3, + }, + unavailable_reason: "Open a PlotX document before changing figure typography.", }, - unavailable_reason: "Select a plot whose series draws contours before changing contour levels.", -}]; + PropertyGroup { + section: panel::APODIZATION_SECTION, + label: LocalizedText("Apodization"), + icon: egui_phosphor::regular::WAVEFORM, + ribbon: RibbonSpot { + tab: WorkflowTab::Process, + group: "Processing", + priority: 1, + }, + unavailable_reason: "Select a dataset with an apodization processing step.", + }, +]; impl PropertyPresentation { /// The tier lives on the definition; presentation reads it so the panel @@ -135,6 +179,21 @@ const CONTOUR_HOME: HomeRoute = HomeRoute { section: panel::CONTOUR_SECTION, }; +const LINE_HOME: HomeRoute = HomeRoute { + panel: PanelRoute::SecondarySidebar, + section: panel::LINE_SECTION, +}; + +const TYPOGRAPHY_HOME: HomeRoute = HomeRoute { + panel: PanelRoute::SecondarySidebar, + section: panel::TYPOGRAPHY_SECTION, +}; + +const APODIZATION_HOME: HomeRoute = HomeRoute { + panel: PanelRoute::Processing, + section: panel::APODIZATION_SECTION, +}; + pub const PRESENTATIONS: &[PropertyPresentation] = &[ PropertyPresentation { id: contour::BASE_MAGNITUDE, @@ -201,6 +260,41 @@ pub const PRESENTATIONS: &[PropertyPresentation] = &[ home_route: CONTOUR_HOME, canvas_step: false, }, + PropertyPresentation { + id: line::STROKE_WIDTH, + localized_label: LocalizedText("Stroke width"), + localized_aliases: &[LocalizedText("line thickness")], + home_route: LINE_HOME, + canvas_step: false, + }, + PropertyPresentation { + id: typography::TICK_PT, + localized_label: LocalizedText("Tick-label size"), + localized_aliases: &[LocalizedText("figure font size")], + home_route: TYPOGRAPHY_HOME, + canvas_step: false, + }, + PropertyPresentation { + id: apodization::KIND, + localized_label: LocalizedText("Window"), + localized_aliases: &[LocalizedText("apodization window")], + home_route: APODIZATION_HOME, + canvas_step: false, + }, + PropertyPresentation { + id: apodization::LB_HZ, + localized_label: LocalizedText("LB"), + localized_aliases: &[LocalizedText("line broadening")], + home_route: APODIZATION_HOME, + canvas_step: false, + }, + PropertyPresentation { + id: apodization::GB_HZ, + localized_label: LocalizedText("GB"), + localized_aliases: &[LocalizedText("gaussian broadening")], + home_route: APODIZATION_HOME, + canvas_step: false, + }, ]; pub fn presentation(id: PropertyId) -> Option<&'static PropertyPresentation> { diff --git a/crates/app/src/ui/properties/panel.rs b/crates/app/src/ui/properties/panel.rs index 5077636..3d5fb4b 100644 --- a/crates/app/src/ui/properties/panel.rs +++ b/crates/app/src/ui/properties/panel.rs @@ -12,14 +12,20 @@ use egui_phosphor::regular as icon; use plotx_core::automation::TargetRef; use plotx_core::properties::{ AggregateValue, ContourBaseReadout, EncodingKind, PropertyDefinition, PropertyId, - PropertyValue, ResolvedProperty, ResolvedPropertySet, ResolvedSchema, ValueCopies, ValueSchema, - contour, + PropertyReadout, PropertyValue, ResolvedProperty, ResolvedPropertySet, ResolvedSchema, + ValueCopies, ValueSchema, contour, }; use plotx_core::state::{ObjectId, PlotxApp, PropertyFocus}; /// The home-route section id of the contour rows. The route table and the /// collapsing header below must agree on it, so both read this constant. pub(crate) const CONTOUR_SECTION: &str = "object.contour"; +/// The home section for line-encoding rows on selected plot objects. +pub(crate) const LINE_SECTION: &str = "object.line"; +/// The document root's figure typography rows. +pub(crate) const TYPOGRAPHY_SECTION: &str = "document.figure_typography"; +/// The processing editor's per-step apodization rows. +pub(crate) const APODIZATION_SECTION: &str = "dataset.apodization"; /// What a control shows in place of a number or a choice when there is none: /// the sources behind the row do not agree, so no value may be presented as the @@ -48,6 +54,34 @@ fn no_single_value_hint(targets: usize, copies: ValueCopies) -> String { format!("No single value: {sources}. Setting it now applies to all of them.") } +/// What a section counts itself in, in both grammatical numbers. +/// +/// English does not pluralize by appending an `s` — "2 contour seriess" is what +/// that produces — so both forms are declared rather than derived. +#[derive(Clone, Copy)] +struct SectionNoun { + singular: &'static str, + plural: &'static str, +} + +impl SectionNoun { + const fn new(singular: &'static str, plural: &'static str) -> Self { + Self { singular, plural } + } + + fn of(self, count: usize) -> &'static str { + if count == 1 { + self.singular + } else { + self.plural + } + } + + fn counted(self, count: usize) -> String { + format!("{count} {}", self.of(count)) + } +} + /// One catalog row, already resolved against the selection. struct Row { presentation: &'static PropertyPresentation, @@ -90,7 +124,19 @@ impl Row { enum Pending { Write(PropertyId, PropertyValue), Reset(PropertyId), - ResetEncoding, + ResetEncoding(EncodingKind), +} + +/// What a continuous control did to the gesture it belongs to this frame. +/// +/// A drag writes every frame; only the release ends the gesture. The control +/// reports the transition and the section acts on it once the immutable borrows +/// are done, so every catalog row coalesces the same way regardless of which +/// typed store it happens to write. +#[derive(Clone, Copy, PartialEq)] +enum GestureEdge { + Started, + Stopped, } /// Render the contour section for the current selection. Returns `false` when @@ -106,10 +152,82 @@ pub(crate) fn contour_section( .iter() .flat_map(|&object| app.series_targets(canvas, object)) .collect(); + render_section( + app, + CONTOUR_SECTION, + "Contour", + SectionNoun::new("contour series", "contour series"), + &targets, + Some(EncodingKind::Contour), + ui, + ) +} + +/// Render line properties over the current plot selection. +pub(crate) fn line_section( + app: &mut PlotxApp, + canvas: usize, + objects: &[ObjectId], + ui: &mut Ui, +) -> bool { + let targets: Vec = objects + .iter() + .flat_map(|&object| app.series_targets(canvas, object)) + .collect(); + render_section( + app, + LINE_SECTION, + "Line", + SectionNoun::new("line series", "line series"), + &targets, + None, + ui, + ) +} + +/// Render document-owned typography without requiring any canvas object. +pub(crate) fn typography_section(app: &mut PlotxApp, ui: &mut Ui) -> bool { + let target = app.document_target(); + render_section( + app, + TYPOGRAPHY_SECTION, + "Figure typography", + SectionNoun::new("document", "documents"), + std::slice::from_ref(&target), + None, + ui, + ) +} + +/// Render the catalog rows for one expanded apodization step in the existing +/// processing editor. The step list supplies the stable component target; this +/// panel supplies the same schema, reset and action path as every other scope. +pub(crate) fn apodization_section(app: &mut PlotxApp, target: &TargetRef, ui: &mut Ui) -> bool { + render_section( + app, + APODIZATION_SECTION, + "Apodization", + SectionNoun::new("processing step", "processing steps"), + std::slice::from_ref(target), + None, + ui, + ) +} + +#[allow(clippy::too_many_arguments)] +fn render_section( + app: &mut PlotxApp, + section: &'static str, + title: &'static str, + status_noun: SectionNoun, + targets: &[TargetRef], + reset_encoding: Option, + ui: &mut Ui, +) -> bool { if targets.is_empty() { return false; } - let rows = resolve_rows(app, &targets); + let rows = resolve_rows_for(app, targets, section); if rows.is_empty() { return false; } @@ -119,29 +237,32 @@ pub(crate) fn contour_section( let focused_here = focus.is_some_and(|focus| rows.iter().any(|row| row.presentation.id == focus.property)); + // Every target in the selection is not what this section acts on: the + // heading counts the ones that actually supply one of its rows, so a page + // holding one contour plot and one line plot does not report two of each. + let applicable = applicable_targets(&rows); + ui.separator(); ui.horizontal(|ui| { - ui.strong("Contour"); + ui.strong(title); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { - ui.weak(match targets.len() { - 1 => "1 series".to_owned(), - count => format!("{count} series"), - }); + ui.weak(status_noun.counted(applicable.len())); }); }); // The rows rendered without expanding anything are exactly the list the // budget check counts, so the check cannot pass while the panel shows more. - let essential: Vec = super::essential_in(CONTOUR_SECTION) + let essential: Vec = super::essential_in(section) .into_iter() .map(|entry| entry.id) .collect(); let mut pending: Option = None; + let mut gesture: Option<(PropertyId, GestureEdge)> = None; for row in rows .iter() .filter(|row| essential.contains(&row.presentation.id)) { - property_row(row, focus, now, &mut pending, ui); + property_row(row, focus, now, &mut pending, &mut gesture, ui); } let advanced: Vec<&Row> = rows @@ -149,7 +270,7 @@ pub(crate) fn contour_section( .filter(|row| !essential.contains(&row.presentation.id)) .collect(); if !advanced.is_empty() { - let id = ui.make_persistent_id(("property_section", CONTOUR_SECTION)); + let id = ui.make_persistent_id(("property_section", section)); let mut state = egui::collapsing_header::CollapsingState::load_with_default_open(ui.ctx(), id, false); if focused_here @@ -163,20 +284,21 @@ pub(crate) fn contour_section( state.store(ui.ctx()); } egui::CollapsingHeader::new("Advanced") - .id_salt(("property_section", CONTOUR_SECTION)) + .id_salt(("property_section", section)) .show(ui, |ui| { for row in advanced { - property_row(row, focus, now, &mut pending, ui); + property_row(row, focus, now, &mut pending, &mut gesture, ui); } }); } - if ui - .small_button("Reset contour") - .on_hover_text("Rebuild this series' encoding from its defaults") - .clicked() + if let Some(encoding) = reset_encoding + && ui + .small_button("Reset contour") + .on_hover_text("Rebuild this series' encoding from its defaults") + .clicked() { - pending = Some(Pending::ResetEncoding); + pending = Some(Pending::ResetEncoding(encoding)); } // The reveal is one-shot: once the section has been drawn with the row in @@ -190,20 +312,35 @@ pub(crate) fn contour_section( app.session.ui.property_focus = None; } + // Opened before the write and closed after it, so the frame that starts a + // drag is already inside the gesture and the frame that ends one is the last + // it records. + if let Some((property, GestureEdge::Started)) = gesture { + app.begin_property_gesture(property); + } if let Some(pending) = pending { - apply(app, &targets, pending); + // Still the whole selection: a target this section cannot supply is + // reported as a skip rather than quietly left out of the write. + apply(app, targets, pending, status_noun); + } + if let Some((_, GestureEdge::Stopped)) = gesture { + app.end_property_gesture(); } true } +#[cfg(test)] fn resolve_rows(app: &PlotxApp, targets: &[TargetRef]) -> Vec { + resolve_rows_for(app, targets, CONTOUR_SECTION) +} + +fn resolve_rows_for(app: &PlotxApp, targets: &[TargetRef], section: &str) -> Vec { let mut rows = Vec::new(); for presentation in PRESENTATIONS { let Some(definition) = presentation.definition() else { continue; }; - if definition.applicability.encoding != Some(plotx_core::properties::EncodingKind::Contour) - { + if presentation.home_route.section != section { continue; } let set = app.resolve_property_set(presentation.id, targets); @@ -219,9 +356,15 @@ fn resolve_rows(app: &PlotxApp, targets: &[TargetRef]) -> Vec { // as the row's would pass one series' threshold off as the selection's, // which is the same misrepresentation the control itself refuses when it // blanks its number. - let readout = (presentation.id == contour::BASE_MAGNITUDE && set.value.uniform().is_some()) - .then(|| app.contour_base_readout(&first.target)) - .flatten(); + let readout = if presentation.id == contour::BASE_MAGNITUDE && set.value.uniform().is_some() + { + match app.property_readout(first) { + Ok(PropertyReadout::ContourBase(readout)) => Some(readout), + Ok(PropertyReadout::Value(_)) | Err(_) => None, + } + } else { + None + }; rows.push(Row { presentation, definition, @@ -238,6 +381,7 @@ fn property_row( focus: Option, now: f64, pending: &mut Option, + gesture: &mut Option<(PropertyId, GestureEdge)>, ui: &mut Ui, ) { let highlighted = focus @@ -247,7 +391,7 @@ fn property_row( ui.horizontal(|ui| { ui.label(row.presentation.localized_label.get()) .on_hover_text(row.definition.canonical_label); - control(row, pending, ui); + control(row, pending, gesture, ui); if row.modified() { modified_marker(row, pending, ui); } @@ -300,7 +444,12 @@ fn modified_marker(row: &Row, pending: &mut Option, ui: &mut Ui) { /// be enough to make the whole selection agree — but it displays nothing that /// could be read as the current setting: an em dash instead of a number or a /// choice, and no checkbox state at all. -fn control(row: &Row, pending: &mut Option, ui: &mut Ui) { +fn control( + row: &Row, + pending: &mut Option, + gesture: &mut Option<(PropertyId, GestureEdge)>, + ui: &mut Ui, +) { let mixed = row.mixed(); let Some(value) = row.editing_value() else { ui.weak("unavailable"); @@ -332,7 +481,9 @@ fn control(row: &Row, pending: &mut Option, ui: &mut Ui) { (ResolvedSchema::Int { min, max }, PropertyValue::Int(current)) => { let mut current = current; let drag = DragValue::new(&mut current).speed(0.25).range(*min..=*max); - if ui.add(hide_value(drag, mixed)).changed() { + let response = ui.add(hide_value(drag, mixed)); + note_gesture(row, &response, gesture); + if response.changed() { *pending = Some(Pending::Write( row.presentation.id, PropertyValue::Int(current), @@ -341,12 +492,16 @@ fn control(row: &Row, pending: &mut Option, ui: &mut Ui) { } (ResolvedSchema::Float { bounds, log, unit }, PropertyValue::Float(current)) => { let mut next = current; - // A logarithmic quantity spans many decades, so the drag step has - // to follow the value rather than the (unbounded) range. - let speed = if *log { - (current.abs() * 0.02).max(f64::MIN_POSITIVE) - } else { - ((bounds.max - bounds.min) / 200.0).max(1.0e-3) + // The definition's own notch wins. Deriving one from the range is + // the last resort, because a range states what is admissible, not + // what is usual: line broadening is legal out to +-10 kHz and is set + // in tenths of a hertz. + let speed = match declared_drag_step(row) { + Some(step) => step, + // A logarithmic quantity spans many decades, so the drag step + // has to follow the value rather than the (unbounded) range. + None if *log => (current.abs() * 0.02).max(f64::MIN_POSITIVE), + None => ((bounds.max - bounds.min) / 200.0).max(1.0e-3), }; // An egui range is inclusive, so an open bound is entered here by // asking the schema for the smallest value it admits rather than by @@ -354,7 +509,9 @@ fn control(row: &Row, pending: &mut Option, ui: &mut Ui) { let drag = DragValue::new(&mut next) .speed(speed) .range(bounds.lowest()..=bounds.max); - if ui.add(hide_value(drag, mixed)).changed() { + let response = ui.add(hide_value(drag, mixed)); + note_gesture(row, &response, gesture); + if response.changed() { *pending = Some(Pending::Write( row.presentation.id, PropertyValue::Float(next), @@ -424,6 +581,28 @@ fn control(row: &Row, pending: &mut Option, ui: &mut Ui) { } } +/// The drag notch this row's definition declares, if it declares one. +fn declared_drag_step(row: &Row) -> Option { + match row.definition.value_schema { + ValueSchema::Float { drag_step, .. } => drag_step, + _ => None, + } +} + +/// Report a continuous control's drag edges to the section that owns the +/// gesture. Only the edges: what happens in between is an ordinary write. +fn note_gesture( + row: &Row, + response: &egui::Response, + gesture: &mut Option<(PropertyId, GestureEdge)>, +) { + if response.drag_started() { + *gesture = Some((row.presentation.id, GestureEdge::Started)); + } else if response.drag_stopped() { + *gesture = Some((row.presentation.id, GestureEdge::Stopped)); + } +} + /// Blank a drag control's readout while leaving it draggable and typable. The /// number it still carries only decides where a drag starts; it is never shown. fn hide_value(drag: DragValue<'_>, hidden: bool) -> DragValue<'_> { @@ -454,14 +633,28 @@ fn describe(row: &Row, value: PropertyValue) -> String { } } -fn apply(app: &mut PlotxApp, targets: &[TargetRef], pending: Pending) { +/// The union of the targets this section's rows apply to, in selection order. +fn applicable_targets(rows: &[Row]) -> Vec { + let mut targets: Vec = Vec::new(); + for address in rows + .iter() + .flat_map(|row| row.set.applicable_targets.iter()) + { + if !targets.contains(&address.target) { + targets.push(address.target.clone()); + } + } + targets +} + +fn apply(app: &mut PlotxApp, targets: &[TargetRef], pending: Pending, status_noun: SectionNoun) { let planned = match pending { Pending::Write(property, value) => app.plan_property_write(property, targets, &value), Pending::Reset(property) => app.plan_property_reset(property, targets), // Scoped to the encoding this section is about: a plot that stacks a // contour over a heatmap must not have the heatmap rebuilt by a button // that names the contour. - Pending::ResetEncoding => app.plan_encoding_reset(EncodingKind::Contour, targets), + Pending::ResetEncoding(encoding) => app.plan_encoding_reset(encoding, targets), }; match planned { Ok(commit) => { @@ -470,17 +663,18 @@ fn apply(app: &mut PlotxApp, targets: &[TargetRef], pending: Pending) { // A skipped target is reported, never silently dropped: the user // asked for the whole selection and must learn what it did not do. app.session.status = if skipped.is_empty() { - format!("Updated {applied} contour series.") + format!("Updated {}.", status_noun.counted(applied)) } else { format!( - "Updated {applied} contour series; skipped {}: {}", + "Updated {}; skipped {}: {}", + status_noun.counted(applied), skipped.len(), - skipped[0].1 + skipped[0].message ) }; } Err(error) => { - app.session.status = format!("Could not change the contour: {error}"); + app.session.status = format!("Could not change {}: {error}", status_noun.plural); } } } diff --git a/crates/app/src/ui/properties/panel_tests.rs b/crates/app/src/ui/properties/panel_tests.rs index c4f87ce..889aaf1 100644 --- a/crates/app/src/ui/properties/panel_tests.rs +++ b/crates/app/src/ui/properties/panel_tests.rs @@ -141,3 +141,33 @@ fn a_uniform_row_still_carries_its_resolved_level() { .expect("a single agreeing series states what its multiple means"); assert_eq!(readout.magnitude, 5.0); } + +/// The heading is a count of what the section acts on, and it has to be a +/// sentence. Appending an `s` produced "2 contour seriess", and counting the +/// whole selection produced a two where only one plot draws a contour — both +/// are the heading claiming something the rows do not do. +#[test] +fn the_heading_counts_what_the_section_supplies_and_says_it_in_english() { + let (mut app, ids) = fixture::contour_page(2); + fixture::draw_as_heatmap(&mut app, ids[1]); + let targets: Vec = ids + .iter() + .flat_map(|&id| app.series_targets(0, id)) + .collect(); + assert_eq!(targets.len(), 2, "both plots are in the selection"); + + let rows = resolve_rows(&app, &targets); + let applicable = applicable_targets(&rows); + assert_eq!( + applicable.len(), + 1, + "only one of the two selected series draws a contour" + ); + + let series = SectionNoun::new("contour series", "contour series"); + assert_eq!(series.counted(applicable.len()), "1 contour series"); + assert_eq!(series.counted(2), "2 contour series"); + let document = SectionNoun::new("document", "documents"); + assert_eq!(document.counted(1), "1 document"); + assert_eq!(document.counted(2), "2 documents"); +} diff --git a/crates/app/src/ui/properties/readout.rs b/crates/app/src/ui/properties/readout.rs index 705a530..c7e47a4 100644 --- a/crates/app/src/ui/properties/readout.rs +++ b/crates/app/src/ui/properties/readout.rs @@ -6,7 +6,7 @@ //! unmeasured or degenerate anchor looks like — honestly, and without ever //! asking for the measurement. -use plotx_core::properties::{ContourAnchor, ContourBaseReadout}; +use plotx_core::properties::{ContourAnchor, ContourBaseReadout, PropertyReadout, PropertyValue}; use plotx_core::state::{ CONTOUR_BASE_ABSOLUTE, CONTOUR_BASE_BACKGROUND_SCALE, CONTOUR_BASE_FRACTION_OF_RANGE, CONTOUR_BASE_NOISE_FLOOR, @@ -93,6 +93,52 @@ pub(crate) fn aggregate_summary(readouts: &[ContourBaseReadout]) -> Option Option { + let first = readouts.first()?; + match first { + PropertyReadout::ContourBase(_) => { + let contours: Vec = readouts + .iter() + .filter_map(|readout| match readout { + PropertyReadout::ContourBase(readout) => Some(*readout), + PropertyReadout::Value(_) => None, + }) + .collect(); + if contours.len() != readouts.len() { + return Some(format!( + "{} series — no single property readout", + readouts.len() + )); + } + aggregate_summary(&contours) + } + PropertyReadout::Value(value) => { + if readouts.iter().all(|readout| readout == first) { + Some(value_summary(*value)) + } else { + Some(format!( + "{} series — no single property value", + readouts.len() + )) + } + } + } +} + +fn value_summary(value: PropertyValue) -> String { + match value { + PropertyValue::Bool(value) => value.to_string(), + PropertyValue::Int(value) => value.to_string(), + PropertyValue::Float(value) => number(value), + PropertyValue::Enum(value) => value.to_owned(), + PropertyValue::Color(color) => format!("#{:02x}{:02x}{:02x}", color.r, color.g, color.b), + } +} + /// Just the resolved half, for a row that already shows the number the control /// edits. `None` when the number is the level and there is nothing to add. pub(crate) fn resolution_suffix(readout: &ContourBaseReadout) -> Option { diff --git a/crates/app/src/ui/properties/tests.rs b/crates/app/src/ui/properties/tests.rs index 6b23017..879ef60 100644 --- a/crates/app/src/ui/properties/tests.rs +++ b/crates/app/src/ui/properties/tests.rs @@ -119,7 +119,10 @@ fn aliases_are_indexed_by_the_unified_search() { /// panel, and the remedy is to move rows to `Advanced`, not to raise the limit. #[test] fn no_panel_section_exceeds_its_essential_budget() { - for panel in [PanelRoute::SecondarySidebar] { + // Every route, not a hand-picked one: a section whose panel is missing here + // has no build-time budget at all, which is how the processing rows grew + // theirs unchecked. + for panel in [PanelRoute::SecondarySidebar, PanelRoute::Processing] { for section in panel.sections() { let essential = essential_in(section); assert!( diff --git a/crates/app/src/ui/tools/processing/editors.rs b/crates/app/src/ui/tools/processing/editors.rs index 0d9a3b0..234232f 100644 --- a/crates/app/src/ui/tools/processing/editors.rs +++ b/crates/app/src/ui/tools/processing/editors.rs @@ -4,6 +4,7 @@ use super::{commit_kind, edit_step, set_phase_method}; use egui::{DragValue, Ui}; use egui_phosphor::regular as icon; use plotx_core::actions::DatasetProcessingState; +use plotx_core::automation::{ComponentRef, ResourceRef, TargetRef}; use plotx_core::state::{Dataset, PhaseAxis, PlotxApp}; use plotx_processing::{ Apodization, AutoPhaseMethod, BaselineMethod, NormalizeMethod, PhaseParams, ProcessingStep, @@ -36,7 +37,16 @@ pub(super) fn editor( ui: &mut Ui, ) { match &step.kind { - StepKind::Apodize(a) => apodize_editor(app, di, axis, step.id, *a, ui), + StepKind::Apodize(_) => { + let Some(dataset) = app.doc.datasets.get(di) else { + return; + }; + let target = TargetRef { + resource: ResourceRef::from(dataset.resource_id()), + component: Some(ComponentRef::ProcessingStep(step.id)), + }; + crate::ui::properties::panel::apodization_section(app, &target, ui); + } StepKind::ZeroFill(z) => zero_fill_editor(app, di, axis, step.id, *z, ui), StepKind::Phase(p) => phase_editor(app, di, axis, step.id, *p, ui), StepKind::Baseline(m) => baseline_editor(app, di, axis, step.id, *m, ui), @@ -63,90 +73,6 @@ pub(super) fn editor( } } -fn apodize_editor( - app: &mut PlotxApp, - di: usize, - axis: PhaseAxis, - id: StepId, - cur: Apodization, - ui: &mut Ui, -) { - let mut kind = apo_variant(cur); - ui.horizontal(|ui| { - ui.label("Window"); - egui::ComboBox::from_id_salt((di, id, "apo")) - .selected_text(kind.label()) - .show_ui(ui, |ui| { - for v in ApoVariant::ALL { - ui.selectable_value(&mut kind, v, v.label()); - } - }); - }); - if kind != apo_variant(cur) { - commit_kind(app, di, axis, id, StepKind::Apodize(kind.apodization(cur))); - return; - } - - match cur { - Apodization::Exponential { lb_hz } => { - param_drag( - app, - di, - axis, - id, - ui, - "LB (Hz)", - lb_hz, - 0.5, - true, - true, - |k, v| { - if let StepKind::Apodize(Apodization::Exponential { lb_hz }) = k { - *lb_hz = v; - } - }, - ); - } - Apodization::Gaussian { lb_hz, gb_hz } => { - param_drag( - app, - di, - axis, - id, - ui, - "LB (Hz)", - lb_hz, - 0.5, - true, - true, - |k, v| { - if let StepKind::Apodize(Apodization::Gaussian { lb_hz, .. }) = k { - *lb_hz = v; - } - }, - ); - param_drag( - app, - di, - axis, - id, - ui, - "GB (Hz)", - gb_hz, - 0.5, - true, - true, - |k, v| { - if let StepKind::Apodize(Apodization::Gaussian { gb_hz, .. }) = k { - *gb_hz = v; - } - }, - ); - } - Apodization::None | Apodization::CosineBell => {} - } -} - fn zero_fill_editor( app: &mut PlotxApp, di: usize, @@ -610,58 +536,6 @@ impl BaselineVariant { } } -#[derive(Clone, Copy, PartialEq, Eq)] -enum ApoVariant { - None, - CosineBell, - Exponential, - Gaussian, -} - -impl ApoVariant { - const ALL: [Self; 4] = [ - Self::None, - Self::CosineBell, - Self::Exponential, - Self::Gaussian, - ]; - - fn label(self) -> &'static str { - match self { - Self::None => "None", - Self::CosineBell => "Cosine bell", - Self::Exponential => "Exponential", - Self::Gaussian => "Gaussian", - } - } - - fn apodization(self, cur: Apodization) -> Apodization { - let (lb, gb) = match cur { - Apodization::Exponential { lb_hz } => (lb_hz, 0.0), - Apodization::Gaussian { lb_hz, gb_hz } => (lb_hz, gb_hz), - _ => (1.0, 1.0), - }; - match self { - Self::None => Apodization::None, - Self::CosineBell => Apodization::CosineBell, - Self::Exponential => Apodization::Exponential { lb_hz: lb }, - Self::Gaussian => Apodization::Gaussian { - lb_hz: lb, - gb_hz: gb, - }, - } - } -} - -fn apo_variant(a: Apodization) -> ApoVariant { - match a { - Apodization::None => ApoVariant::None, - Apodization::CosineBell => ApoVariant::CosineBell, - Apodization::Exponential { .. } => ApoVariant::Exponential, - Apodization::Gaussian { .. } => ApoVariant::Gaussian, - } -} - #[derive(Clone, Copy, PartialEq, Eq)] enum ZfChoice { None, diff --git a/crates/app/src/ui/tools/processing/mod.rs b/crates/app/src/ui/tools/processing/mod.rs index cce7f2e..211c825 100644 --- a/crates/app/src/ui/tools/processing/mod.rs +++ b/crates/app/src/ui/tools/processing/mod.rs @@ -144,6 +144,13 @@ fn row( op: &mut Option<(StepId, RowOp)>, ) { let id = step.id; + // Expansion is this panel's own state and nothing else's. A search hit that + // wants a step opened sets it in `reveal_property`, once, as the direct + // consequence of the user activating the hit; deriving it here from the + // property focus instead let the focus's own highlight timer collapse the + // row again ~800 ms later — a layout change with no user action behind it — + // and, because a focus names a property rather than a step, opened every + // step that could carry the setting at once and asked each to scroll. let expanded = app.session.ui.proc_expanded_step == Some((owner, id)); ui.horizontal(|ui| { ui.weak(icon::DOTS_SIX_VERTICAL); diff --git a/crates/core/src/actions/app_impl/mod.rs b/crates/core/src/actions/app_impl/mod.rs index 5ed7cb4..21448df 100644 --- a/crates/core/src/actions/app_impl/mod.rs +++ b/crates/core/src/actions/app_impl/mod.rs @@ -75,6 +75,7 @@ impl PlotxApp { self.session.ui.canvas_size_edit = None; self.session.ui.processing_edit = None; self.session.ui.processing_session = None; + self.session.ui.property_gesture = None; self.session.ui.inspector_edit = None; self.session.ui.axis_overrides_before = None; self.session.ui.selection = Selection::None; @@ -84,25 +85,6 @@ impl PlotxApp { self.session.ui.processing_scheme_dialog = None; } - pub fn set_dataset_processing_state(&mut self, dataset: usize, state: &DatasetProcessingState) { - if let (Some(Dataset::Nmr2D(current)), DatasetProcessingState::Nmr2D { params, preset }) = - (self.doc.datasets.get_mut(dataset), state) - { - current.params = params.clone(); - current.preset = *preset; - self.schedule_2d_processing(dataset, false); - return; - } - let Some(current) = self.doc.datasets.get_mut(dataset) else { - return; - }; - if let Err(error) = state.apply_to(current) { - self.session.status = error.to_string(); - return; - } - self.recompute_integrals_2d_after_processing(dataset); - self.rebuild_canvases_for(dataset); - } pub fn finish_pending_wheel_zoom(&mut self, now: f64, force: bool) { let Some(pending) = self.session.ui.wheel_zoom.clone() else { return; @@ -126,7 +108,10 @@ impl PlotxApp { ); } } - fn apply_action(&mut self, action: &Action) { + /// Apply an action's `after` state to the live document without touching + /// history. Callers that record the step themselves — a paused processing + /// commit, a coalesced gesture — use this and then record once. + pub(crate) fn apply_action(&mut self, action: &Action) { macro_rules! dataset_index { ($id:expr) => { match self.doc.dataset_index($id) { diff --git a/crates/core/src/actions/app_impl/processing.rs b/crates/core/src/actions/app_impl/processing.rs index ddfcf7d..0c6c328 100644 --- a/crates/core/src/actions/app_impl/processing.rs +++ b/crates/core/src/actions/app_impl/processing.rs @@ -4,8 +4,10 @@ use super::*; impl PlotxApp { /// Record an action whose final state is already live, without applying it - /// again. Multi-surface processing sessions use this for their final commit. - fn record_applied_processing_action(&mut self, action: Action) -> Result<(), ActionApplyError> { + /// again. Multi-surface processing sessions and coalesced catalog gestures + /// use this for their final commit: both have been writing the live document + /// all along and need only the one history entry that covers the run. + pub(crate) fn record_applied_action(&mut self, action: Action) -> Result<(), ActionApplyError> { if action.is_noop() { return Ok(()); } @@ -45,6 +47,30 @@ impl PlotxApp { } } + /// Write a recipe into the live dataset and re-derive from it now. Whether + /// that means a full retransform or a cheap re-apply is decided by comparing + /// the recipes, not by the caller. Lives beside the pause gate because it is + /// the other half of it: this is what "not paused" does. + pub fn set_dataset_processing_state(&mut self, dataset: usize, state: &DatasetProcessingState) { + if let (Some(Dataset::Nmr2D(current)), DatasetProcessingState::Nmr2D { params, preset }) = + (self.doc.datasets.get_mut(dataset), state) + { + current.params = params.clone(); + current.preset = *preset; + self.schedule_2d_processing(dataset, false); + return; + } + let Some(current) = self.doc.datasets.get_mut(dataset) else { + return; + }; + if let Err(error) = state.apply_to(current) { + self.session.status = error.to_string(); + return; + } + self.recompute_integrals_2d_after_processing(dataset); + self.rebuild_canvases_for(dataset); + } + /// Commit a processing edit through the pause gate: recompute now when /// unpaused, or stash the recipe and defer the recompute to [`Self::apply_paused_processing`]. pub fn commit_processing_edit( @@ -119,7 +145,7 @@ impl PlotxApp { let dataset = &self.doc.datasets[dataset_index]; let after = DatasetProcessingState::from_dataset(dataset); let action = Action::update_dataset_processing(edit.dataset, edit.before, after); - if let Err(error) = self.record_applied_processing_action(action) { + if let Err(error) = self.record_applied_action(action) { self.session.status = error.to_string(); } } diff --git a/crates/core/src/actions/mod.rs b/crates/core/src/actions/mod.rs index d3833e6..3e8131a 100644 --- a/crates/core/src/actions/mod.rs +++ b/crates/core/src/actions/mod.rs @@ -83,6 +83,32 @@ pub struct PendingPageLayoutEdit { pub before: PageLayout, } +/// Coalesces one continuous property-catalog control gesture into a single undo +/// step. +/// +/// A drag emits one write per frame. Recording each of them would spend the +/// whole bounded undo history on a single gesture and evict everything the user +/// did before it, so the frames are applied live and outside history and the +/// gesture keeps only the two actions that bound it: `first` still carries the +/// state the gesture started from, `last` the state it ended at. Both hold +/// absolute before/after snapshots of the stores they touch, so reverting the +/// pair restores the pre-gesture state and re-applying it reproduces the final +/// one. +/// +/// A recipe edit is not represented here at all. Processing already owns a +/// mechanism for exactly this — a processing session, whose live edits are +/// recorded once when it ends — and the gesture borrows it rather than keeping +/// a second copy of the same frames. +pub struct PendingPropertyGesture { + pub property: crate::properties::PropertyId, + pub first: Option, + pub last: Option, + /// Set when this gesture opened the processing session it writes through, + /// and must therefore be the one to close it. A session opened by something + /// else — on-plot phasing, say — outlives the gesture. + pub owns_processing_session: bool, +} + /// Coalesces a single object-inspector interaction (a DragValue drag, a colour /// pick, a text edit) into one undo step: the pre-edit frames and styles of the /// touched objects, committed once the interaction ends. diff --git a/crates/core/src/actions/processing_state.rs b/crates/core/src/actions/processing_state.rs index bfaf2b6..9fe1b44 100644 --- a/crates/core/src/actions/processing_state.rs +++ b/crates/core/src/actions/processing_state.rs @@ -1,4 +1,5 @@ use super::*; +use plotx_processing::ProcessingStep; impl DatasetProcessingState { pub fn from_dataset(dataset: &Dataset) -> Self { @@ -17,6 +18,24 @@ impl DatasetProcessingState { } } + /// Every step of every axis this recipe carries. + /// + /// A caller that holds a `StepId` wants the step, not the dimension it + /// happens to sit in: step identity is owner-local and stable, while the + /// axis split is a detail of how a recipe is stored. Answering it here keeps + /// that detail with the type that owns the variants instead of copying the + /// split into every editor that addresses a step. + pub fn steps_mut(&mut self) -> impl Iterator { + let pipelines: Vec<&mut AxisPipeline> = match self { + Self::Nmr { pipeline, .. } => vec![pipeline], + Self::Nmr2D { params, .. } => vec![&mut params.f2, &mut params.f1], + Self::Table | Self::Electrophysiology(_) | Self::Afm => Vec::new(), + }; + pipelines + .into_iter() + .flat_map(|pipeline| pipeline.steps.iter_mut()) + } + /// Apply this recipe to a canonical dataset and rebuild only as much cached /// processing state as the recipe change requires. UI actions and headless /// workflows share this path so a scheme has identical numerical semantics. diff --git a/crates/core/src/automation/properties.rs b/crates/core/src/automation/properties.rs index 96f222b..fc49b51 100644 --- a/crates/core/src/automation/properties.rs +++ b/crates/core/src/automation/properties.rs @@ -17,8 +17,8 @@ use super::registry::parse; use super::*; use crate::properties::{ AggregateValue, Availability, EnumVariant, FloatBounds, PropertyAccess, PropertyAddress, - PropertyDefinition, PropertyError, PropertyValue, ResolvedProperty, ResolvedSchema, - ValueSchema, definition_by_key, variant_list, + PropertyDefinition, PropertyError, PropertySkip, PropertyValue, ResolvedProperty, + ResolvedSchema, ValueSchema, definition_by_key, variant_list, }; use crate::state::PlotxApp; use serde::{Deserialize, Serialize}; @@ -82,26 +82,28 @@ pub(super) fn refine_plan( expanded.push(planned); continue; } - let components = app.resource_series_targets(&planned.target.resource); + let components = app.resource_property_targets(&planned.target.resource, definition); if components.is_empty() { expanded.push(PlannedTarget { target: planned.target, status: TargetCompatibility::Skipped, - reason: "this plot has no series to address".to_owned(), + reason: format!( + "this resource has no {} component to address", + definition.applicability.component.as_str() + ), }); continue; } for target in components { - // Applicability per component comes from the definition, resolved - // against the series' own field — never from the tool descriptor, - // which cannot see a series at all. A read is the cheapest form of - // that same question, and asking it here means the plan preview and - // the commit agree by construction. + // Applicability per component comes from the definition, never from + // the tool descriptor. A read is the cheapest form of that same + // question, and asking it here means the plan preview and the + // commit agree by construction. let address = PropertyAddress::new(target.clone(), definition.id); let (status, reason) = match app.resolve_property(&address) { Ok(_) => ( TargetCompatibility::Compatible, - format!("{} applies to this series", definition.canonical_label), + format!("{} applies to this component", definition.canonical_label), ), Err(error) => (TargetCompatibility::Skipped, error.to_string()), }; @@ -162,6 +164,7 @@ fn inspect( target: address.target.clone(), outcome: TargetOutcome::Succeeded, message: "read".to_owned(), + skip_reason: None, fingerprints: Vec::new(), })); targets.extend(skipped_results(&set.skipped_targets)); @@ -187,10 +190,11 @@ fn inspect( /// Execute a validated commit and report every target exactly once. /// -/// A commit that applies to nothing is not executed at all. Running an empty -/// composite would advance the document revision and land in the undo stack, -/// so a call that changed nothing would be indistinguishable from one that did -/// — and would give a caller an undo entry that undoes nothing. +/// A commit that applies to nothing is not executed at all. What that buys is +/// the *report*, not the document: an empty composite is a no-op the action +/// layer already drops, so nothing would reach history either way. Stopping +/// here is what lets the result say the targets were skipped, and why, instead +/// of reporting a success a caller cannot tell apart from a real change. fn commit_and_report( app: &mut PlotxApp, plan: &ToolPlan, @@ -209,6 +213,7 @@ fn commit_and_report( target: address.target.clone(), outcome: TargetOutcome::Succeeded, message: verb.to_owned(), + skip_reason: None, fingerprints: Vec::new(), })); targets.extend(skipped_results(&skipped)); @@ -229,8 +234,12 @@ fn commit_and_report( verification: vec![VerificationRecord { check: "revision_advanced".to_owned(), passed: applied.is_empty() || after > before, + // A commit can apply to nothing because every target refused the + // property *or* because every target already held the value, and + // those are opposite answers to "did I address the right thing?". + // The per-target reasons say which; this line must not assert one. message: if applied.is_empty() { - "no target accepted this property; nothing was committed".to_owned() + "nothing was committed; see each target's reason".to_owned() } else { "atomic document commit completed".to_owned() }, @@ -249,6 +258,12 @@ fn skipped_from_plan(plan: &ToolPlan) -> Vec { target: target.target.clone(), outcome: TargetOutcome::Skipped, message: target.reason.clone(), + // Plan-time skips keep prose only. Some come from the shared + // kind/capability gate, which runs before the catalog sees the + // target at all; the rest are `refine_plan`'s own applicability + // answer. Neither is ever a same-value no-op, so the absence of a + // reason here is itself the distinction a caller needs. + skip_reason: None, fingerprints: Vec::new(), }) .collect() @@ -256,25 +271,24 @@ fn skipped_from_plan(plan: &ToolPlan) -> Vec { /// The targets the *planner* skipped, as opposed to the shared gate. /// -/// Through these tools this is currently always empty, and deliberately so: -/// `refine_plan` decides applicability by asking `resolve_property`, which is -/// the same question the planner asks, so a target the planner would skip has -/// already been marked `Skipped` at plan time and never reaches the commit. The -/// only way the two could disagree is a document that changed between planning -/// and execution, which the revision check rejects outright. +/// The ordinary way this is non-empty is a write whose value a target already +/// holds: `refine_plan` decides applicability by asking `resolve_property`, so +/// every target that is merely inapplicable was already marked `Skipped` at plan +/// time — but "does this apply" and "would this change anything" are different +/// questions, and only the planner asks the second. A `properties.set` that +/// re-sends the value a series is already showing reports every target here. /// -/// It is kept rather than dropped because the planner's contract is that it -/// reports skips, and this adapter's job is to forward what it reports. Deleting -/// this would replace an empty list with a silent discard, and the first planner -/// rule that does not have a read-side equivalent — a write refused for a reason -/// a read cannot see — would vanish from the result with nothing to notice it. -fn skipped_results(skipped: &[(TargetRef, String)]) -> Vec { +/// Each skip carries a typed reason as well as its prose, so a caller can tell +/// a same-value no-op from a target that refused the property without matching +/// on wording. +fn skipped_results(skipped: &[PropertySkip]) -> Vec { skipped .iter() - .map(|(target, reason)| TargetResult { - target: target.clone(), + .map(|skip| TargetResult { + target: skip.target.clone(), outcome: TargetOutcome::Skipped, - message: reason.clone(), + message: skip.message.clone(), + skip_reason: Some(skip.reason.as_str().to_owned()), fingerprints: Vec::new(), }) .collect() diff --git a/crates/core/src/automation/properties_tests.rs b/crates/core/src/automation/properties_tests.rs index bafce4a..a667ba5 100644 --- a/crates/core/src/automation/properties_tests.rs +++ b/crates/core/src/automation/properties_tests.rs @@ -6,11 +6,15 @@ use super::*; use crate::properties::tests::{contour_app, contour_spec}; -use crate::properties::{contour, definition_by_key}; +use crate::properties::{ + AggregateValue, PropertyAddress, PropertyValue, apodization, contour, definition_by_key, + typography, +}; use crate::state::{ CONTOUR_BASE_FRACTION_OF_RANGE, CONTOUR_BASE_NOISE_FLOOR, CanvasObject, CanvasObjectKind, - ObjectFrame, PlotxApp, SeriesBinding, TextBox, + Dataset, NmrDataset, ObjectFrame, PlotxApp, SeriesBinding, TextBox, }; +use plotx_io::{Domain, NmrData}; pub(super) fn request( app: &PlotxApp, @@ -319,6 +323,104 @@ fn inspect_reads_the_value_and_reports_skips() { ); } +/// A document-scoped property expands to the document root itself instead of +/// pretending it owns a series. This is the `ComponentKind::None` counterpart +/// to the existing plot-object expansion test. +#[test] +fn document_property_tools_address_the_document_root() { + let (mut app, _) = contour_app(); + let request = request( + &app, + TOOL_SET, + serde_json::json!({"key": typography::TICK_PT.as_str(), "value": 9.5}), + vec![DOCUMENT_RESOURCE_ID.to_owned()], + CallerType::Agent, + ); + let plan = plan_tool(&app, request).expect("the document property plans"); + assert_eq!(plan.targets.len(), 1); + assert_eq!(plan.targets[0].status, TargetCompatibility::Compatible); + assert!(plan.targets[0].target.component.is_none()); + let authority = plan.required_authority; + let result = execute_tool(&mut app, plan, authority).expect("the document property executes"); + assert!( + result + .targets + .iter() + .any(|target| target.outcome == TargetOutcome::Succeeded), + "the document root is reported as an applied target" + ); + assert_eq!(app.doc.style_library.figure_typography.tick_pt, 9.5); +} + +/// Dataset resources expand to their stable processing-step components. Only +/// the apodization step accepts this property; the other real pipeline steps +/// remain visible as reported skips rather than being silently omitted. +#[test] +fn dataset_property_tools_expand_processing_steps_and_report_non_apodization_skips() { + let mut app = PlotxApp::new(); + app.doc + .datasets + .push(Dataset::Nmr(Box::new(NmrDataset::load(NmrData { + points: (0..32) + .map(|value| num_complex::Complex64::new(f64::from(value), 0.0)) + .collect(), + domain: Domain::Time, + spectral_width_hz: 4_000.0, + observe_freq_mhz: 400.0, + carrier_ppm: 0.0, + nucleus: "1H".to_owned(), + source: "automation apodization".to_owned(), + group_delay: 0.0, + })))); + let dataset = app.doc.datasets[0].resource_id().to_string(); + let request = request( + &app, + TOOL_SET, + serde_json::json!({ + "key": apodization::KIND.as_str(), + "value": apodization::APODIZATION_EXPONENTIAL, + }), + vec![dataset], + CallerType::Agent, + ); + let plan = plan_tool(&app, request).expect("the dataset step property plans"); + let compatible = plan + .targets + .iter() + .filter(|target| target.status == TargetCompatibility::Compatible) + .collect::>(); + assert_eq!(compatible.len(), 1, "only the apodization step accepts it"); + assert!( + plan.targets + .iter() + .any(|target| target.status == TargetCompatibility::Skipped), + "the rest of the real pipeline is reported as skipped" + ); + let target = compatible[0].target.clone(); + let authority = plan.required_authority; + let result = execute_tool(&mut app, plan, authority).expect("the accepted step executes"); + assert!( + result + .targets + .iter() + .any(|target| target.outcome == TargetOutcome::Succeeded), + "the apodization component reports success" + ); + assert!( + result + .targets + .iter() + .any(|target| target.outcome == TargetOutcome::Skipped), + "the non-apodization components report their skips" + ); + assert_eq!( + app.resolve_property(&PropertyAddress::new(target, apodization::KIND)) + .expect("the stable step target still resolves") + .value, + AggregateValue::Uniform(PropertyValue::Enum(apodization::APODIZATION_EXPONENTIAL)), + ); +} + /// A read-only tool must not be usable to write, and the refusal has to happen /// before anything is planned. #[test] @@ -446,7 +548,11 @@ fn the_property_tools_are_gated_by_capability() { ); assert_eq!( descriptor.target_kinds, - vec![ResourceKindId::new(KIND_CANVAS_OBJECT)], + vec![ + ResourceKindId::new(KIND_DOCUMENT), + ResourceKindId::new(KIND_DATASET), + ResourceKindId::new(KIND_CANVAS_OBJECT), + ], "{id}" ); } @@ -487,3 +593,96 @@ fn every_property_definition_is_reachable_by_key() { ); } } + +/// Admission to the property tools is a capability question, and the capability +/// is the catalog's own answer about whether a resource has components to +/// address. Deriving it from the dataset variant instead would put a data-domain +/// branch in the admission gate — the one thing the encoding and property +/// registries exist to avoid — and would drift the moment a kind gained or lost +/// addressable components. +#[test] +fn the_catalog_capability_follows_addressable_components_not_the_dataset_kind() { + let (mut app, _) = contour_app(); + app.doc + .datasets + .push(Dataset::Nmr(Box::new(NmrDataset::load(NmrData { + points: (0..8) + .map(|value| num_complex::Complex64::new(f64::from(value), 0.0)) + .collect(), + domain: Domain::Frequency, + spectral_width_hz: 4_000.0, + observe_freq_mhz: 400.0, + carrier_ppm: 0.0, + nucleus: "1H".to_owned(), + source: "capability gate".to_owned(), + group_delay: 0.0, + })))); + let catalog = CapabilityId::new(CAP_PROPERTY_CATALOG); + let provider = ProjectResourceProvider::new(&app); + let descriptors = provider.descriptors(); + let mut checked = 0; + for dataset in &app.doc.datasets { + let id = dataset.resource_id().to_string(); + let descriptor = descriptors + .iter() + .find(|descriptor| descriptor.resource.id == id) + .unwrap_or_else(|| panic!("dataset {id} has a descriptor")); + assert_eq!( + descriptor.capabilities.contains(&catalog), + crate::properties::has_addressable_components(dataset), + "the capability of {id} disagrees with what the catalog can address" + ); + checked += 1; + } + assert!( + checked >= 2, + "the fixture covers more than one dataset kind" + ); +} + +/// A caller has to be able to tell "that is already the value" from "that does +/// not apply here" without reading English. The two are opposite answers to +/// whether the call addressed the right thing, and a re-sent value is the +/// ordinary way a skip reaches the result at all. +#[test] +fn a_same_value_write_reports_a_typed_skip_rather_than_a_denial() { + let (mut app, _) = contour_app(); + add_line_series(&mut app); + let request = set_request(&app, contour::COUNT.as_str(), serde_json::json!(9)); + run(&mut app, request).expect("the contour series accepts the write"); + + let request = set_request(&app, contour::COUNT.as_str(), serde_json::json!(9)); + let result = run(&mut app, request).expect("re-sending the same value is not an error"); + let skipped: Vec<_> = result + .targets + .iter() + .filter(|target| target.outcome == TargetOutcome::Skipped) + .collect(); + assert_eq!(skipped.len(), 2, "{:?}", result.targets); + + let reasons: Vec> = skipped + .iter() + .map(|target| target.skip_reason.as_deref()) + .collect(); + assert!( + reasons.contains(&Some("already_at_value")), + "the contour series held the value already: {reasons:?}" + ); + // The line series was ruled out at plan time, by the same applicability + // question, and reaches the result through the shared gate's own list. It + // carries no catalog reason — which is precisely what makes the two + // distinguishable without reading either message. + assert!( + reasons.contains(&None), + "the line series never had this property: {reasons:?}" + ); + + // The verification line may not claim the property was refused: one of these + // targets accepted it and simply had nothing to change. + let verification = &result.verification[0]; + assert!( + !verification.message.contains("no target accepted"), + "a same-value write is not a denial: {}", + verification.message + ); +} diff --git a/crates/core/src/automation/registry.rs b/crates/core/src/automation/registry.rs index d818ac9..97d46e3 100644 --- a/crates/core/src/automation/registry.rs +++ b/crates/core/src/automation/registry.rs @@ -334,7 +334,7 @@ fn descriptors() -> Vec { "Inspect a property", "Read one catalog property across the components of the selected resources", super::properties::PropertyKeyParams, - [KIND_CANVAS_OBJECT], + [KIND_DOCUMENT, KIND_DATASET, KIND_CANVAS_OBJECT], [CAP_PROPERTY_CATALOG], EffectLevel::ReadOnly, true @@ -344,7 +344,7 @@ fn descriptors() -> Vec { "Set a property", "Write one catalog property through the same planner the panel controls use", super::properties::PropertyWriteParams, - [KIND_CANVAS_OBJECT], + [KIND_DOCUMENT, KIND_DATASET, KIND_CANVAS_OBJECT], [CAP_PROPERTY_CATALOG], EffectLevel::Reversible, true @@ -354,7 +354,7 @@ fn descriptors() -> Vec { "Reset a property", "Re-derive one catalog property from its default policy in each target's context", super::properties::PropertyKeyParams, - [KIND_CANVAS_OBJECT], + [KIND_DOCUMENT, KIND_DATASET, KIND_CANVAS_OBJECT], [CAP_PROPERTY_CATALOG], EffectLevel::Reversible, true diff --git a/crates/core/src/automation/resources.rs b/crates/core/src/automation/resources.rs index 2012d3b..1328ecc 100644 --- a/crates/core/src/automation/resources.rs +++ b/crates/core/src/automation/resources.rs @@ -7,12 +7,15 @@ use crate::state::{Dataset, PlotxApp}; use std::collections::BTreeMap; pub const KIND_DATASET: &str = "plotx.dataset"; +pub const KIND_DOCUMENT: &str = "plotx.document"; pub const KIND_CANVAS: &str = "plotx.canvas"; pub const KIND_TABLE_ROW: &str = "plotx.table.row"; pub const KIND_TABLE_COLUMN: &str = "plotx.table.column"; pub const KIND_FIELD: &str = "plotx.field"; pub const KIND_CANVAS_OBJECT: &str = "plotx.canvas.object"; pub const KIND_EXTERNAL_INPUT: &str = "plotx.external_input"; +/// The one document root inside a `PlotxApp` target space. +pub const DOCUMENT_RESOURCE_ID: &str = "document"; pub const CAP_RENAME: &str = "resource.rename"; pub const CAP_RENDER: &str = "figure.render"; @@ -88,6 +91,13 @@ impl<'a> ProjectResourceProvider<'a> { if matches!(dataset, Dataset::Nmr(_) | Dataset::Nmr2D(_)) { capabilities.push(cap(CAP_PROCESSING_SCHEME)); } + // Asked of the catalog, not of the variant: the gate is "does this + // resource hold components the catalog can address", and the catalog is + // the only thing that knows. A resource kind that grows addressable + // components later gains the tools without a branch being added here. + if crate::properties::has_addressable_components(dataset) { + capabilities.push(cap(CAP_PROPERTY_CATALOG)); + } let mut metadata = BTreeMap::new(); metadata.insert("domain".to_owned(), format!("{:?}", dataset.domain())); metadata.insert("index_hint".to_owned(), index.to_string()); @@ -253,7 +263,22 @@ impl ResourceProvider for ProjectResourceProvider<'_> { } fn descriptors(&self) -> Vec { - let mut descriptors = Vec::new(); + let mut descriptors = vec![ResourceDescriptor { + resource: ResourceRef { + id: DOCUMENT_RESOURCE_ID.to_owned(), + kind: ResourceKindId::new(KIND_DOCUMENT), + parent_id: None, + local_id: None, + }, + name: "PlotX document".to_owned(), + capabilities: vec![cap(CAP_PROPERTY_CATALOG)], + children: Vec::new(), + dimensions: Vec::new(), + units: Vec::new(), + metadata: BTreeMap::new(), + lineage: Vec::new(), + revision: self.revision(), + }]; for (index, dataset) in self.app.doc.datasets.iter().enumerate() { let parent = self.dataset_descriptor(index, dataset); if let Dataset::Table(table) = dataset { diff --git a/crates/core/src/automation/tool_executors.rs b/crates/core/src/automation/tool_executors.rs index 5b06f06..9a3db0e 100644 --- a/crates/core/src/automation/tool_executors.rs +++ b/crates/core/src/automation/tool_executors.rs @@ -256,6 +256,7 @@ pub(super) fn execute_import( target: external, outcome: TargetOutcome::Failed, message: error.to_string(), + skip_reason: None, fingerprints: fingerprint_file(path, "selected_input") .into_iter() .collect(), @@ -290,6 +291,7 @@ pub(super) fn execute_import( target: external, outcome: TargetOutcome::Succeeded, message: "imported into the canonical PlotX data model".to_owned(), + skip_reason: None, fingerprints, }); } @@ -381,6 +383,7 @@ pub(super) fn execute_transform( target, outcome: TargetOutcome::Succeeded, message: "executed as a pinned PlotX RelPlanV1 input".into(), + skip_reason: None, fingerprints: Vec::new(), }) .collect(), @@ -427,6 +430,7 @@ pub(super) fn execute_export( target: target.target.clone(), outcome: TargetOutcome::Skipped, message: target.reason.clone(), + skip_reason: None, fingerprints: Vec::new(), }) .collect::>(); @@ -443,6 +447,7 @@ pub(super) fn execute_export( target: TargetRef::resource(target.clone()), outcome: TargetOutcome::Failed, message: "output already exists and overwrite is false".to_owned(), + skip_reason: None, fingerprints: expected .iter() .filter_map(|path| fingerprint_file(path, "existing_output")) @@ -475,6 +480,7 @@ pub(super) fn execute_export( target: TargetRef::resource(target.clone()), outcome: TargetOutcome::Failed, message: error.to_string(), + skip_reason: None, fingerprints: Vec::new(), }); continue; @@ -489,6 +495,7 @@ pub(super) fn execute_export( .map(|path| path.display().to_string()) .collect::>() .join(", "), + skip_reason: None, fingerprints: written .iter() .filter_map(|path| fingerprint_file(path, "output")) @@ -545,6 +552,7 @@ fn commit_actions( } else { target.reason.clone() }, + skip_reason: None, fingerprints: Vec::new(), }) .collect(), diff --git a/crates/core/src/automation/types.rs b/crates/core/src/automation/types.rs index 61269ec..1f71725 100644 --- a/crates/core/src/automation/types.rs +++ b/crates/core/src/automation/types.rs @@ -361,6 +361,12 @@ pub struct TargetResult { pub target: TargetRef, pub outcome: TargetOutcome, pub message: String, + /// Why a skipped target was skipped, as a stable enumerated token. `message` + /// is prose written to be read and free to be reworded; a caller that has to + /// tell "already this value" from "does not apply" branches on this instead. + /// Absent unless the outcome is `Skipped`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub skip_reason: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub fingerprints: Vec, } diff --git a/crates/core/src/project/pipeline_conv.rs b/crates/core/src/project/pipeline_conv.rs index 7bf18fa..4bc1d2f 100644 --- a/crates/core/src/project/pipeline_conv.rs +++ b/crates/core/src/project/pipeline_conv.rs @@ -150,7 +150,21 @@ fn apodization_from_dto(a: &ApodizationDto) -> Apodization { ApodizationDto::None => Apodization::None, ApodizationDto::CosineBell => Apodization::CosineBell, ApodizationDto::Exponential { lb_hz } => Apodization::Exponential { lb_hz }, - ApodizationDto::Gaussian { lb_hz, gb_hz } => Apodization::Gaussian { lb_hz, gb_hz }, + // A Gaussian window's time-domain term is `(pi*gb)^2 / (4 ln 2)`, so a + // stored `gb` of zero leaves nothing but exponential growth and a + // negative one behaves as its own magnitude. Neither is a window this + // application can show or edit, so a file carrying one is repaired to + // the neutral value rather than opened into a state the panel refuses. + // Not to the bound's smallest admissible value: that is the next number + // above zero, and the resulting gain overflows to infinity. + ApodizationDto::Gaussian { lb_hz, gb_hz } => Apodization::Gaussian { + lb_hz, + gb_hz: if gb_hz.is_finite() && gb_hz > 0.0 { + gb_hz + } else { + crate::properties::apodization::GB_DEFAULT_HZ + }, + }, } } diff --git a/crates/core/src/properties/apodization.rs b/crates/core/src/properties/apodization.rs new file mode 100644 index 0000000..38f3617 --- /dev/null +++ b/crates/core/src/properties/apodization.rs @@ -0,0 +1,572 @@ +//! Dataset-owned apodization-step properties. +//! +//! A processing step is an owner-local component, not a path through a +//! pipeline. `StepId` is consequently the only component identity this module +//! accepts; resolving the containing axis is an implementation detail of the +//! dataset snapshot the typed action already owns. + +use super::provider::PropertyProvider; +use super::target::dataset_steps; +use super::{ + AggregateValue, Applicability, Availability, ComponentKind, DefaultPolicy, EditOp, EnumVariant, + FloatBounds, PropertyAccess, PropertyAddress, PropertyDefinition, PropertyError, PropertyId, + PropertyTransaction, PropertyValue, ResolvedProperty, ResolvedSchema, ScopeKind, Tier, + ValueCopies, ValueSchema, definition, +}; +use crate::actions::DatasetProcessingState; +use crate::automation::ComponentRef; +use crate::state::{Dataset, DatasetId, PhaseAxis, PlotxApp}; +use plotx_processing::{Apodization, ProcessingStep, StepId, StepKind, StepSource}; + +pub const KIND: PropertyId = PropertyId("dataset.processing.apodization.kind"); +pub const LB_HZ: PropertyId = PropertyId("dataset.processing.apodization.lb_hz"); +pub const GB_HZ: PropertyId = PropertyId("dataset.processing.apodization.gb_hz"); + +pub const APODIZATION_NONE: &str = "none"; +pub const APODIZATION_COSINE_BELL: &str = "cosine_bell"; +pub const APODIZATION_EXPONENTIAL: &str = "exponential"; +pub const APODIZATION_GAUSSIAN: &str = "gaussian"; + +/// Line broadening admits both signs. In the window this crate applies, +/// `exp(+pi*lb*t - g*t^2)` for a Gaussian and `exp(-pi*lb*t)` for an +/// exponential, a positive LB narrows lines under a Gaussian — the +/// Lorentz-to-Gauss resolution enhancement — and a negative one broadens them +/// further. Both are wanted, so neither sign is excluded. +/// +/// The range is deliberately wide, and therefore says nothing about how far one +/// drag notch should move the value. That is why the definitions declare the +/// notch separately. +const LB_BOUNDS: FloatBounds = FloatBounds::inclusive(-10_000.0, 10_000.0); +/// Gaussian broadening is open at zero, and the bound is not decoration. The +/// window's Gaussian term is `g = (pi*gb)^2 / (4 ln 2)`, so `gb = 0` leaves +/// `exp(+pi*lb*t)` — a pure exponential *growth* with no maximum and no decay, +/// which is not a window at all. `g` is also even in `gb`, so a negative value +/// behaves exactly as its magnitude: admitting one would let the panel read back +/// a number that does not describe what the transform did. +const GB_BOUNDS: FloatBounds = FloatBounds::above(0.0, 10_000.0); +/// Half a hertz per notch — the step the inline processing editor used before +/// these parameters moved into the catalog, and the resolution at which typical +/// line broadenings (0.3–5 Hz) are actually chosen. +const PARAMETER_STEP: f64 = 0.5; +/// The broadening a window that has none yet starts from. Switching a step to +/// exponential or Gaussian and resetting one both land here, so the two cannot +/// disagree about what this parameter's neutral value is. +pub const LB_DEFAULT_HZ: f64 = 1.0; +/// The Gaussian broadening a window that has none yet starts from. It may not be +/// zero: [`GB_BOUNDS`] excludes it, and a seed the schema rejects would put the +/// step in a state its own control refuses to accept back. +pub const GB_DEFAULT_HZ: f64 = 1.0; +const APODIZATION_KINDS: &[EnumVariant] = &[ + EnumVariant::new(APODIZATION_NONE, "None"), + EnumVariant::new(APODIZATION_COSINE_BELL, "Cosine bell"), + EnumVariant::new(APODIZATION_EXPONENTIAL, "Exponential"), + EnumVariant::new(APODIZATION_GAUSSIAN, "Gaussian"), +]; + +const APODIZATION_STEP: Applicability = Applicability::component(ComponentKind::ProcessingStep); + +pub(crate) const DEFINITIONS: &[PropertyDefinition] = &[ + PropertyDefinition { + id: KIND, + scope_kind: ScopeKind::Dataset, + value_schema: ValueSchema::Enum { + variants: APODIZATION_KINDS, + }, + access: PropertyAccess::ReadWrite, + applicability: APODIZATION_STEP, + default_policy: DefaultPolicy::ProcessingFactory, + tier: Tier::Essential, + copies: ValueCopies::PerTarget, + canonical_label: "Apodization window", + canonical_aliases: &["apodization", "window function", "exponential", "gaussian"], + }, + PropertyDefinition { + id: LB_HZ, + scope_kind: ScopeKind::Dataset, + value_schema: ValueSchema::Float { + bounds: LB_BOUNDS, + log: false, + drag_step: Some(PARAMETER_STEP), + }, + access: PropertyAccess::ReadWrite, + applicability: APODIZATION_STEP, + default_policy: DefaultPolicy::ProcessingFactory, + // Line broadening is the most-reached-for setting in NMR processing and + // sat beside the window choice in the editor this replaced. Advanced is + // for what a user rarely looks for, not for what merely has a number. + tier: Tier::Essential, + copies: ValueCopies::PerTarget, + canonical_label: "Apodization line broadening", + canonical_aliases: &["LB", "line broadening", "apodization lb"], + }, + PropertyDefinition { + id: GB_HZ, + scope_kind: ScopeKind::Dataset, + value_schema: ValueSchema::Float { + bounds: GB_BOUNDS, + log: false, + drag_step: Some(PARAMETER_STEP), + }, + access: PropertyAccess::ReadWrite, + applicability: APODIZATION_STEP, + default_policy: DefaultPolicy::ProcessingFactory, + // Only ever shown alongside LB, and meaningless without it. + tier: Tier::Essential, + copies: ValueCopies::PerTarget, + canonical_label: "Apodization Gaussian broadening", + canonical_aliases: &["GB", "gaussian broadening", "apodization gb"], + }, +]; + +pub(crate) struct ApodizationProvider; + +pub(crate) static PROVIDER: ApodizationProvider = ApodizationProvider; + +impl PropertyProvider for ApodizationProvider { + fn definitions(&self) -> &'static [PropertyDefinition] { + DEFINITIONS + } + + fn read( + &self, + app: &PlotxApp, + address: &PropertyAddress, + ) -> Result { + let definition = property_definition(address.definition)?; + let context = context(app, address, definition)?; + let value = value_of(definition, context.current)?; + Ok(ResolvedProperty { + address: address.clone(), + value: AggregateValue::Uniform(value), + default_value: default_value(definition, context.factory)?, + availability: Availability::Editable, + schema: schema_for(definition, context.current)?, + }) + } + + fn edit( + &self, + app: &PlotxApp, + transaction: &mut PropertyTransaction, + address: &PropertyAddress, + operation: EditOp, + ) -> Result<(), PropertyError> { + let definition = property_definition(address.definition)?; + let context = context(app, address, definition)?; + let value = match operation { + EditOp::Set(value) => checked_value(definition, context.current, value)?, + // A hand-added step has no factory setting behind it, so there is + // nothing to reset *to*. Saying so skips this target and leaves the + // rest of a multi-target reset to land. + EditOp::Reset => default_value(definition, context.factory)?.ok_or_else(|| { + PropertyError::NotApplicable(format!( + "{} was added by hand and has no factory setting to reset to", + definition.canonical_label + )) + })?, + EditOp::Step(_) => { + return Err(PropertyError::InvalidValue { + property: definition.id, + message: "this processing setting has no step gesture".to_owned(), + }); + } + }; + let state = transaction.processing_state(app, context.dataset)?; + let apodization = apodization_mut(state, context.step, &address.target)?; + write(definition, apodization, value) + } +} + +#[derive(Clone, Copy)] +struct ApodizationContext { + dataset: DatasetId, + step: StepId, + current: Apodization, + /// The factory's window for this step, absent for a step the user added. + factory: Option, +} + +fn property_definition(id: PropertyId) -> Result<&'static PropertyDefinition, PropertyError> { + definition(id).ok_or_else(|| PropertyError::UnknownProperty(id.as_str().to_owned())) +} + +fn context( + app: &PlotxApp, + address: &PropertyAddress, + definition: &'static PropertyDefinition, +) -> Result { + let actual = ComponentKind::of(address.target.component.as_ref()); + if actual != definition.applicability.component { + return Err(PropertyError::ComponentKind { + property: definition.id, + expected: definition.applicability.component.as_str(), + actual: actual.as_str(), + }); + } + let Some(ComponentRef::ProcessingStep(step)) = address.target.component else { + return Err(PropertyError::ComponentKind { + property: definition.id, + expected: ComponentKind::ProcessingStep.as_str(), + actual: actual.as_str(), + }); + }; + let dataset = DatasetId::try_from(&address.target.resource).map_err(|error| { + PropertyError::NotApplicable(format!( + "{} needs a dataset resource: {error}", + definition.id + )) + })?; + let dataset_value = app + .doc + .dataset_by_id(dataset) + .ok_or_else(|| PropertyError::UnknownTarget(address.target.resource.id.clone()))?; + let (axis, processing_step) = step_in_dataset(dataset_value, step, &address.target)?; + let StepKind::Apodize(current) = processing_step.kind else { + return Err(PropertyError::NotApplicable(format!( + "{} addresses an apodization step, but step {} is {}", + definition.canonical_label, + step.get(), + step_kind_name(&processing_step.kind) + ))); + }; + Ok(ApodizationContext { + dataset, + step, + current, + factory: factory_default(dataset_value, axis, processing_step), + }) +} + +/// Locate an addressed step among the ones the dataset actually exposes. +/// +/// The search runs over [`dataset_steps`], so a step on an axis the rest of the +/// application hides is not addressable here either. +fn step_in_dataset<'a>( + dataset: &'a Dataset, + id: StepId, + target: &crate::automation::TargetRef, +) -> Result<(PhaseAxis, &'a ProcessingStep), PropertyError> { + dataset_steps(dataset) + .find(|(_, step)| step.id == id) + .ok_or_else(|| PropertyError::UnknownTarget(target.describe())) +} + +/// The window the factory recipe puts in this step, if it puts one there at all. +/// +/// The answer is read out of the factory itself rather than re-derived from the +/// dataset's shape: a second derivation agrees with the factory only until one +/// of them changes. A step the user added by hand has no counterpart in the +/// factory recipe and therefore no default — `None` here, not "no window". +/// Claiming one would mark a freshly added step as already modified, and would +/// let its reset button turn a window the user deliberately chose into a step +/// that sits in the pipeline doing nothing. +fn factory_default( + dataset: &Dataset, + axis: PhaseAxis, + step: &ProcessingStep, +) -> Option { + if step.source != StepSource::Default { + return None; + } + dataset + .factory_pipeline(axis)? + .steps + .iter() + .find(|candidate| candidate.id == step.id) + .and_then(|candidate| match candidate.kind { + StepKind::Apodize(apodization) => Some(apodization), + _ => None, + }) +} + +fn value_of( + definition: &'static PropertyDefinition, + apodization: Apodization, +) -> Result { + match (definition.id, apodization) { + (KIND, value) => Ok(PropertyValue::Enum(kind_of(value))), + (LB_HZ, Apodization::Exponential { lb_hz } | Apodization::Gaussian { lb_hz, .. }) => { + Ok(PropertyValue::Float(lb_hz)) + } + (GB_HZ, Apodization::Gaussian { gb_hz, .. }) => Ok(PropertyValue::Float(gb_hz)), + _ => Err(unavailable_parameter(definition, apodization)), + } +} + +fn default_value( + definition: &'static PropertyDefinition, + factory: Option, +) -> Result, PropertyError> { + match definition.default_policy { + DefaultPolicy::ProcessingFactory => { + let Some(factory) = factory else { + return Ok(None); + }; + match definition.id { + KIND => Ok(Some(PropertyValue::Enum(kind_of(factory)))), + LB_HZ => Ok(Some(PropertyValue::Float(match factory { + Apodization::Exponential { lb_hz } | Apodization::Gaussian { lb_hz, .. } => { + lb_hz + } + Apodization::None | Apodization::CosineBell => LB_DEFAULT_HZ, + }))), + GB_HZ => Ok(Some(PropertyValue::Float(match factory { + Apodization::Gaussian { gb_hz, .. } => gb_hz, + Apodization::None + | Apodization::CosineBell + | Apodization::Exponential { .. } => GB_DEFAULT_HZ, + }))), + _ => Err(PropertyError::UnknownProperty( + definition.id.as_str().to_owned(), + )), + } + } + DefaultPolicy::Fixed(value) => Ok(Some(value)), + DefaultPolicy::EncodingFactory | DefaultPolicy::None => Err(PropertyError::InvalidValue { + property: definition.id, + message: "this property has no processing default".to_owned(), + }), + } +} + +fn schema_for( + definition: &'static PropertyDefinition, + apodization: Apodization, +) -> Result { + match definition.id { + KIND => Ok(ResolvedSchema::Enum { + variants: APODIZATION_KINDS.iter().collect(), + }), + LB_HZ + if matches!( + apodization, + Apodization::Exponential { .. } | Apodization::Gaussian { .. } + ) => + { + Ok(parameter_schema(definition)) + } + GB_HZ if matches!(apodization, Apodization::Gaussian { .. }) => { + Ok(parameter_schema(definition)) + } + _ => Err(unavailable_parameter(definition, apodization)), + } +} + +/// The bounds one broadening parameter is admitted by. They differ: line +/// broadening is signed, Gaussian broadening is not. +fn parameter_bounds(definition: &'static PropertyDefinition) -> FloatBounds { + if definition.id == GB_HZ { + GB_BOUNDS + } else { + LB_BOUNDS + } +} + +fn parameter_schema(definition: &'static PropertyDefinition) -> ResolvedSchema { + ResolvedSchema::Float { + bounds: parameter_bounds(definition), + log: false, + unit: "Hz", + } +} + +fn checked_value( + definition: &'static PropertyDefinition, + apodization: Apodization, + value: PropertyValue, +) -> Result { + match definition.id { + KIND => match value { + PropertyValue::Enum(value) if variant(value).is_some() => { + Ok(PropertyValue::Enum(value)) + } + PropertyValue::Enum(value) => Err(PropertyError::InvalidValue { + property: definition.id, + message: format!("'{value}' is not an apodization window"), + }), + value => wrong_kind(definition, value, "an apodization window"), + }, + LB_HZ | GB_HZ => { + let _ = value_of(definition, apodization)?; + match value { + PropertyValue::Float(value) => { + parameter_bounds(definition).check( + definition.id, + definition.canonical_label, + value, + )?; + Ok(PropertyValue::Float(value)) + } + value => wrong_kind(definition, value, "a number"), + } + } + _ => Err(PropertyError::UnknownProperty( + definition.id.as_str().to_owned(), + )), + } +} + +fn wrong_kind( + definition: &'static PropertyDefinition, + value: PropertyValue, + expected: &str, +) -> Result { + Err(PropertyError::InvalidValue { + property: definition.id, + message: format!("expected {expected}, got {}", value.kind()), + }) +} + +fn write( + definition: &'static PropertyDefinition, + apodization: &mut Apodization, + value: PropertyValue, +) -> Result<(), PropertyError> { + match (definition.id, value) { + (KIND, PropertyValue::Enum(kind)) => { + let Some(kind) = variant(kind) else { + return Err(PropertyError::InvalidValue { + property: definition.id, + message: "the apodization window is not recognized".to_owned(), + }); + }; + *apodization = kind.with_current(*apodization); + Ok(()) + } + (LB_HZ, PropertyValue::Float(value)) => match apodization { + Apodization::Exponential { lb_hz } | Apodization::Gaussian { lb_hz, .. } => { + *lb_hz = value; + Ok(()) + } + current => Err(unavailable_parameter(definition, *current)), + }, + (GB_HZ, PropertyValue::Float(value)) => match apodization { + Apodization::Gaussian { gb_hz, .. } => { + *gb_hz = value; + Ok(()) + } + current => Err(unavailable_parameter(definition, *current)), + }, + (_, value) => Err(PropertyError::InvalidValue { + property: definition.id, + message: format!( + "expected the property's declared value, got {}", + value.kind() + ), + }), + } +} + +fn apodization_mut<'a>( + state: &'a mut DatasetProcessingState, + id: StepId, + target: &crate::automation::TargetRef, +) -> Result<&'a mut Apodization, PropertyError> { + let step = state + .steps_mut() + .find(|step| step.id == id) + .ok_or_else(|| PropertyError::UnknownTarget(target.describe()))?; + match &mut step.kind { + StepKind::Apodize(apodization) => Ok(apodization), + kind => Err(PropertyError::NotApplicable(format!( + "step {} is {}, not an apodization step", + id.get(), + step_kind_name(kind) + ))), + } +} + +fn unavailable_parameter( + definition: &'static PropertyDefinition, + apodization: Apodization, +) -> PropertyError { + let required = match definition.id { + LB_HZ => "an exponential or Gaussian window", + GB_HZ => "a Gaussian window", + _ => "this apodization window", + }; + PropertyError::NotApplicable(format!( + "{} is available only with {required}; this step uses {}", + definition.canonical_label, + // The label, not the wire id: this sentence is read by a person, and + // "cosine_bell" is the identifier the choice is stored under. + window_label(apodization) + )) +} + +#[derive(Clone, Copy)] +enum ApodizationKind { + None, + CosineBell, + Exponential, + Gaussian, +} + +impl ApodizationKind { + /// Switch the window kind, carrying over whatever the current one already + /// says. A parameter the current kind does not carry starts from this + /// module's declared neutral value — the same one a reset lands on, and one + /// the schema admits. Seeding `gb` with zero produced a Gaussian whose own + /// control refuses the value it was given, and whose transform grows without + /// bound. + fn with_current(self, current: Apodization) -> Apodization { + let (lb_hz, gb_hz) = match current { + Apodization::Exponential { lb_hz } => (lb_hz, GB_DEFAULT_HZ), + Apodization::Gaussian { lb_hz, gb_hz } => (lb_hz, gb_hz), + Apodization::None | Apodization::CosineBell => (LB_DEFAULT_HZ, GB_DEFAULT_HZ), + }; + match self { + Self::None => Apodization::None, + Self::CosineBell => Apodization::CosineBell, + Self::Exponential => Apodization::Exponential { lb_hz }, + Self::Gaussian => Apodization::Gaussian { lb_hz, gb_hz }, + } + } +} + +/// The choice's own display label, taken from the variant list the control is +/// built from so the two cannot word it differently. +fn window_label(apodization: Apodization) -> &'static str { + let id = kind_of(apodization); + APODIZATION_KINDS + .iter() + .find(|variant| variant.id == id) + .map(|variant| variant.canonical_label) + .unwrap_or(id) +} + +fn kind_of(apodization: Apodization) -> &'static str { + match apodization { + Apodization::None => APODIZATION_NONE, + Apodization::CosineBell => APODIZATION_COSINE_BELL, + Apodization::Exponential { .. } => APODIZATION_EXPONENTIAL, + Apodization::Gaussian { .. } => APODIZATION_GAUSSIAN, + } +} + +fn variant(value: &str) -> Option { + match value { + APODIZATION_NONE => Some(ApodizationKind::None), + APODIZATION_COSINE_BELL => Some(ApodizationKind::CosineBell), + APODIZATION_EXPONENTIAL => Some(ApodizationKind::Exponential), + APODIZATION_GAUSSIAN => Some(ApodizationKind::Gaussian), + _ => None, + } +} + +fn step_kind_name(kind: &StepKind) -> &'static str { + match kind { + StepKind::Apodize(_) => "apodization", + StepKind::ZeroFill(_) => "zero fill", + StepKind::Fft => "FFT", + StepKind::Phase(_) => "phase", + StepKind::Baseline(_) => "baseline", + StepKind::Reference(_) => "reference", + StepKind::Magnitude => "magnitude", + StepKind::Smooth(_) => "smoothing", + StepKind::Normalize(_) => "normalization", + StepKind::Bin(_) => "binning", + StepKind::Reverse => "reverse", + StepKind::Invert => "invert", + } +} diff --git a/crates/core/src/properties/apodization_tests.rs b/crates/core/src/properties/apodization_tests.rs new file mode 100644 index 0000000..4656b0e --- /dev/null +++ b/crates/core/src/properties/apodization_tests.rs @@ -0,0 +1,497 @@ +//! Dataset processing-step catalog slice. + +use super::*; +use crate::actions::Action; +use crate::automation::{ComponentRef, ResourceRef, TargetRef}; +use crate::state::{Dataset, Nmr2DDataset, PhaseAxis, PlotxApp}; +use num_complex::Complex64; +use plotx_io::{Dim, Domain, NmrData2D, QuadMode}; +use plotx_processing::{Apodization, StepId, StepKind}; + +fn apodization_target(app: &PlotxApp) -> TargetRef { + let Dataset::Nmr2D(dataset) = &app.doc.datasets[0] else { + panic!("the contour fixture owns a 2D NMR dataset"); + }; + let step = dataset + .params + .f2 + .steps + .iter() + .find(|step| matches!(step.kind, StepKind::Apodize(_))) + .expect("the time-domain default has an apodization step"); + TargetRef { + resource: ResourceRef::from(dataset.resource_id), + component: Some(ComponentRef::ProcessingStep(step.id)), + } +} + +fn time_domain_app() -> PlotxApp { + time_domain_app_of(None) +} + +/// A pseudo-2D acquisition: the indirect dimension is an array, so the +/// application never Fourier transforms it and never shows its recipe. +fn pseudo_2d_app() -> PlotxApp { + time_domain_app_of(Some("dosy")) +} + +fn time_domain_app_of(experiment: Option<&str>) -> PlotxApp { + let dimension = |nucleus: &str| Dim { + spectral_width_hz: 4_000.0, + observe_freq_mhz: 400.0, + carrier_ppm: 0.0, + nucleus: nucleus.to_owned(), + group_delay: 0.0, + }; + let data = NmrData2D { + data: (0..16) + .map(|value| Complex64::new(f64::from(value), 0.5)) + .collect(), + rows: 4, + cols: 4, + domain: Domain::Time, + direct: dimension("1H"), + indirect: dimension("13C"), + quad: QuadMode::Complex, + indirect_conjugate: false, + experiment: experiment.map(ToOwned::to_owned), + pseudo_axis: None, + diffusion: None, + nus: None, + source: "apodization step".to_owned(), + }; + let mut app = PlotxApp::new(); + app.doc + .datasets + .push(Dataset::Nmr2D(Box::new(Nmr2DDataset::load(data)))); + app +} + +fn apodization_at(app: &PlotxApp, target: &TargetRef) -> (StepId, Apodization) { + let Some(ComponentRef::ProcessingStep(id)) = target.component else { + panic!("the helper constructs a processing-step target"); + }; + let Dataset::Nmr2D(dataset) = &app.doc.datasets[0] else { + panic!("the contour fixture owns a 2D NMR dataset"); + }; + let step = dataset + .params + .f2 + .steps + .iter() + .chain(dataset.params.f1.steps.iter()) + .find(|step| step.id == id) + .expect("the stable step id still resolves"); + let StepKind::Apodize(apodization) = step.kind else { + panic!("the target remains an apodization step"); + }; + (step.id, apodization) +} + +/// One `StepId`-addressed property changes both which fields exist and the +/// typed processing action that owns the pipeline. No axis index or pipeline +/// path appears at the property boundary. +#[test] +fn apodization_step_has_a_dependent_schema_and_a_typed_processing_action() { + let mut app = time_domain_app(); + let target = apodization_target(&app); + let (stable_id, initial) = apodization_at(&app, &target); + assert_eq!(initial, Apodization::CosineBell); + + let kind = PropertyAddress::new(target.clone(), apodization::KIND); + let lb = PropertyAddress::new(target.clone(), apodization::LB_HZ); + let gb = PropertyAddress::new(target.clone(), apodization::GB_HZ); + assert_eq!( + app.resolve_property(&kind) + .expect("the window choice resolves") + .value, + AggregateValue::Uniform(PropertyValue::Enum(apodization::APODIZATION_COSINE_BELL)) + ); + assert!(matches!( + app.resolve_property(&lb), + Err(PropertyError::NotApplicable(message)) if message.contains("exponential or Gaussian") + )); + + let commit = app + .plan_property_write( + apodization::KIND, + std::slice::from_ref(&target), + &PropertyValue::Enum(apodization::APODIZATION_EXPONENTIAL), + ) + .expect("an apodization kind change plans"); + let Action::Composite(actions) = &commit.action else { + panic!("a catalog commit is composite"); + }; + assert!(matches!( + actions.as_slice(), + [Action::UpdateDatasetProcessing { .. }] + )); + app.commit_property(commit); + assert_eq!(apodization_at(&app, &target).0, stable_id); + assert_eq!( + apodization_at(&app, &target).1, + Apodization::Exponential { lb_hz: 1.0 } + ); + let resolved_lb = app.resolve_property(&lb).expect("LB now exists"); + assert_eq!( + resolved_lb.value, + AggregateValue::Uniform(PropertyValue::Float(1.0)) + ); + assert!(matches!( + resolved_lb.schema, + ResolvedSchema::Float { unit: "Hz", .. } + )); + assert!(matches!( + app.resolve_property(&gb), + Err(PropertyError::NotApplicable(message)) if message.contains("Gaussian") + )); + + let commit = app + .plan_property_write( + apodization::KIND, + std::slice::from_ref(&target), + &PropertyValue::Enum(apodization::APODIZATION_GAUSSIAN), + ) + .expect("a Gaussian window plans"); + app.commit_property(commit); + assert_eq!(apodization_at(&app, &target).0, stable_id); + assert!(matches!( + app.resolve_property(&gb) + .expect("GB appears only for Gaussian") + .schema, + ResolvedSchema::Float { unit: "Hz", .. } + )); + + for (property, value) in [ + (apodization::LB_HZ, PropertyValue::Float(2.5)), + (apodization::GB_HZ, PropertyValue::Float(4.0)), + ] { + let commit = app + .plan_property_write(property, std::slice::from_ref(&target), &value) + .expect("the visible parameter writes"); + app.commit_property(commit); + } + assert_eq!( + apodization_at(&app, &target).1, + Apodization::Gaussian { + lb_hz: 2.5, + gb_hz: 4.0 + } + ); + + let commit = app + .plan_property_reset(apodization::KIND, std::slice::from_ref(&target)) + .expect("the target-specific processing factory resets the default step"); + app.commit_property(commit); + assert_eq!(apodization_at(&app, &target).0, stable_id); + assert_eq!(apodization_at(&app, &target).1, Apodization::CosineBell); + assert!(matches!( + app.resolve_property(&lb), + Err(PropertyError::NotApplicable(_)) + )); +} + +/// The catalog exposes exactly the processing steps the rest of the application +/// exposes. A pseudo-2D acquisition has no transformed indirect dimension, so +/// the Processing panel shows only F2; expanding F1 here would let a headless +/// caller write a recipe nobody can see, navigate to, or undo from a panel, and +/// be told it succeeded. +#[test] +fn a_pseudo_2d_dataset_exposes_only_the_steps_the_application_shows() { + let app = pseudo_2d_app(); + let Dataset::Nmr2D(dataset) = &app.doc.datasets[0] else { + panic!("the fixture owns a 2D NMR dataset"); + }; + assert!( + !dataset.is_true_2d(), + "the fixture has to be pseudo-2D for this to mean anything" + ); + let resource = ResourceRef::from(dataset.resource_id); + let expected = dataset.params.f2.steps.len(); + assert!(!dataset.params.f1.steps.is_empty()); + + let targets = app.resource_processing_step_targets(&resource); + assert_eq!(targets.len(), expected); + let hidden: Vec = dataset.params.f1.steps.iter().map(|step| step.id).collect(); + for target in &targets { + let Some(ComponentRef::ProcessingStep(id)) = target.component else { + panic!("a step target names a step"); + }; + assert!( + !hidden.contains(&id), + "step {id:?} lives on the axis the application hides" + ); + } +} + +/// The window choice is the panel's own control, so the panel's "Pause +/// auto-recompute" switch has to govern it. A catalog write that recomputed +/// anyway would make the switch silently do nothing, and would never record the +/// pending edit the Apply button is built on. +#[test] +fn a_paused_panel_stages_a_catalog_recipe_write_instead_of_recomputing() { + let mut app = time_domain_app(); + let target = apodization_target(&app); + app.session.ui.proc_paused = true; + + let commit = app + .plan_property_write( + apodization::KIND, + std::slice::from_ref(&target), + &PropertyValue::Enum(apodization::APODIZATION_EXPONENTIAL), + ) + .expect("an apodization kind change plans"); + assert_eq!(app.commit_property(commit), 1); + + assert!( + app.has_pending_processing(), + "a paused write has to leave an Apply for the user to press" + ); + assert!( + !app.can_undo(), + "a staged recipe is not history until it is applied" + ); + assert_eq!( + apodization_at(&app, &target).1, + Apodization::Exponential { lb_hz: 1.0 }, + "the staged recipe is live so the panel shows what Apply would do" + ); +} + +/// One drag of a continuous control is one edit. Recording a step per frame +/// would spend the bounded history on a single gesture and evict everything +/// before it. +#[test] +fn a_control_gesture_records_one_undo_step_for_the_whole_drag() { + let mut app = time_domain_app(); + let target = apodization_target(&app); + let commit = app + .plan_property_write( + apodization::KIND, + std::slice::from_ref(&target), + &PropertyValue::Enum(apodization::APODIZATION_EXPONENTIAL), + ) + .expect("an exponential window plans"); + app.commit_property(commit); + let history = app.session.undo_stack.len(); + + app.begin_property_gesture(apodization::LB_HZ); + for lb in [2.0, 3.0, 4.0] { + let commit = app + .plan_property_write( + apodization::LB_HZ, + std::slice::from_ref(&target), + &PropertyValue::Float(lb), + ) + .expect("each frame of the drag plans"); + app.commit_property(commit); + assert_eq!( + app.session.undo_stack.len(), + history, + "a frame of a drag is a live preview, not an edit" + ); + } + assert_eq!( + apodization_at(&app, &target).1, + Apodization::Exponential { lb_hz: 4.0 }, + "the preview is live while the drag runs" + ); + app.end_property_gesture(); + + assert_eq!(app.session.undo_stack.len(), history + 1); + app.undo(); + assert_eq!( + apodization_at(&app, &target).1, + Apodization::Exponential { lb_hz: 1.0 }, + "one undo has to take back the whole gesture" + ); + app.redo(); + assert_eq!( + apodization_at(&app, &target).1, + Apodization::Exponential { lb_hz: 4.0 }, + "and one redo has to restore where it ended" + ); +} + +/// The drag notch is declared, not derived. A range states what is admissible: +/// broadening is legal out to +-10 kHz, so a range-derived notch would move it a +/// hundred hertz per pixel while real values sit between 0.3 and 5 Hz. +#[test] +fn the_broadening_notch_is_declared_rather_than_derived_from_the_range() { + for id in [apodization::LB_HZ, apodization::GB_HZ] { + let definition = definition(id).expect("the parameter is registered"); + let ValueSchema::Float { + bounds, drag_step, .. + } = definition.value_schema + else { + panic!("a broadening parameter is a float"); + }; + let derived = (bounds.max - bounds.min) / 200.0; + let declared = drag_step.expect("the definition declares its own notch"); + assert!( + declared <= 1.0 && declared < derived, + "{id} declares {declared} where the range alone would give {derived}" + ); + } +} + +/// A step the user added is not a deviation from anything. Reporting a factory +/// value for it marked it modified the moment it appeared, and its reset button +/// replaced the window the user had just chosen with a step that does nothing. +#[test] +fn a_hand_added_step_has_no_factory_setting_to_reset_to() { + let mut app = time_domain_app(); + let Dataset::Nmr2D(dataset) = &mut app.doc.datasets[0] else { + panic!("the fixture owns a 2D NMR dataset"); + }; + let id = StepId::new(4_096); + let mut added = plotx_processing::ProcessingStep::new( + id, + StepKind::Apodize(Apodization::Exponential { lb_hz: 1.0 }), + plotx_processing::StepSource::User, + ); + added.enabled = true; + dataset.params.f2.steps.insert(0, added); + let resource = ResourceRef::from(dataset.resource_id); + let target = TargetRef { + resource, + component: Some(ComponentRef::ProcessingStep(id)), + }; + + let kind = PropertyAddress::new(target.clone(), apodization::KIND); + let resolved = app + .resolve_property(&kind) + .expect("the added step resolves"); + assert_eq!(resolved.default_value, None); + assert!( + !resolved.is_modified(), + "a step the user just added is not a change from a default" + ); + + let commit = app + .plan_property_reset(apodization::KIND, std::slice::from_ref(&target)) + .expect("a reset with nothing to reset is a skip, not a failure"); + assert!(commit.applied.is_empty()); + assert_eq!(commit.skipped.len(), 1); + assert_eq!(commit.skipped[0].reason, super::SkipReason::NotApplicable); + app.commit_property(commit); + assert_eq!( + apodization_at(&app, &target).1, + Apodization::Exponential { lb_hz: 1.0 }, + "the window the user chose survives a reset that had nothing to do" + ); +} + +/// The factory default is read out of the factory, not re-derived. This is what +/// keeps "what does reset give me" and "what does a new document get" from +/// drifting apart. +#[test] +fn the_step_default_is_read_from_the_pipeline_factory() { + let app = time_domain_app(); + let target = apodization_target(&app); + let expected = app.doc.datasets[0] + .factory_pipeline(PhaseAxis::F2) + .expect("the direct axis has a factory recipe") + .steps + .iter() + .find_map(|step| match step.kind { + StepKind::Apodize(apodization) => Some(apodization), + _ => None, + }) + .expect("the time-domain factory recipe has an apodization step"); + + let kind = PropertyAddress::new(target, apodization::KIND); + assert_eq!( + app.resolve_property(&kind) + .expect("the default step resolves") + .default_value, + Some(PropertyValue::Enum(match expected { + Apodization::None => apodization::APODIZATION_NONE, + Apodization::CosineBell => apodization::APODIZATION_COSINE_BELL, + Apodization::Exponential { .. } => apodization::APODIZATION_EXPONENTIAL, + Apodization::Gaussian { .. } => apodization::APODIZATION_GAUSSIAN, + })) + ); +} + +/// Gaussian broadening is open at zero because the transform says so: the window +/// is `exp(+pi*lb*t - g*t^2)` with `g = (pi*gb)^2 / (4 ln 2)`, so `gb = 0` is +/// unbounded growth and `-gb` behaves as `+gb`. Line broadening keeps both signs, +/// which is what makes resolution enhancement reachable. +#[test] +fn gaussian_broadening_is_open_at_zero_while_line_broadening_keeps_both_signs() { + let mut app = time_domain_app(); + let target = apodization_target(&app); + let commit = app + .plan_property_write( + apodization::KIND, + std::slice::from_ref(&target), + &PropertyValue::Enum(apodization::APODIZATION_GAUSSIAN), + ) + .expect("a Gaussian window plans"); + app.commit_property(commit); + + // Switching in has to land on a value the control itself would accept. + let seeded = apodization_at(&app, &target).1; + let Apodization::Gaussian { gb_hz, .. } = seeded else { + panic!("the window is Gaussian now"); + }; + assert_eq!(gb_hz, apodization::GB_DEFAULT_HZ); + let gb = PropertyAddress::new(target.clone(), apodization::GB_HZ); + let resolved = app.resolve_property(&gb).expect("GB resolves"); + let ResolvedSchema::Float { bounds, .. } = resolved.schema else { + panic!("GB is a float"); + }; + assert!(bounds.admits(gb_hz), "the seed is inside its own bound"); + assert_eq!(resolved.default_value, Some(PropertyValue::Float(gb_hz))); + + for refused in [0.0, -1.0] { + assert!( + matches!( + app.plan_property_write( + apodization::GB_HZ, + std::slice::from_ref(&target), + &PropertyValue::Float(refused), + ), + Err(PropertyError::InvalidValue { .. }) + ), + "a Gaussian broadening of {refused} is not a window" + ); + } + let commit = app + .plan_property_write( + apodization::LB_HZ, + std::slice::from_ref(&target), + &PropertyValue::Float(-2.0), + ) + .expect("a negative line broadening stays reachable"); + app.commit_property(commit); + assert_eq!( + apodization_at(&app, &target).1, + Apodization::Gaussian { + lb_hz: -2.0, + gb_hz: apodization::GB_DEFAULT_HZ + } + ); +} + +/// A sentence a user reads names the choice the way the control names it. The +/// wire id is what the value is stored and transmitted under, and putting it in +/// prose leaks an identifier into the interface. +#[test] +fn an_unavailable_parameter_names_the_window_the_way_the_control_does() { + let app = time_domain_app(); + let target = apodization_target(&app); + let lb = PropertyAddress::new(target, apodization::LB_HZ); + let Err(PropertyError::NotApplicable(message)) = app.resolve_property(&lb) else { + panic!("the default 2D window is a cosine bell, which carries no LB"); + }; + assert!( + message.contains("Cosine bell"), + "the reason names the choice by its label: {message}" + ); + assert!( + !message.contains(apodization::APODIZATION_COSINE_BELL), + "and never by its wire id: {message}" + ); +} diff --git a/crates/core/src/properties/contour.rs b/crates/core/src/properties/contour.rs index 243d61c..fb75465 100644 --- a/crates/core/src/properties/contour.rs +++ b/crates/core/src/properties/contour.rs @@ -15,17 +15,23 @@ //! asymmetric ladder be overwritten without the user ever seeing it. use super::model::*; +use super::provider::PropertyProvider; +use super::target::{ + not_applicable_encoding, resolved_schema as standard_resolved_schema, series_context, +}; +use super::{PropertyAddress, PropertyTransaction, definition, permitted_variants, variant_list}; use crate::automation::{ CAP_FIELD_BOUNDED, CAP_FIELD_LOCATION_SCALE, CAP_FIELD_NOISE_SCALE, CAP_FIELD_SCALAR_GRID_2D_REGULAR, CAP_FIELD_SIGNED, }; use crate::state::{ CONTOUR_BASE_ABSOLUTE, CONTOUR_BASE_BACKGROUND_SCALE, CONTOUR_BASE_FRACTION_OF_RANGE, - CONTOUR_BASE_NOISE_FLOOR, PeakMagnitude, contour_base_kind, contour_base_policy, + CONTOUR_BASE_NOISE_FLOOR, PeakMagnitude, PlotxApp, contour_base_kind, contour_base_policy, + default_contour_spec, field_peak_magnitude, }; use plotx_figure::{ ColorSource, ContourBasePolicy, ContourLevelSpec, ContourSpec, PositiveFiniteF32, - PositiveFiniteF64, UnitInterval, + PositiveFiniteF64, SeriesEncoding, UnitInterval, }; pub const BASE_POLICY: PropertyId = PropertyId("series.contour.base.policy"); @@ -76,13 +82,14 @@ const SIGNED_CONTOUR: Applicability = Applicability::encoding(ComponentKind::Series, EncodingKind::Contour) .requiring(&[CAP_FIELD_SCALAR_GRID_2D_REGULAR, CAP_FIELD_SIGNED]); -pub(super) const DEFINITIONS: &[PropertyDefinition] = &[ +pub(crate) const DEFINITIONS: &[PropertyDefinition] = &[ PropertyDefinition { id: BASE_MAGNITUDE, scope_kind: ScopeKind::Object, value_schema: ValueSchema::Float { bounds: FloatBounds::above(0.0, f64::MAX), log: true, + drag_step: None, }, access: PropertyAccess::ReadWrite, applicability: CONTOUR, @@ -137,6 +144,7 @@ pub(super) const DEFINITIONS: &[PropertyDefinition] = &[ value_schema: ValueSchema::Float { bounds: RATIO_BOUNDS, log: false, + drag_step: None, }, access: PropertyAccess::ReadWrite, applicability: CONTOUR, @@ -188,6 +196,7 @@ pub(super) const DEFINITIONS: &[PropertyDefinition] = &[ value_schema: ValueSchema::Float { bounds: LINE_WIDTH_BOUNDS, log: false, + drag_step: None, }, access: PropertyAccess::ReadWrite, applicability: CONTOUR, @@ -199,6 +208,159 @@ pub(super) const DEFINITIONS: &[PropertyDefinition] = &[ }, ]; +pub(crate) struct ContourProvider; + +pub(crate) static PROVIDER: ContourProvider = ContourProvider; + +impl PropertyProvider for ContourProvider { + fn definitions(&self) -> &'static [PropertyDefinition] { + DEFINITIONS + } + + fn read( + &self, + app: &PlotxApp, + address: &PropertyAddress, + ) -> Result { + let definition = definition(address.definition).ok_or_else(|| { + PropertyError::UnknownProperty(address.definition.as_str().to_owned()) + })?; + let context = series_context(app, &address.target, definition)?; + let SeriesEncoding::Contour(spec) = context.encoding else { + return Err(not_applicable_encoding(definition, context.encoding)); + }; + let value = read(definition.id, spec) + .ok_or_else(|| PropertyError::UnknownProperty(definition.id.as_str().to_owned()))?; + let default_value = + default_value(definition, &context, spec).and_then(|value| value.uniform().copied()); + let availability = match definition.access { + PropertyAccess::ReadOnly => Availability::ReadOnly, + PropertyAccess::ReadWrite => Availability::Editable, + }; + Ok(ResolvedProperty { + address: address.clone(), + value, + default_value, + availability, + schema: resolved_schema(definition.id, spec) + .unwrap_or_else(|| standard_resolved_schema(definition, &context.capabilities)), + }) + } + + fn readout( + &self, + app: &PlotxApp, + address: &PropertyAddress, + ) -> Result { + if address.definition != BASE_MAGNITUDE { + return super::readout::uniform_readout(self.read(app, address)?); + } + // Validate the address without resolving the property's default. The + // default factory may inspect a field payload; a readout must remain a + // cache-only observation and therefore cannot call the full reader. + let definition = definition(address.definition).ok_or_else(|| { + PropertyError::UnknownProperty(address.definition.as_str().to_owned()) + })?; + let context = series_context(app, &address.target, definition)?; + if !matches!(context.encoding, SeriesEncoding::Contour(_)) { + return Err(not_applicable_encoding(definition, context.encoding)); + } + super::readout::contour_base_readout(app, &address.target) + .map(super::PropertyReadout::ContourBase) + .ok_or_else(|| { + PropertyError::NotApplicable( + "the contour base readout needs a contour series".to_owned(), + ) + }) + } + + fn edit( + &self, + app: &PlotxApp, + transaction: &mut PropertyTransaction, + address: &PropertyAddress, + operation: EditOp, + ) -> Result<(), PropertyError> { + let definition = definition(address.definition).ok_or_else(|| { + PropertyError::UnknownProperty(address.definition.as_str().to_owned()) + })?; + let context = series_context(app, &address.target, definition)?; + let SeriesEncoding::Contour(current) = context.encoding else { + return Err(not_applicable_encoding(definition, context.encoding)); + }; + let value = match operation { + EditOp::Set(value) => { + let permitted = permitted_variants(&definition.value_schema, &context.capabilities); + if let PropertyValue::Enum(choice) = value + && !permitted.iter().any(|variant| variant.id == choice) + { + return Err(PropertyError::InvalidValue { + property: definition.id, + message: format!( + "'{choice}' needs a capability this field does not expose; this field allows {}", + variant_list(&permitted) + ), + }); + } + value + } + EditOp::Reset => default_value(definition, &context, current) + .and_then(|value| value.uniform().copied()) + .ok_or(PropertyError::InvalidValue { + property: definition.id, + message: "the default factory has no single value for this setting".to_owned(), + })?, + EditOp::Step(direction) => { + let binding = transaction.data_binding(app, context.canvas, context.object)?; + let series = binding + .series + .iter_mut() + .find(|series| series.id == context.series) + .ok_or_else(|| PropertyError::UnknownTarget(address.target.describe()))?; + let SeriesEncoding::Contour(spec) = &mut series.encoding else { + return Err(PropertyError::NotApplicable( + "the series is no longer a contour".to_owned(), + )); + }; + return step(definition.id, spec, direction); + } + }; + let binding = transaction.data_binding(app, context.canvas, context.object)?; + let series = binding + .series + .iter_mut() + .find(|series| series.id == context.series) + .ok_or_else(|| PropertyError::UnknownTarget(address.target.describe()))?; + let SeriesEncoding::Contour(spec) = &mut series.encoding else { + return Err(PropertyError::NotApplicable( + "the series is no longer a contour".to_owned(), + )); + }; + write(definition.id, spec, &value, &|| { + field_peak_magnitude(context.dataset, context.field) + }) + } +} + +fn default_value( + definition: &'static PropertyDefinition, + context: &super::target::SeriesContext<'_>, + current: &ContourSpec, +) -> Option> { + match definition.default_policy { + DefaultPolicy::ProcessingFactory | DefaultPolicy::None => None, + DefaultPolicy::Fixed(value) => Some(AggregateValue::Uniform(value)), + DefaultPolicy::EncodingFactory => { + let defaults = default_contour_spec(&context.capabilities, &|| { + field_peak_magnitude(context.dataset, context.field) + }); + default_for(definition.id, &defaults, current, &|| { + field_peak_magnitude(context.dataset, context.field) + }) + } + } +} + /// Read one property out of a spec. `None` means the id is not a contour /// property; applicability has already been decided by the caller. /// diff --git a/crates/core/src/properties/line.rs b/crates/core/src/properties/line.rs new file mode 100644 index 0000000..63dbe18 --- /dev/null +++ b/crates/core/src/properties/line.rs @@ -0,0 +1,159 @@ +//! Line-encoding properties on a plot object's series. + +use super::provider::PropertyProvider; +use super::target::{SeriesContext, not_applicable_encoding, resolved_schema, series_context}; +use super::{ + AggregateValue, Applicability, Availability, ComponentKind, DefaultPolicy, EditOp, + EncodingKind, FloatBounds, PropertyAccess, PropertyAddress, PropertyDefinition, PropertyError, + PropertyId, PropertyTransaction, PropertyValue, ResolvedProperty, ScopeKind, Tier, ValueCopies, + ValueSchema, definition, +}; +use crate::state::{ + PlotxApp, PresentationProfile, RequestedChart, default_encoding, field_peak_magnitude, +}; +use plotx_figure::{PositiveFiniteF32, SeriesEncoding}; + +pub const STROKE_WIDTH: PropertyId = PropertyId("series.line.stroke_width"); + +const WIDTH_BOUNDS: FloatBounds = FloatBounds::inclusive(0.05, 10.0); +const WIDTH_STEP: f64 = 0.25; + +pub(crate) const DEFINITIONS: &[PropertyDefinition] = &[PropertyDefinition { + id: STROKE_WIDTH, + scope_kind: ScopeKind::Object, + value_schema: ValueSchema::Float { + bounds: WIDTH_BOUNDS, + log: false, + drag_step: Some(WIDTH_STEP), + }, + access: PropertyAccess::ReadWrite, + applicability: Applicability::encoding(ComponentKind::Series, EncodingKind::Line), + default_policy: DefaultPolicy::EncodingFactory, + tier: Tier::Essential, + copies: ValueCopies::PerTarget, + canonical_label: "Line stroke width", + canonical_aliases: &["line width", "stroke width", "trace thickness"], +}]; + +pub(crate) struct LineProvider; + +pub(crate) static PROVIDER: LineProvider = LineProvider; + +impl PropertyProvider for LineProvider { + fn definitions(&self) -> &'static [PropertyDefinition] { + DEFINITIONS + } + + fn read( + &self, + app: &PlotxApp, + address: &PropertyAddress, + ) -> Result { + let definition = definition(address.definition).ok_or_else(|| { + PropertyError::UnknownProperty(address.definition.as_str().to_owned()) + })?; + let context = series_context(app, &address.target, definition)?; + let SeriesEncoding::Line(line) = context.encoding else { + return Err(not_applicable_encoding(definition, context.encoding)); + }; + Ok(ResolvedProperty { + address: address.clone(), + value: AggregateValue::Uniform(PropertyValue::Float(f64::from(line.width.get()))), + default_value: factory_width(&context).map(PropertyValue::Float), + availability: Availability::Editable, + schema: resolved_schema(definition, &context.capabilities), + }) + } + + fn edit( + &self, + app: &PlotxApp, + transaction: &mut PropertyTransaction, + address: &PropertyAddress, + operation: EditOp, + ) -> Result<(), PropertyError> { + let definition = definition(address.definition).ok_or_else(|| { + PropertyError::UnknownProperty(address.definition.as_str().to_owned()) + })?; + let context = series_context(app, &address.target, definition)?; + let SeriesEncoding::Line(current) = context.encoding else { + return Err(not_applicable_encoding(definition, context.encoding)); + }; + let width = match operation { + EditOp::Set(PropertyValue::Float(value)) => { + WIDTH_BOUNDS.check(definition.id, "line width", value)? + } + EditOp::Set(value) => { + return Err(PropertyError::InvalidValue { + property: definition.id, + message: format!("expected a number, got {}", value.kind()), + }); + } + // `DefaultPolicy::EncodingFactory` is a promise about *where* the + // default comes from. Restating a literal here would keep that + // promise only until the factory started choosing a width from the + // field it is drawing. + EditOp::Reset => factory_width(&context).ok_or_else(|| { + PropertyError::NotApplicable( + "the source field is gone, so there is no factory line to rebuild".to_owned(), + ) + })?, + EditOp::Step(step) => { + let current = f64::from(current.width.get()); + let next = match step { + super::PropertyStep::Raise => (current + WIDTH_STEP).min(WIDTH_BOUNDS.max), + super::PropertyStep::Lower => (current - WIDTH_STEP).max(WIDTH_BOUNDS.min), + }; + if next == current { + return Err(PropertyError::InvalidValue { + property: definition.id, + message: format!( + "the line width is already at the {} bound ({current})", + match step { + super::PropertyStep::Raise => "upper", + super::PropertyStep::Lower => "lower", + } + ), + }); + } + next + } + }; + let width = PositiveFiniteF32::new(width as f32).ok_or(PropertyError::InvalidValue { + property: definition.id, + message: "line width must be a finite positive number".to_owned(), + })?; + let binding = transaction.data_binding(app, context.canvas, context.object)?; + let series = binding + .series + .iter_mut() + .find(|series| series.id == context.series) + .ok_or_else(|| PropertyError::UnknownTarget(address.target.describe()))?; + let SeriesEncoding::Line(line) = &mut series.encoding else { + return Err(PropertyError::NotApplicable( + "the series is no longer a line".to_owned(), + )); + }; + line.width = width; + Ok(()) + } +} + +/// The stroke width the encoding factory would give this series right now. +/// +/// Asked of the same factory that materializes a new encoding, in the target's +/// own context, which is exactly what `DefaultPolicy::EncodingFactory` declares. +fn factory_width(context: &SeriesContext<'_>) -> Option { + let descriptor = context.dataset.field_descriptor(context.field)?; + let encoding = default_encoding( + &descriptor.capabilities, + &descriptor.metadata, + RequestedChart::Line, + &PresentationProfile::default(), + &|| field_peak_magnitude(context.dataset, context.field), + ); + match encoding { + SeriesEncoding::Line(line) => Some(f64::from(line.width.get())), + _ => None, + } +} diff --git a/crates/core/src/properties/mod.rs b/crates/core/src/properties/mod.rs index c4c25aa..4818787 100644 --- a/crates/core/src/properties/mod.rs +++ b/crates/core/src/properties/mod.rs @@ -8,13 +8,22 @@ //! model. Presentation — localized labels and panel routing — belongs to the //! application crate and is keyed by the same [`PropertyId`]. +pub mod apodization; pub mod contour; +pub mod line; mod model; +mod provider; mod readout; mod service; +mod target; +mod transaction; +pub mod typography; pub use model::*; -pub use readout::{ContourAnchor, ContourBaseReadout}; +pub use readout::{ContourAnchor, ContourBaseReadout, PropertyReadout}; + +pub(crate) use provider::{PropertyProvider, PropertyProviderGroup}; +pub(crate) use transaction::PropertyTransaction; use crate::state::FieldCapabilities; use std::sync::LazyLock; @@ -22,13 +31,41 @@ use std::sync::LazyLock; /// The single aggregation point. Each area exports its own slice of /// definitions; adding one here is what makes it addressable, searchable and /// resettable everywhere at once. -static CATALOG: LazyLock> = - LazyLock::new(|| contour::DEFINITIONS.iter().collect()); +pub(crate) static GROUPS: &[PropertyProviderGroup] = &[ + PropertyProviderGroup { + provider: &apodization::PROVIDER, + }, + PropertyProviderGroup { + provider: &contour::PROVIDER, + }, + PropertyProviderGroup { + provider: &line::PROVIDER, + }, + PropertyProviderGroup { + provider: &typography::PROVIDER, + }, +]; + +static CATALOG: LazyLock> = LazyLock::new(|| { + GROUPS + .iter() + .flat_map(|group| group.provider.definitions()) + .collect() +}); pub fn catalog() -> &'static [&'static PropertyDefinition] { &CATALOG } +/// Whether a dataset holds any component this catalog can address. +/// +/// The automation resource gate asks this instead of testing what kind of data +/// the dataset holds, so admission to the `properties.*` tools stays a +/// capability question (§1 principle 3). +pub fn has_addressable_components(dataset: &crate::state::Dataset) -> bool { + target::dataset_has_property_components(dataset) +} + pub fn definition(id: PropertyId) -> Option<&'static PropertyDefinition> { catalog() .iter() @@ -45,6 +82,19 @@ pub fn definition_by_key(key: &str) -> Option<&'static PropertyDefinition> { .find(|definition| definition.id.as_str() == key) } +pub(crate) fn provider_for(id: PropertyId) -> Option<&'static dyn PropertyProvider> { + GROUPS + .iter() + .find(|group| { + group + .provider + .definitions() + .iter() + .any(|definition| definition.id == id) + }) + .map(|group| group.provider) +} + /// The enum choices a field's capabilities actually permit. Capability decides /// whether a choice can exist at all; the user's current value decides which one /// is in force. @@ -106,3 +156,7 @@ mod step_tests; #[cfg(test)] #[path = "scope_tests.rs"] mod scope_tests; + +#[cfg(test)] +#[path = "apodization_tests.rs"] +mod apodization_tests; diff --git a/crates/core/src/properties/model.rs b/crates/core/src/properties/model.rs index 757f111..31def8f 100644 --- a/crates/core/src/properties/model.rs +++ b/crates/core/src/properties/model.rs @@ -214,9 +214,27 @@ impl FloatBounds { #[derive(Clone, Copy, Debug, PartialEq)] pub enum ValueSchema { Bool, - Int { min: i64, max: i64 }, - Float { bounds: FloatBounds, log: bool }, - Enum { variants: &'static [EnumVariant] }, + Int { + min: i64, + max: i64, + }, + Float { + bounds: FloatBounds, + log: bool, + /// How far one notch of a direct-manipulation drag moves the value. + /// + /// A control that has to invent this can only derive it from the range, + /// and a range is a statement about what is *admissible*, not about what + /// is *usual*: line broadening is legal out to ±10 kHz and typically set + /// between 0.3 and 5 Hz, so a range-derived notch moves it by a hundred + /// hertz a pixel. The quantity's own scale is knowledge the definition + /// has and the control does not, so the definition states it. `None` + /// leaves the control to fall back on the range. + drag_step: Option, + }, + Enum { + variants: &'static [EnumVariant], + }, Color, } @@ -297,9 +315,13 @@ pub enum DefaultPolicy { /// Re-run the same factory that materializes a new encoding, in the /// target's current context, and read this property out of the result. EncodingFactory, + /// Re-run the typed processing recipe factory appropriate to the addressed + /// step. Unlike an encoding factory, this is owned by a dataset pipeline + /// and can vary between a 1D and a 2D default recipe. + ProcessingFactory, /// A literal that does not depend on the target. Fixed(PropertyValue), - /// Read-only values have no default to reset to. + /// Values with no meaningful reset target, normally read-only provenance. None, } @@ -341,6 +363,19 @@ impl PropertyStep { } } +/// One typed edit operation shared by all catalog entry points. +/// +/// The service owns selection-wide planning; a provider receives one target and +/// this operation, applies it to its typed working copy, or explains why that +/// target cannot accept it. Keeping the operation here prevents set/reset/step +/// from growing three structurally identical planners as new providers arrive. +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum EditOp { + Set(PropertyValue), + Reset, + Step(PropertyStep), +} + /// Which owner-local component the address must name. Field and column /// properties are addressed through their own child `ResourceRef` with no /// component at all, so they are absent here (§3.1). @@ -570,12 +605,71 @@ impl AggregateValue { } } +/// Why one target of a selection-wide read or write did nothing. +/// +/// The reason is typed because callers branch on it. "This target already holds +/// the value you asked for" is a success a caller should carry on from; "this +/// property does not apply to that target" means it addressed the wrong thing. +/// Leaving the two indistinguishable behind free text forces a caller to match +/// on prose that exists to be read by a person and is free to be reworded. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SkipReason { + /// The target already holds the requested value, so nothing was written. + AlreadyAtValue, + /// The property does not apply to this target. + NotApplicable, + /// The address no longer names anything in the document. + TargetMissing, +} + +impl SkipReason { + pub const fn as_str(self) -> &'static str { + match self { + Self::AlreadyAtValue => "already_at_value", + Self::NotApplicable => "not_applicable", + Self::TargetMissing => "target_missing", + } + } + + /// The reason a failed read or edit amounts to. + pub const fn of(error: &PropertyError) -> Self { + match error { + PropertyError::UnknownTarget(_) => Self::TargetMissing, + _ => Self::NotApplicable, + } + } +} + +/// One target a selection-wide operation passed over, carrying the reason in +/// both the form a caller branches on and the form a person reads. +#[derive(Clone, Debug, PartialEq)] +pub struct PropertySkip { + pub target: TargetRef, + pub reason: SkipReason, + pub message: String, +} + +impl PropertySkip { + pub fn new(target: TargetRef, reason: SkipReason, message: String) -> Self { + Self { + target, + reason, + message, + } + } + + /// The skip a failed read or edit amounts to, keeping the error's own words. + pub fn from_error(target: TargetRef, error: &PropertyError) -> Self { + Self::new(target, SkipReason::of(error), error.to_string()) + } +} + /// One property read across a selection. Targets the property does not apply to /// are reported with a reason rather than silently dropped. #[derive(Clone, Debug, PartialEq)] pub struct ResolvedPropertySet { pub applicable_targets: Vec, - pub skipped_targets: Vec<(TargetRef, String)>, + pub skipped_targets: Vec, pub value: AggregateValue, } @@ -586,7 +680,7 @@ pub struct ResolvedPropertySet { pub struct PropertyCommit { pub action: crate::actions::Action, pub applied: Vec, - pub skipped: Vec<(TargetRef, String)>, + pub skipped: Vec, } impl fmt::Debug for PropertyCommit { diff --git a/crates/core/src/properties/provider.rs b/crates/core/src/properties/provider.rs new file mode 100644 index 0000000..c6c5d2c --- /dev/null +++ b/crates/core/src/properties/provider.rs @@ -0,0 +1,54 @@ +//! The narrow dispatch seam between the catalog service and one property's +//! owning domain model. +//! +//! A provider owns one target at a time: it resolves its canonical state, +//! validates an operation, and asks the transaction for the storage it owns. +//! Selection aggregation, skipped-target reporting, and atomic commits remain +//! in `service`, so providers cannot grow parallel planners. + +use super::{ + EditOp, PropertyAddress, PropertyDefinition, PropertyError, PropertyReadout, + PropertyTransaction, ResolvedProperty, +}; +use crate::state::PlotxApp; + +/// The implementation seam for one registered family of properties. +/// +/// This is deliberately crate-private. It makes existing domain providers +/// explicit without turning the catalog into a third-party plugin system. +pub(crate) trait PropertyProvider: Sync { + fn definitions(&self) -> &'static [PropertyDefinition]; + + fn read( + &self, + app: &PlotxApp, + address: &PropertyAddress, + ) -> Result; + + fn edit( + &self, + app: &PlotxApp, + transaction: &mut PropertyTransaction, + address: &PropertyAddress, + operation: EditOp, + ) -> Result<(), PropertyError>; + + /// Return the value one canvas or panel label should show for this exact + /// address. The normal case is the resolved scalar; providers with cached + /// semantic context may override it without adding an encoding branch to + /// the service. + fn readout( + &self, + app: &PlotxApp, + address: &PropertyAddress, + ) -> Result { + super::readout::uniform_readout(self.read(app, address)?) + } +} + +/// One explicit registration entry. Adding a property family means adding its +/// module and exactly one item to `GROUPS`; `catalog()` derives all definitions +/// from this slice. +pub(crate) struct PropertyProviderGroup { + pub provider: &'static dyn PropertyProvider, +} diff --git a/crates/core/src/properties/provider_tests.rs b/crates/core/src/properties/provider_tests.rs new file mode 100644 index 0000000..3a747ee --- /dev/null +++ b/crates/core/src/properties/provider_tests.rs @@ -0,0 +1,216 @@ +//! Cross-scope provider slices that would otherwise make the contour fixture +//! module too large. + +use super::*; + +/// The document scope is not a disguised canvas-object scope. Typography can +/// be addressed before a page or plot exists, and its provider compiles the +/// edit to the document action that already owns undo/rebuild semantics. +#[test] +fn document_typography_is_addressable_without_a_canvas_object() { + let mut app = PlotxApp::new(); + let target = app.document_target(); + assert!(target.component.is_none()); + assert_eq!(target.resource.kind.0, "plotx.document"); + + let address = PropertyAddress::new(target.clone(), typography::TICK_PT); + let before = app + .resolve_property(&address) + .expect("the document resolves"); + assert_eq!( + before.value, + AggregateValue::Uniform(PropertyValue::Float(7.0)) + ); + assert_eq!(before.default_value, Some(PropertyValue::Float(7.0))); + + let commit = app + .plan_property_write( + typography::TICK_PT, + std::slice::from_ref(&target), + &PropertyValue::Float(9.5), + ) + .expect("the document property plans"); + let Action::Composite(actions) = &commit.action else { + panic!("a catalog commit is composite"); + }; + assert!(matches!( + actions.as_slice(), + [Action::SetFigureTypography { .. }] + )); + assert_eq!(app.commit_property(commit), 1); + assert_eq!(app.doc.style_library.figure_typography.tick_pt, 9.5); +} + +/// A write of the value already in typed storage is not an applied edit. The +/// caller gets an explicit skip, and the empty composite cannot create a fake +/// undo/revision entry. +#[test] +fn a_same_value_write_is_reported_without_an_empty_commit() { + let mut app = PlotxApp::new(); + let target = app.document_target(); + let commit = app + .plan_property_write( + typography::TICK_PT, + std::slice::from_ref(&target), + &PropertyValue::Float(7.0), + ) + .expect("the existing value is a valid request"); + assert!( + commit.applied.is_empty(), + "a no-op is not reported as applied" + ); + assert_eq!(commit.skipped.len(), 1); + assert!(commit.skipped[0].message.contains("already has that value")); + let Action::Composite(actions) = &commit.action else { + panic!("a no-op still has the catalog composite shape"); + }; + assert!( + actions.is_empty(), + "the transaction contains no typed action" + ); + let revision = app.doc.automation_revision; + assert_eq!(app.commit_property(commit), 0); + assert_eq!( + app.doc.automation_revision, revision, + "a no-op does not create an automation revision" + ); +} + +/// A heterogeneous plot selection must retain both facts: the two line series +/// disagree (`Mixed`), and a contour in the same selection was not silently +/// treated as a line. Compatible targets still compile to one atomic composite. +#[test] +fn line_stroke_width_reports_mixed_values_and_skips_other_encodings() { + let (mut app, contour) = contour_app(); + app.doc + .datasets + .push(Dataset::Nmr(Box::new(NmrDataset::load(nmr1d_with( + "lines", + ))))); + let mut line_targets = Vec::new(); + for name in ["Line A", "Line B"] { + let id = app.doc.canvases[0].allocate_object_id(); + let object = app.build_plot_object( + 1, + ObjectFrame::new(0.0, 0.0, 100.0, 40.0), + id, + name.to_owned(), + ); + app.doc.canvases[0].objects.push(object); + line_targets.push( + app.series_targets(0, id) + .into_iter() + .next() + .expect("one line series"), + ); + } + let second = line_targets[1].clone(); + let object: crate::state::ObjectId = second + .resource + .local_id + .as_deref() + .expect("object id") + .parse() + .expect("object id parses"); + let Some(ComponentRef::Series(series)) = second.component else { + panic!("line target addresses its series"); + }; + let line = app.doc.canvases[0] + .object_mut(object) + .and_then(|object| object.plot_mut()) + .and_then(|plot| { + plot.binding + .series + .iter_mut() + .find(|candidate| candidate.id == series) + }) + .expect("second line series"); + let plotx_figure::SeriesEncoding::Line(line) = &mut line.encoding else { + panic!("the 1D field materializes a line encoding"); + }; + line.width = plotx_figure::PositiveFiniteF32::new(2.0).expect("literal width"); + + let targets = vec![ + line_targets[0].clone(), + line_targets[1].clone(), + contour.clone(), + ]; + let set = app.resolve_property_set(line::STROKE_WIDTH, &targets); + assert_eq!(set.value, AggregateValue::Mixed); + assert_eq!(set.applicable_targets.len(), 2); + assert_eq!(set.skipped_targets.len(), 1); + assert!( + set.skipped_targets[0].message.contains("line") + && set.skipped_targets[0].message.contains("contour"), + "the incompatible target is named rather than discarded: {}", + set.skipped_targets[0].message + ); + + let commit = app + .plan_property_write(line::STROKE_WIDTH, &targets, &PropertyValue::Float(3.0)) + .expect("the compatible line series plan together"); + assert_eq!(commit.applied.len(), 2); + assert_eq!(commit.skipped.len(), 1); + let Action::Composite(actions) = &commit.action else { + panic!("a multi-object edit is composite"); + }; + assert_eq!( + actions.len(), + 2, + "each line object keeps its typed binding action" + ); + assert!( + actions + .iter() + .all(|action| matches!(action, Action::SetDataBinding { .. })) + ); + app.commit_property(commit); + for target in line_targets { + let resolved = app + .resolve_property(&PropertyAddress::new(target, line::STROKE_WIDTH)) + .expect("line width still resolves"); + assert_eq!( + resolved.value, + AggregateValue::Uniform(PropertyValue::Float(3.0)) + ); + } + assert!( + matches!( + app.resolve_property(&PropertyAddress::new(contour, line::STROKE_WIDTH)), + Err(PropertyError::NotApplicable(_)) + ), + "the contour did not receive a line-only write" + ); +} + +/// A second steppable encoding reads through the property-address dispatch, not +/// through the old contour-only target lookup. Its ordinary scalar payload is +/// deliberately distinct from the contour provider's richer semantic payload. +#[test] +fn a_line_readout_dispatches_by_property_address() { + let (mut app, _) = contour_app(); + app.doc + .datasets + .push(Dataset::Nmr(Box::new(NmrDataset::load(nmr1d_with( + "line readout", + ))))); + let id = app.doc.canvases[0].allocate_object_id(); + let object = app.build_plot_object( + 1, + ObjectFrame::new(0.0, 0.0, 100.0, 40.0), + id, + "Line".to_owned(), + ); + app.doc.canvases[0].objects.push(object); + let target = app + .series_targets(0, id) + .into_iter() + .next() + .expect("one line series"); + + assert_eq!( + app.property_readout(&PropertyAddress::new(target, line::STROKE_WIDTH)) + .expect("the line readout resolves"), + PropertyReadout::Value(PropertyValue::Float(1.0)) + ); +} diff --git a/crates/core/src/properties/readout.rs b/crates/core/src/properties/readout.rs index 2da3161..8df3ff7 100644 --- a/crates/core/src/properties/readout.rs +++ b/crates/core/src/properties/readout.rs @@ -17,6 +17,8 @@ //! is exactly the coupling the field runtime exists to prevent. use super::contour; +use super::target::series_context_unchecked; +use super::{PropertyError, PropertyValue, ResolvedProperty}; use crate::automation::TargetRef; use crate::state::{ ContourResolution, EstimateKey, EstimateKind, EstimateResult, EstimatedScale, FieldRef, @@ -25,6 +27,35 @@ use crate::state::{ }; use plotx_figure::{ContourBasePolicy, SeriesEncoding, UnitInterval}; +/// A display-oriented value belonging to one addressed property. +/// +/// Most settings need only their resolved scalar. Providers with a value whose +/// meaning depends on cached scientific state can return a richer variant +/// without teaching the service which encoding owns it. +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum PropertyReadout { + Value(PropertyValue), + ContourBase(ContourBaseReadout), +} + +/// Turn an ordinary resolved value into a readout. +/// +/// A display readout cannot invent one value for a property whose own copies +/// disagree. Callers get an explicit error and can leave the label absent +/// rather than rendering an arbitrary target as the selection's value. +pub(crate) fn uniform_readout( + resolved: ResolvedProperty, +) -> Result { + let property = resolved.address.definition; + let value = resolved.value.uniform().copied().ok_or_else(|| { + PropertyError::NotApplicable(format!( + "{} has no single value to show in a readout", + property.as_str() + )) + })?; + Ok(PropertyReadout::Value(value)) +} + /// Whether the number a user set can currently be turned into a level, and why /// not when it cannot. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -77,114 +108,118 @@ pub struct ContourBaseReadout { pub anchor: ContourAnchor, } -impl PlotxApp { - /// Read what one contour series' lowest level currently means. - /// - /// Returns `None` when the target is not a contour series at all. A target - /// that is one but whose estimate has not arrived returns a readout with - /// [`ContourAnchor::Measuring`]: the difference between "no such setting" - /// and "not measured yet" is one the interface has to be able to state. - pub fn contour_base_readout(&self, target: &TargetRef) -> Option { - let context = self.series_context_unchecked(target).ok()?; - let SeriesEncoding::Contour(spec) = context.encoding else { - return None; - }; - let source = VersionedFieldRef { - field: FieldRef { - resource: context.dataset.resource_id(), - field: context.field, +/// Read what one contour series' lowest level currently means. +/// +/// This is the contour provider's specialization of the generic property +/// readout. It remains cache-only: a target whose estimate has not arrived +/// returns [`ContourAnchor::Measuring`] rather than starting work. +pub(crate) fn contour_base_readout( + app: &PlotxApp, + target: &TargetRef, +) -> Option { + let context = series_context_unchecked(app, target).ok()?; + let SeriesEncoding::Contour(spec) = context.encoding else { + return None; + }; + let source = VersionedFieldRef { + field: FieldRef { + resource: context.dataset.resource_id(), + field: context.field, + }, + // A field with no runtime version has never been drawn, so nothing + // about it is cached. Minting one here would be this module + // allocating session state on behalf of a label. + version: app.session.compute.current_field_version(FieldRef { + resource: context.dataset.resource_id(), + field: context.field, + })?, + }; + let summary = app.session.compute.peek_field_summary(source); + let anchor = match &spec.positive.base { + ContourBasePolicy::Absolute(_) | ContourBasePolicy::FractionOfRange(_) => { + ContourAnchor::Direct + } + ContourBasePolicy::NoiseFloor { + peak_fraction, + estimator, + .. + } => floored_anchor_of( + app, + &EstimateKey { + source, + kind: EstimateKind::Noise, + estimator: estimator.clone(), }, - // A field with no runtime version has never been drawn, so nothing - // about it is cached. Minting one here would be this module - // allocating session state on behalf of a label. - version: self.session.compute.current_field_version(FieldRef { - resource: context.dataset.resource_id(), - field: context.field, - })?, - }; - let summary = self.session.compute.peek_field_summary(source); - let anchor = match &spec.positive.base { - ContourBasePolicy::Absolute(_) | ContourBasePolicy::FractionOfRange(_) => { - ContourAnchor::Direct - } - ContourBasePolicy::NoiseFloor { - peak_fraction, - estimator, - .. - } => self.floored_anchor_of( - &EstimateKey { - source, - kind: EstimateKind::Noise, - estimator: estimator.clone(), - }, - *peak_fraction, - summary, - ), - ContourBasePolicy::BackgroundScale { estimator, .. } => self.anchor_of(&EstimateKey { + *peak_fraction, + summary, + ), + ContourBasePolicy::BackgroundScale { estimator, .. } => anchor_of( + app, + &EstimateKey { source, kind: EstimateKind::Background, estimator: estimator.clone(), - }), - }; - // Resolution is pure arithmetic over a cached summary and cached - // estimates; the payload is never touched. A miss simply yields - // `Pending`, which is reported rather than acted on. - let lowest_level = summary.and_then(|summary| { - match resolve_contour_levels(source, spec, summary, |key| { - self.session.compute.peek_estimate(key).cloned() - }) { - ContourResolution::Ready { levels, .. } => { - levels.positive.first().map(|level| level.get()) - } - ContourResolution::Pending(_) | ContourResolution::Unavailable => None, + }, + ), + }; + // Resolution is pure arithmetic over a cached summary and cached + // estimates; the payload is never touched. A miss simply yields + // `Pending`, which is reported rather than acted on. + let lowest_level = summary.and_then(|summary| { + match resolve_contour_levels(source, spec, summary, |key| { + app.session.compute.peek_estimate(key).cloned() + }) { + ContourResolution::Ready { levels, .. } => { + levels.positive.first().map(|level| level.get()) } - }); - Some(ContourBaseReadout { - kind: contour_base_kind(&spec.positive.base), - magnitude: contour::base_magnitude(&spec.positive.base), - lowest_level, - peak_fraction: contour::base_peak_fraction(&spec.positive.base), - anchor, - }) - } - - /// Classify a floored noise anchor: whether the estimate or the floor is the - /// scale in force right now. - /// - /// Without a cached summary the floor cannot be compared against anything, - /// and answering `Measured` would assert the estimate is in force when it - /// may not be. A field with no cached summary has never been drawn, so - /// reporting the measurement as still outstanding is both true and the only - /// thing this read-only module may do about it. - fn floored_anchor_of( - &self, - key: &EstimateKey, - peak_fraction: UnitInterval, - summary: Option, - ) -> ContourAnchor { - let (Some(EstimateResult::Scale(estimate)), Some(summary)) = - (self.session.compute.peek_estimate(key), summary) - else { - return ContourAnchor::Measuring; - }; - match resolved_noise_scale(estimate.scale, peak_fraction, summary) { - // Neither term measured anything: a flat field with no peak either. - (scale, _) if scale <= 0.0 => ContourAnchor::Degenerate, - (_, NoiseScaleTerm::PeakFloor) => ContourAnchor::Floored, - (_, NoiseScaleTerm::Estimated) => ContourAnchor::Measured, + ContourResolution::Pending(_) | ContourResolution::Unavailable => None, } + }); + Some(ContourBaseReadout { + kind: contour_base_kind(&spec.positive.base), + magnitude: contour::base_magnitude(&spec.positive.base), + lowest_level, + peak_fraction: contour::base_peak_fraction(&spec.positive.base), + anchor, + }) +} + +/// Classify a floored noise anchor: whether the estimate or the floor is the +/// scale in force right now. +/// +/// Without a cached summary the floor cannot be compared against anything, and +/// answering `Measured` would assert the estimate is in force when it may not +/// be. A field with no cached summary has never been drawn, so reporting the +/// measurement as outstanding is both true and the only thing this read-only +/// module may do about it. +fn floored_anchor_of( + app: &PlotxApp, + key: &EstimateKey, + peak_fraction: UnitInterval, + summary: Option, +) -> ContourAnchor { + let (Some(EstimateResult::Scale(estimate)), Some(summary)) = + (app.session.compute.peek_estimate(key), summary) + else { + return ContourAnchor::Measuring; + }; + match resolved_noise_scale(estimate.scale, peak_fraction, summary) { + // Neither term measured anything: a flat field with no peak either. + (scale, _) if scale <= 0.0 => ContourAnchor::Degenerate, + (_, NoiseScaleTerm::PeakFloor) => ContourAnchor::Floored, + (_, NoiseScaleTerm::Estimated) => ContourAnchor::Measured, } +} - fn anchor_of(&self, key: &EstimateKey) -> ContourAnchor { - let scale = match self.session.compute.peek_estimate(key) { - None => return ContourAnchor::Measuring, - Some(EstimateResult::Scale(estimate)) => estimate.scale, - Some(EstimateResult::LocationScale(estimate)) => estimate.scale, - }; - match scale { - EstimatedScale::Positive(_) => ContourAnchor::Measured, - EstimatedScale::Degenerate => ContourAnchor::Degenerate, - } +fn anchor_of(app: &PlotxApp, key: &EstimateKey) -> ContourAnchor { + let scale = match app.session.compute.peek_estimate(key) { + None => return ContourAnchor::Measuring, + Some(EstimateResult::Scale(estimate)) => estimate.scale, + Some(EstimateResult::LocationScale(estimate)) => estimate.scale, + }; + match scale { + EstimatedScale::Positive(_) => ContourAnchor::Measured, + EstimatedScale::Degenerate => ContourAnchor::Degenerate, } } diff --git a/crates/core/src/properties/readout_tests.rs b/crates/core/src/properties/readout_tests.rs index 64c4bc1..195ed50 100644 --- a/crates/core/src/properties/readout_tests.rs +++ b/crates/core/src/properties/readout_tests.rs @@ -6,7 +6,9 @@ use crate::contour_probe; use crate::properties::tests::contour_app; -use crate::properties::{ContourAnchor, PropertyValue, contour}; +use crate::properties::{ + ContourAnchor, ContourBaseReadout, PropertyAddress, PropertyReadout, PropertyValue, contour, +}; use crate::state::{ CONTOUR_BASE_ABSOLUTE, CONTOUR_BASE_NOISE_FLOOR, ChartSpec, DataBinding, DataDomain, PlotxApp, StackSpec, @@ -23,6 +25,20 @@ fn binding(app: &PlotxApp) -> DataBinding { .clone() } +/// Read the contour's semantic payload through the generic per-property seam. +fn contour_readout(app: &PlotxApp, target: &crate::automation::TargetRef) -> ContourBaseReadout { + match app + .property_readout(&PropertyAddress::new( + target.clone(), + contour::BASE_MAGNITUDE, + )) + .expect("the fixture draws a contour") + { + PropertyReadout::ContourBase(readout) => readout, + PropertyReadout::Value(value) => panic!("expected a contour payload, got {value:?}"), + } +} + /// Draw the fixture's plot once, the way a frame does. fn rebuild(app: &mut PlotxApp) { let binding = binding(app); @@ -59,9 +75,7 @@ fn an_unmeasured_anchor_reads_as_measuring_without_queueing_anything() { let (app, target) = contour_app(); contour_probe::reset(); - let readout = app - .contour_base_readout(&target) - .expect("the fixture draws a contour"); + let readout = contour_readout(&app, &target); assert_eq!(readout.kind, CONTOUR_BASE_NOISE_FLOOR); assert_eq!(readout.anchor, ContourAnchor::Measuring); @@ -81,9 +95,7 @@ fn a_cached_estimate_resolves_the_level_and_reading_it_stays_free() { warm(&mut app); contour_probe::reset(); - let readout = app - .contour_base_readout(&target) - .expect("the fixture draws a contour"); + let readout = contour_readout(&app, &target); assert_eq!(readout.anchor, ContourAnchor::Measured); assert_eq!(readout.kind, CONTOUR_BASE_NOISE_FLOOR); @@ -95,7 +107,7 @@ fn a_cached_estimate_resolves_the_level_and_reading_it_stays_free() { ); for _ in 0..8 { - assert_eq!(app.contour_base_readout(&target), Some(readout)); + assert_eq!(contour_readout(&app, &target), readout); } assert_eq!( contour_probe::queued_estimates(), @@ -131,9 +143,7 @@ fn a_field_whose_sigma_falls_under_the_floor_reads_as_floored() { warm(&mut app); contour_probe::reset(); - let readout = app - .contour_base_readout(&target) - .expect("the fixture draws a contour"); + let readout = contour_readout(&app, &target); assert_eq!(readout.kind, CONTOUR_BASE_NOISE_FLOOR); assert_eq!( @@ -168,7 +178,7 @@ fn a_directly_anchored_base_needs_no_estimate() { warm(&mut app); contour_probe::reset(); - let readout = app.contour_base_readout(&target).expect("still a contour"); + let readout = contour_readout(&app, &target); assert_eq!(readout.anchor, ContourAnchor::Direct); assert_eq!(contour_probe::field_payload_materializations(), 0); diff --git a/crates/core/src/properties/scope_tests.rs b/crates/core/src/properties/scope_tests.rs index 8e1da20..67c438f 100644 --- a/crates/core/src/properties/scope_tests.rs +++ b/crates/core/src/properties/scope_tests.rs @@ -85,6 +85,19 @@ fn stacked_app() -> (PlotxApp, TargetRef, TargetRef) { fn resetting_one_encoding_leaves_a_stacked_encoding_untouched() { let (mut app, contour, heatmap) = stacked_app(); let before = encoding_of(&app, &heatmap); + // Move the contour off its factory encoding first, so the reset has + // something to do. A reset that finds a series already at its default now + // reports a skip like every other control in the section, and the scope this + // test is about would be invisible behind that. + let levels = contour_spec(&app, &contour).positive.count; + let commit = app + .plan_property_write( + contour::COUNT, + std::slice::from_ref(&contour), + &PropertyValue::Int(i64::from(levels) + 1), + ) + .expect("the level count is writable"); + app.commit_property(commit); let commit = app .plan_encoding_reset(EncodingKind::Contour, &[contour.clone(), heatmap.clone()]) @@ -101,9 +114,9 @@ fn resetting_one_encoding_leaves_a_stacked_encoding_untouched() { "the series outside the scope is reported, never silently included" ); assert!( - commit.skipped[0].1.contains("heatmap"), + commit.skipped[0].message.contains("heatmap"), "the reason names what the series actually draws: {}", - commit.skipped[0].1 + commit.skipped[0].message ); app.commit_property(commit); @@ -379,3 +392,39 @@ fn applicability_names_the_contour_under_a_heatmap_drawn_first() { "only the series that draws a contour has a lowest contour level" ); } + +/// "Reset contour" is a control in the same section as the individual settings, +/// and has to answer the same way they do. Reporting "Updated 1 contour series." +/// for a series that was already at its factory encoding told the user something +/// happened when nothing did. +#[test] +fn resetting_an_encoding_that_is_already_the_factorys_reports_a_skip() { + let (mut app, contour) = contour_app(); + let commit = app + .plan_encoding_reset(EncodingKind::Contour, std::slice::from_ref(&contour)) + .expect("the reset plans"); + assert!( + commit.applied.is_empty(), + "a fresh series is what the factory produces: {:?}", + commit.applied + ); + assert_eq!(commit.skipped.len(), 1); + assert_eq!(commit.skipped[0].reason, SkipReason::AlreadyAtValue); + assert_eq!(app.commit_property(commit), 0); + + // And it still applies once the series has actually moved. + let levels = contour_spec(&app, &contour).positive.count; + let commit = app + .plan_property_write( + contour::COUNT, + std::slice::from_ref(&contour), + &PropertyValue::Int(i64::from(levels) + 1), + ) + .expect("the level count is writable"); + app.commit_property(commit); + let commit = app + .plan_encoding_reset(EncodingKind::Contour, std::slice::from_ref(&contour)) + .expect("the reset plans"); + assert_eq!(commit.applied.len(), 1); + assert!(commit.skipped.is_empty()); +} diff --git a/crates/core/src/properties/service.rs b/crates/core/src/properties/service.rs index 3a5b9af..16281f2 100644 --- a/crates/core/src/properties/service.rs +++ b/crates/core/src/properties/service.rs @@ -1,41 +1,42 @@ -//! Reading and writing catalog properties against the live document. +//! Selection-wide property planning and commit execution. //! -//! One planner serves every entry point. A UI control, and later an automation -//! tool, both hand a typed [`PropertyValue`] to [`PlotxApp::plan_property_write`] -//! and receive a single atomic [`crate::actions::Action`] plus an explicit list -//! of the targets that were skipped and why. +//! Providers own one addressed target. This service is intentionally the only +//! place that loops over targets, reports skips, and produces an atomic action; +//! duplicating those responsibilities in each provider would create a second +//! planner for every property family. -use super::model::*; -use super::{contour, definition, permitted_variants, variant_list}; -use crate::actions::Action; +use super::target::{ + canvas_object, document_target, processing_step_targets, series_context_unchecked, + series_targets, +}; +use super::{ + AggregateValue, ComponentKind, EditOp, EncodingKind, PropertyAccess, PropertyAddress, + PropertyCommit, PropertyDefinition, PropertyError, PropertyId, PropertySkip, PropertyStep, + PropertyTransaction, PropertyValue, ResolvedProperty, ResolvedPropertySet, SkipReason, + definition, provider_for, +}; +use crate::actions::{Action, DatasetProcessingState, PendingPropertyGesture}; use crate::automation::{ComponentRef, ResourceRef, TargetRef, canvas_object_ref}; use crate::state::{ - CanvasId, DataBinding, Dataset, FieldCapabilities, FieldId, ObjectId, PlotxApp, - PresentationProfile, RequestedChart, SeriesId, default_contour_spec, default_encoding, + DatasetId, ObjectId, PlotxApp, PresentationProfile, RequestedChart, default_encoding, field_peak_magnitude, }; use plotx_figure::SeriesEncoding; -/// A target resolved down to the one series it names, plus everything -/// applicability and defaults need. Indices here are one-shot lookup positions -/// and never leave this struct. -pub(super) struct SeriesContext<'a> { - canvas: usize, - object: ObjectId, - series: SeriesId, - pub(super) dataset: &'a Dataset, - pub(super) field: FieldId, - capabilities: FieldCapabilities, - pub(super) encoding: &'a SeriesEncoding, -} - impl PlotxApp { - /// The address of one contour-style property on one series of a plot. + /// The target of a document-owned property. It deliberately has no canvas + /// or object component: figure typography belongs to the document even + /// when the document currently contains no pages. + pub fn document_target(&self) -> TargetRef { + document_target() + } + + /// The address of one series-owned property on one plot object. pub fn series_target( &self, canvas: usize, object: ObjectId, - series: SeriesId, + series: crate::state::SeriesId, ) -> Option { let canvas_id = self.doc.canvases.get(canvas)?.resource_id; Some(TargetRef { @@ -46,73 +47,56 @@ impl PlotxApp { /// Every series of one plot object, in binding order. pub fn series_targets(&self, canvas: usize, object: ObjectId) -> Vec { - let Some(canvas_document) = self.doc.canvases.get(canvas) else { - return Vec::new(); - }; - let resource = canvas_object_ref(canvas_document.resource_id, object); - canvas_document - .object(object) - .and_then(|object| object.plot()) - .map(|plot| { - plot.binding - .series - .iter() - .map(|series| TargetRef { - resource: resource.clone(), - component: Some(ComponentRef::Series(series.id)), - }) - .collect() - }) - .unwrap_or_default() + series_targets(self, canvas, object) } - /// Every series component of one plot-object *resource*. + /// Every series component of one plot-object resource. /// - /// This is the expansion an automation plan performs: a selector names - /// resources, and a property whose scope is a series has to become one - /// target per series before anything can be said about applicability. It - /// resolves the reference through the same lookup a property write does, so - /// a plan and the commit that follows it cannot disagree about which - /// components exist. + /// Automation selectors choose resources. A provider definition then + /// determines the component shape the resource expands to; series are the + /// only shape implemented at this point. pub fn resource_series_targets(&self, resource: &ResourceRef) -> Vec { - self.canvas_object(resource) + canvas_object(self, resource) .map(|(canvas, object)| self.series_targets(canvas, object)) .unwrap_or_default() } - /// Read one property against one target. + /// Every processing-step component of one dataset resource. + pub fn resource_processing_step_targets(&self, resource: &ResourceRef) -> Vec { + processing_step_targets(self, resource) + } + + /// Expand one resource according to a definition's component shape. + /// + /// This is target-shape dispatch only. Providers still choose typed storage + /// through [`PropertyTransaction`]; the service never chooses storage by + /// scope kind. + pub fn resource_property_targets( + &self, + resource: &ResourceRef, + definition: &'static PropertyDefinition, + ) -> Vec { + match definition.applicability.component { + ComponentKind::None => vec![TargetRef::resource(resource.clone())], + ComponentKind::Series => self.resource_series_targets(resource), + ComponentKind::ProcessingStep => self.resource_processing_step_targets(resource), + } + } + + /// Read one property against one target through its registered provider. pub fn resolve_property( &self, address: &PropertyAddress, ) -> Result { - let definition = self.definition_for(address.definition)?; - let context = self.series_context(&address.target, definition)?; - let SeriesEncoding::Contour(spec) = context.encoding else { - return Err(not_applicable_encoding(definition, context.encoding)); - }; - let value = contour::read(definition.id, spec) - .ok_or_else(|| PropertyError::UnknownProperty(definition.id.as_str().to_owned()))?; - let default_value = match definition.default_policy { - DefaultPolicy::None => None, - DefaultPolicy::Fixed(value) => Some(value), - DefaultPolicy::EncodingFactory => self.factory_default(definition.id, &context), - }; - let availability = match definition.access { - PropertyAccess::ReadOnly => Availability::ReadOnly, - PropertyAccess::ReadWrite => Availability::Editable, - }; - Ok(ResolvedProperty { - address: address.clone(), - value, - default_value, - availability, - schema: contour::resolved_schema(definition.id, spec) - .unwrap_or_else(|| resolved_schema(definition, &context.capabilities)), - }) + self.definition_for(address.definition)?; + let provider = provider_for(address.definition).ok_or_else(|| { + PropertyError::UnknownProperty(address.definition.as_str().to_owned()) + })?; + provider.read(self, address) } /// Read one property across a selection, reporting both the aggregate value - /// and every target the property does not apply to. + /// and every target that cannot supply the property. pub fn resolve_property_set( &self, property: PropertyId, @@ -120,10 +104,6 @@ impl PlotxApp { ) -> ResolvedPropertySet { let mut applicable_targets = Vec::new(); let mut skipped_targets = Vec::new(); - // Each target already answers with an aggregate over its own copies of - // the setting, so folding the selection in with the same operation makes - // an asymmetric ladder inside one target and a disagreement between two - // targets compose rather than mask each other. let mut value = AggregateValue::Unavailable; for target in targets { let address = PropertyAddress::new(target.clone(), property); @@ -132,7 +112,9 @@ impl PlotxApp { value = value.merge(resolved.value); applicable_targets.push(address); } - Err(error) => skipped_targets.push((target.clone(), error.to_string())), + Err(error) => { + skipped_targets.push(PropertySkip::from_error(target.clone(), &error)) + } } } ResolvedPropertySet { @@ -142,128 +124,78 @@ impl PlotxApp { } } - /// Validate a write against every applicable target and fold it into one - /// atomic action. Nothing is executed here, and a single validation failure - /// aborts the whole commit rather than landing on part of the selection. + /// Read one property's display value through its owning provider. + /// + /// This is deliberately address-based rather than encoding-based: a new + /// steppable encoding gets a readout by registering its provider, not by + /// adding another `SeriesEncoding` branch to UI callers. + pub fn property_readout( + &self, + address: &PropertyAddress, + ) -> Result { + self.definition_for(address.definition)?; + let provider = provider_for(address.definition).ok_or_else(|| { + PropertyError::UnknownProperty(address.definition.as_str().to_owned()) + })?; + provider.readout(self, address) + } + + /// Validate a set operation across every applicable target and fold the + /// result into one atomic action. pub fn plan_property_write( &self, property: PropertyId, targets: &[TargetRef], value: &PropertyValue, ) -> Result { - let definition = self.definition_for(property)?; - if definition.access == PropertyAccess::ReadOnly { - return Err(PropertyError::ReadOnly(property)); - } - self.plan(definition, targets, &mut |spec, context| { - let permitted = permitted_variants(&definition.value_schema, &context.capabilities); - if let PropertyValue::Enum(choice) = value - && !permitted.iter().any(|variant| variant.id == *choice) - { - // Name what this field does allow. A caller told only that its - // choice is unavailable has to guess the next one, and the - // permitted set is a fact about the target's capabilities that - // no caller can derive from the static schema. - return Err(PropertyError::InvalidValue { - property, - message: format!( - "'{choice}' needs a capability this field does not expose; this field allows {}", - variant_list(&permitted) - ), - }); - } - contour::write(property, spec, value, &|| { - field_peak_magnitude(context.dataset, context.field) - }) - }) + self.plan_edit(property, targets, EditOp::Set(*value)) } - /// Move one property one step along its own scale, from whatever each - /// target currently holds. - /// - /// This is the entry point for direct-manipulation gestures. It does not - /// compute a value somewhere else and hand it over: the gesture names a - /// direction, and the step is taken inside the same planner, against the - /// same working copy, and validated by the same rules as a value typed into - /// the panel. A gesture therefore cannot become a second source of state, - /// and cannot reach a value the panel would have rejected. + /// Move a property along the scale its provider owns. pub fn plan_property_step( &self, property: PropertyId, targets: &[TargetRef], step: PropertyStep, ) -> Result { - let definition = self.definition_for(property)?; - if definition.access == PropertyAccess::ReadOnly { - return Err(PropertyError::ReadOnly(property)); - } - self.plan(definition, targets, &mut |spec, _context| { - contour::step(property, spec, step) - }) + self.plan_edit(property, targets, EditOp::Step(step)) } - /// Reset one property by re-deriving it from the default policy in each - /// target's current context. + /// Reset one property in each target's current context. pub fn plan_property_reset( &self, property: PropertyId, targets: &[TargetRef], ) -> Result { - let definition = self.definition_for(property)?; - if definition.access == PropertyAccess::ReadOnly { - return Err(PropertyError::ReadOnly(property)); - } - self.plan(definition, targets, &mut |spec, context| { - let value = match definition.default_policy { - DefaultPolicy::Fixed(value) => value, - DefaultPolicy::None => return Err(PropertyError::ReadOnly(property)), - DefaultPolicy::EncodingFactory => { - self.factory_default(property, context) - .ok_or(PropertyError::InvalidValue { - property, - message: "the default factory has no single value for this setting" - .to_owned(), - })? - } - }; - contour::write(property, spec, &value, &|| { - field_peak_magnitude(context.dataset, context.field) - }) - }) + self.plan_edit(property, targets, EditOp::Reset) } - /// Reset a whole encoding by calling the default factory again, replacing it - /// with a freshly materialized concrete encoding. + /// Reset a complete encoding through its existing default factory. /// - /// The caller names *which* encoding it is resetting, and a target drawn as - /// anything else is skipped with a reason rather than rebuilt. A property - /// write gets that filter for free — a control belongs to a definition, and - /// the definition's applicability skips whatever it does not describe — but - /// an encoding reset has no property to compare against, so the scope has to - /// be part of the request. Without it, a reset offered inside one encoding's - /// section reaches every series of the object: the heatmap underneath a - /// contour would be silently rebuilt from defaults by a button that names - /// only the contour, and reported back to the user as a contour. + /// This remains an encoding operation rather than a catalog provider + /// operation: it deliberately names an encoding kind so resetting a contour + /// cannot silently rebuild a heatmap stacked below it. pub fn plan_encoding_reset( &self, encoding: EncodingKind, targets: &[TargetRef], ) -> Result { - let mut plan = BindingPlan::default(); + let mut transaction = PropertyTransaction::default(); let mut skipped = Vec::new(); let mut applied = Vec::new(); for target in targets { - let context = match self.series_context_unchecked(target) { + let context = match series_context_unchecked(self, target) { Ok(context) => context, Err(error) => { - skipped.push((target.clone(), error.to_string())); + skipped.push(PropertySkip::from_error(target.clone(), &error)); continue; } }; let actual = EncodingKind::of(context.encoding); if actual != encoding { - skipped.push(( + skipped.push(PropertySkip::new( target.clone(), + SkipReason::NotApplicable, format!( "this reset rebuilds a {} and this series is drawn as a {}", encoding.as_str(), @@ -273,7 +205,11 @@ impl PlotxApp { continue; } let Some(descriptor) = context.dataset.field_descriptor(context.field) else { - skipped.push((target.clone(), "the source field is gone".to_owned())); + skipped.push(PropertySkip::new( + target.clone(), + SkipReason::TargetMissing, + "the source field is gone".to_owned(), + )); continue; }; let requested = match context.encoding { @@ -282,310 +218,242 @@ impl PlotxApp { SeriesEncoding::Heatmap(_) => RequestedChart::Heatmap, SeriesEncoding::Image(_) => RequestedChart::Image, }; - let encoding = default_encoding( + let replacement = default_encoding( &descriptor.capabilities, &descriptor.metadata, requested, &PresentationProfile::default(), &|| field_peak_magnitude(context.dataset, context.field), ); - let binding = plan.entry(self, context.canvas, context.object)?; + // Measured the same way the individual controls in this section + // measure themselves, so a reset that finds the series already at + // its factory encoding reports a skip instead of an update nobody + // can see. + transaction.begin_target(); + let binding = transaction.data_binding(self, context.canvas, context.object)?; let Some(series) = binding .series .iter_mut() .find(|series| series.id == context.series) else { - skipped.push((target.clone(), "the series is gone".to_owned())); + skipped.push(PropertySkip::new( + target.clone(), + SkipReason::TargetMissing, + "the series is gone".to_owned(), + )); continue; }; - series.encoding = encoding; - applied.push(PropertyAddress::new(target.clone(), PropertyId("encoding"))); + series.encoding = replacement; + if transaction.target_changed() { + applied.push(PropertyAddress::new(target.clone(), PropertyId("encoding"))); + } else { + skipped.push(PropertySkip::new( + target.clone(), + SkipReason::AlreadyAtValue, + format!( + "this {} is already what its factory produces", + encoding.as_str() + ), + )); + } } Ok(PropertyCommit { - action: plan.into_action(), + action: transaction.into_action(), applied, skipped, }) } - /// Execute a validated commit and report what was skipped. Returns the - /// number of targets actually changed. + /// Execute a validated commit and report the number of targets that were + /// actually changed. pub fn commit_property(&mut self, commit: PropertyCommit) -> usize { let applied = commit.applied.len(); - self.execute_action(commit.action); + // A same-value write changes nothing, so its composite is empty and + // `try_execute_action` would drop it anyway. Stopping here is about the + // *report*: the caller is told the target was skipped and why, instead + // of being told an update succeeded that it cannot tell apart from one + // that moved a value. + if applied != 0 { + self.execute_property_action(commit.action); + } applied } - fn definition_for( - &self, - property: PropertyId, - ) -> Result<&'static PropertyDefinition, PropertyError> { - definition(property) - .ok_or_else(|| PropertyError::UnknownProperty(property.as_str().to_owned())) + /// Open a continuous gesture on one catalog control. + /// + /// Between this call and [`Self::end_property_gesture`], commits are applied + /// live but kept out of undo history; the gesture is recorded as one step + /// when it closes. A drag that recorded a step per frame would fill the + /// bounded history and leave the user unable to undo past it. + pub fn begin_property_gesture(&mut self, property: PropertyId) { + if self + .session + .ui + .property_gesture + .as_ref() + .is_some_and(|gesture| gesture.property == property) + { + return; + } + self.end_property_gesture(); + self.session.ui.property_gesture = Some(PendingPropertyGesture { + property, + first: None, + last: None, + owns_processing_session: false, + }); + } + + /// Close the open gesture, recording everything it did as one undo step. + pub fn end_property_gesture(&mut self) { + let Some(gesture) = self.session.ui.property_gesture.take() else { + return; + }; + if gesture.owns_processing_session { + self.finish_processing_session(); + } + let Some(first) = gesture.first else { + return; + }; + // `first` alone would undo the gesture but redo only its first frame. + // Both bounds carry absolute snapshots, so the pair is the whole + // gesture: reverting it restores the state before, applying it + // reproduces the state after. + let action = match gesture.last { + Some(last) => Action::Composite(vec![first, last]), + None => first, + }; + if let Err(error) = self.record_applied_action(action) { + self.session.status = error.to_string(); + } } - /// Shared planning body: resolve, validate, mutate a working copy, and fold - /// every touched object into one action. - fn plan( + /// Execute a catalog action, letting every store it touches keep its own + /// commit rules. + /// + /// A processing recipe is why this exists. "Pause auto-recompute" is a + /// judgement about when a recipe change becomes visible, and + /// [`Self::commit_processing_edit`] is the one place that makes it. Running + /// the composite through the generic executor would recompute immediately + /// and never stage the pending edit, so the switch would silently do + /// nothing for anything the catalog writes — and the catalog must not carry + /// a second copy of the decision to avoid that. + fn execute_property_action(&mut self, action: Action) { + let mut recipes = Vec::new(); + let mut rest = Vec::new(); + split_property_action(action, &mut recipes, &mut rest); + for (dataset, before, after) in recipes { + let Some(index) = self.doc.dataset_index(dataset) else { + continue; + }; + // A gesture borrows the processing session, whose live edits are + // already recorded once when it ends. Opening it here rather than + // in the panel keeps the panel from having to know which store a + // property writes to. + if let Some(gesture) = self.session.ui.property_gesture.as_mut() + && !gesture.owns_processing_session + && self.session.ui.processing_session.is_none() + { + gesture.owns_processing_session = true; + self.begin_processing_session(index); + } + self.commit_processing_edit(index, before, after); + } + if rest.is_empty() { + return; + } + let action = Action::Composite(rest); + let Some(gesture) = self.session.ui.property_gesture.as_mut() else { + self.execute_action(action); + return; + }; + if gesture.first.is_none() { + gesture.first = Some(action.clone()); + } else { + gesture.last = Some(action.clone()); + } + self.apply_action(&action); + self.doc.dirty = true; + } + + fn plan_edit( &self, - definition: &'static PropertyDefinition, + property: PropertyId, targets: &[TargetRef], - edit: &mut dyn FnMut( - &mut plotx_figure::ContourSpec, - &SeriesContext<'_>, - ) -> Result<(), PropertyError>, + operation: EditOp, ) -> Result { - let mut plan = BindingPlan::default(); + let definition = self.definition_for(property)?; + if definition.access == PropertyAccess::ReadOnly { + return Err(PropertyError::ReadOnly(property)); + } + let provider = provider_for(property) + .ok_or_else(|| PropertyError::UnknownProperty(property.as_str().to_owned()))?; + let mut transaction = PropertyTransaction::default(); let mut skipped = Vec::new(); let mut applied = Vec::new(); for target in targets { - let context = match self.series_context(target, definition) { - Ok(context) => context, - Err(error) => { - skipped.push((target.clone(), error.to_string())); - continue; - } - }; - if !matches!(context.encoding, SeriesEncoding::Contour(_)) { - skipped.push(( + let address = PropertyAddress::new(target.clone(), property); + transaction.begin_target(); + match provider.edit(self, &mut transaction, &address, operation) { + Ok(()) if transaction.target_changed() => applied.push(address), + Ok(()) => skipped.push(PropertySkip::new( target.clone(), - not_applicable_encoding(definition, context.encoding).to_string(), - )); - continue; + SkipReason::AlreadyAtValue, + format!("{} already has that value", definition.canonical_label), + )), + Err(error) if skipped_target(&error) => { + transaction.rollback_target(); + skipped.push(PropertySkip::from_error(target.clone(), &error)); + } + // The transaction only contains working copies. Returning here + // drops all of them, so a bad value can never land on a prefix + // of a multi-target selection. + Err(error) => return Err(error), } - let binding = plan.entry(self, context.canvas, context.object)?; - let Some(series) = binding - .series - .iter_mut() - .find(|series| series.id == context.series) - else { - skipped.push((target.clone(), "the series is gone".to_owned())); - continue; - }; - let SeriesEncoding::Contour(spec) = &mut series.encoding else { - skipped.push(( - target.clone(), - "the series is no longer a contour".to_owned(), - )); - continue; - }; - // A failure here aborts the whole commit: the working copies are - // dropped and no action is ever built, so nothing lands partially. - edit(spec, &context)?; - applied.push(PropertyAddress::new(target.clone(), definition.id)); } Ok(PropertyCommit { - action: plan.into_action(), + action: transaction.into_action(), applied, skipped, }) } - /// What the default policy resolves to for this property in the target's - /// current context. - /// - /// The context is the target as it stands now, not only the field it draws: - /// a setting whose meaning depends on another setting the user has changed - /// is re-derived under that change (§8.1). The factory writes one ladder to - /// both halves, so its answer is always `Uniform`; `None` reports the ways - /// that can fail to hold — a target that is no longer a contour, an id the - /// contour reader does not know, or a factory that grew an asymmetric - /// default — instead of inventing a value to reset to. - fn factory_default( + fn definition_for( &self, property: PropertyId, - context: &SeriesContext<'_>, - ) -> Option { - let SeriesEncoding::Contour(current) = context.encoding else { - return None; - }; - let defaults = self.default_contour_spec_for(context); - contour::default_for(property, &defaults, current, &|| { - field_peak_magnitude(context.dataset, context.field) - })? - .uniform() - .copied() - } - - fn default_contour_spec_for(&self, context: &SeriesContext<'_>) -> plotx_figure::ContourSpec { - default_contour_spec(&context.capabilities, &|| { - field_peak_magnitude(context.dataset, context.field) - }) - } - - /// Resolve a target and enforce the definition's applicability. The - /// component shape is checked before any domain lookup happens, so a - /// misaddressed property is rejected rather than reaching plot code. - fn series_context( - &self, - target: &TargetRef, - definition: &'static PropertyDefinition, - ) -> Result, PropertyError> { - let actual = ComponentKind::of(target.component.as_ref()); - if actual != definition.applicability.component { - return Err(PropertyError::ComponentKind { - property: definition.id, - expected: definition.applicability.component.as_str(), - actual: actual.as_str(), - }); - } - let context = self.series_context_unchecked(target)?; - let missing = definition - .applicability - .required_capabilities - .iter() - .find(|capability| !context.capabilities.contains(capability)); - if let Some(missing) = missing { - return Err(PropertyError::NotApplicable(format!( - "{} needs a field that exposes {missing}", - definition.canonical_label - ))); - } - if let Some(expected) = definition.applicability.encoding - && EncodingKind::of(context.encoding) != expected - { - return Err(not_applicable_encoding(definition, context.encoding)); - } - Ok(context) - } - - /// Address resolution only: object, series, source field and capabilities. - pub(super) fn series_context_unchecked( - &self, - target: &TargetRef, - ) -> Result, PropertyError> { - let Some(ComponentRef::Series(series)) = target.component else { - return Err(PropertyError::ComponentKind { - property: PropertyId("series"), - expected: ComponentKind::Series.as_str(), - actual: ComponentKind::of(target.component.as_ref()).as_str(), - }); - }; - let (canvas, object) = self.canvas_object(&target.resource)?; - let plot = self.doc.canvases[canvas] - .object(object) - .and_then(|object| object.plot()) - .ok_or_else(|| PropertyError::UnknownTarget(target.resource.id.clone()))?; - let binding = plot - .binding - .series - .iter() - .find(|binding| binding.id == series) - .ok_or_else(|| { - PropertyError::UnknownTarget(format!("{}/series/{series}", target.resource.id)) - })?; - let dataset = self - .doc - .dataset_by_id(binding.source.resource) - .ok_or_else(|| PropertyError::UnknownTarget(binding.source.resource.to_string()))?; - let capabilities = dataset - .field_descriptor(binding.source.field) - .map(|descriptor| descriptor.capabilities) - .unwrap_or_default(); - Ok(SeriesContext { - canvas, - object, - series, - dataset, - field: binding.source.field, - capabilities, - encoding: &binding.encoding, - }) - } - - /// Turn a canvas-object resource reference into a one-shot lookup position. - fn canvas_object(&self, resource: &ResourceRef) -> Result<(usize, ObjectId), PropertyError> { - let unknown = || PropertyError::UnknownTarget(resource.id.clone()); - if resource.kind.0 != crate::automation::KIND_CANVAS_OBJECT { - return Err(PropertyError::NotApplicable(format!( - "expected a plot object, got {}", - resource.kind.0 - ))); - } - let parent = resource.parent_id.as_deref().ok_or_else(unknown)?; - let local = resource.local_id.as_deref().ok_or_else(unknown)?; - let canvas_id: CanvasId = parent.parse().map_err(|_| unknown())?; - let object: ObjectId = local.parse().map_err(|_| unknown())?; - let canvas = self.doc.canvas_index(canvas_id).ok_or_else(unknown)?; - Ok((canvas, object)) + ) -> Result<&'static super::PropertyDefinition, PropertyError> { + definition(property) + .ok_or_else(|| PropertyError::UnknownProperty(property.as_str().to_owned())) } } -/// Working copies of every plot binding a commit touches, so two series of the -/// same object collapse into one action rather than two whose snapshots would -/// overwrite each other. -#[derive(Default)] -struct BindingPlan { - entries: Vec<(usize, ObjectId, DataBinding, DataBinding)>, -} - -impl BindingPlan { - fn entry( - &mut self, - app: &PlotxApp, - canvas: usize, - object: ObjectId, - ) -> Result<&mut DataBinding, PropertyError> { - if let Some(index) = self - .entries - .iter() - .position(|entry| entry.0 == canvas && entry.1 == object) - { - return Ok(&mut self.entries[index].3); +/// Separate the recipe edits of a catalog action from everything else, so each +/// half reaches the surface that owns its commit rules. +fn split_property_action( + action: Action, + recipes: &mut Vec<(DatasetId, DatasetProcessingState, DatasetProcessingState)>, + rest: &mut Vec, +) { + match action { + Action::Composite(actions) => { + for action in actions { + split_property_action(action, recipes, rest); + } } - let binding = app - .doc - .canvases - .get(canvas) - .and_then(|canvas| canvas.object(object)) - .and_then(|object| object.plot()) - .map(|plot| plot.binding.clone()) - .ok_or_else(|| PropertyError::UnknownTarget(object.to_string()))?; - self.entries - .push((canvas, object, binding.clone(), binding)); - let last = self.entries.len() - 1; - Ok(&mut self.entries[last].3) - } - - fn into_action(self) -> Action { - Action::Composite( - self.entries - .into_iter() - .filter(|(_, _, before, after)| before != after) - .map(|(canvas, object, before, after)| { - Action::set_data_binding(canvas, object, before, after) - }) - .collect(), - ) + Action::UpdateDatasetProcessing { + dataset, + before, + after, + } => recipes.push((dataset, before, after)), + other => rest.push(other), } } -fn not_applicable_encoding( - definition: &'static PropertyDefinition, - encoding: &SeriesEncoding, -) -> PropertyError { - PropertyError::NotApplicable(format!( - "{} applies to a contour, and this series is drawn as a {}", - definition.canonical_label, - EncodingKind::of(encoding).as_str() - )) -} - -fn resolved_schema( - definition: &'static PropertyDefinition, - capabilities: &FieldCapabilities, -) -> ResolvedSchema { - match definition.value_schema { - ValueSchema::Bool => ResolvedSchema::Bool, - ValueSchema::Int { min, max } => ResolvedSchema::Int { min, max }, - ValueSchema::Float { bounds, log } => ResolvedSchema::Float { - bounds, - log, - unit: "", - }, - ValueSchema::Enum { .. } => ResolvedSchema::Enum { - variants: permitted_variants(&definition.value_schema, capabilities), - }, - ValueSchema::Color => ResolvedSchema::Color, - } +fn skipped_target(error: &PropertyError) -> bool { + matches!( + error, + PropertyError::ComponentKind { .. } + | PropertyError::UnknownTarget(_) + | PropertyError::NotApplicable(_) + ) } diff --git a/crates/core/src/properties/target.rs b/crates/core/src/properties/target.rs new file mode 100644 index 0000000..2cceaab --- /dev/null +++ b/crates/core/src/properties/target.rs @@ -0,0 +1,259 @@ +//! Shared target resolution for series-owned property providers. + +use super::{ + ComponentKind, EncodingKind, PropertyDefinition, PropertyError, ResolvedSchema, ValueSchema, + permitted_variants, +}; +use crate::automation::{ComponentRef, ResourceRef, TargetRef, canvas_object_ref}; +use crate::state::{ + CanvasId, Dataset, DatasetId, FieldCapabilities, FieldId, ObjectId, PlotxApp, SeriesId, +}; +use plotx_figure::SeriesEncoding; +use plotx_processing::ProcessingStep; + +/// The singleton project document is the implicit root above all persisted +/// resources. It has no separately persisted UUID because a `PlotxApp` owns +/// exactly one document; the stable root token is therefore sufficient inside +/// that document's target space. +pub(crate) fn document_target() -> TargetRef { + TargetRef::resource(ResourceRef { + id: crate::automation::DOCUMENT_RESOURCE_ID.to_owned(), + kind: crate::automation::ResourceKindId::new(crate::automation::KIND_DOCUMENT), + parent_id: None, + local_id: None, + }) +} + +pub(crate) fn require_document_target( + target: &TargetRef, + definition: &'static PropertyDefinition, +) -> Result<(), PropertyError> { + let actual = ComponentKind::of(target.component.as_ref()); + if actual != ComponentKind::None { + return Err(PropertyError::ComponentKind { + property: definition.id, + expected: ComponentKind::None.as_str(), + actual: actual.as_str(), + }); + } + if target.resource.kind.0 != crate::automation::KIND_DOCUMENT + || target.resource.id != crate::automation::DOCUMENT_RESOURCE_ID + { + return Err(PropertyError::NotApplicable(format!( + "{} belongs to the document, not {}", + definition.canonical_label, target.resource.id + ))); + } + Ok(()) +} + +/// A target resolved to one series plus the field facts providers need. +/// Indices are one-shot lookup positions and never leave this module. +pub(crate) struct SeriesContext<'a> { + pub(crate) canvas: usize, + pub(crate) object: ObjectId, + pub(crate) series: SeriesId, + pub(crate) dataset: &'a Dataset, + pub(crate) field: FieldId, + pub(crate) capabilities: FieldCapabilities, + pub(crate) encoding: &'a SeriesEncoding, +} + +pub(crate) fn series_context<'a>( + app: &'a PlotxApp, + target: &TargetRef, + definition: &'static PropertyDefinition, +) -> Result, PropertyError> { + let actual = ComponentKind::of(target.component.as_ref()); + if actual != definition.applicability.component { + return Err(PropertyError::ComponentKind { + property: definition.id, + expected: definition.applicability.component.as_str(), + actual: actual.as_str(), + }); + } + let context = series_context_unchecked(app, target)?; + let missing = definition + .applicability + .required_capabilities + .iter() + .find(|capability| !context.capabilities.contains(capability)); + if let Some(missing) = missing { + return Err(PropertyError::NotApplicable(format!( + "{} needs a field that exposes {missing}", + definition.canonical_label + ))); + } + if let Some(expected) = definition.applicability.encoding + && EncodingKind::of(context.encoding) != expected + { + return Err(not_applicable_encoding(definition, context.encoding)); + } + Ok(context) +} + +pub(crate) fn series_context_unchecked<'a>( + app: &'a PlotxApp, + target: &TargetRef, +) -> Result, PropertyError> { + let Some(ComponentRef::Series(series)) = target.component else { + return Err(PropertyError::ComponentKind { + property: super::PropertyId("series"), + expected: ComponentKind::Series.as_str(), + actual: ComponentKind::of(target.component.as_ref()).as_str(), + }); + }; + let (canvas, object) = canvas_object(app, &target.resource)?; + let plot = app.doc.canvases[canvas] + .object(object) + .and_then(|object| object.plot()) + .ok_or_else(|| PropertyError::UnknownTarget(target.resource.id.clone()))?; + let binding = plot + .binding + .series + .iter() + .find(|binding| binding.id == series) + .ok_or_else(|| { + PropertyError::UnknownTarget(format!("{}/series/{series}", target.resource.id)) + })?; + let dataset = app + .doc + .dataset_by_id(binding.source.resource) + .ok_or_else(|| PropertyError::UnknownTarget(binding.source.resource.to_string()))?; + let capabilities = dataset + .field_descriptor(binding.source.field) + .map(|descriptor| descriptor.capabilities) + .unwrap_or_default(); + Ok(SeriesContext { + canvas, + object, + series, + dataset, + field: binding.source.field, + capabilities, + encoding: &binding.encoding, + }) +} + +pub(crate) fn canvas_object( + app: &PlotxApp, + resource: &ResourceRef, +) -> Result<(usize, ObjectId), PropertyError> { + let unknown = || PropertyError::UnknownTarget(resource.id.clone()); + if resource.kind.0 != crate::automation::KIND_CANVAS_OBJECT { + return Err(PropertyError::NotApplicable(format!( + "expected a plot object, got {}", + resource.kind.0 + ))); + } + let parent = resource.parent_id.as_deref().ok_or_else(unknown)?; + let local = resource.local_id.as_deref().ok_or_else(unknown)?; + let canvas_id: CanvasId = parent.parse().map_err(|_| unknown())?; + let object: ObjectId = local.parse().map_err(|_| unknown())?; + let canvas = app.doc.canvas_index(canvas_id).ok_or_else(unknown)?; + Ok((canvas, object)) +} + +pub(crate) fn series_targets(app: &PlotxApp, canvas: usize, object: ObjectId) -> Vec { + let Some(canvas_document) = app.doc.canvases.get(canvas) else { + return Vec::new(); + }; + let resource = canvas_object_ref(canvas_document.resource_id, object); + canvas_document + .object(object) + .and_then(|object| object.plot()) + .map(|plot| { + plot.binding + .series + .iter() + .map(|series| TargetRef { + resource: resource.clone(), + component: Some(ComponentRef::Series(series.id)), + }) + .collect() + }) + .unwrap_or_default() +} + +/// Every processing step one dataset exposes, with the axis that owns it. +/// +/// Which axes exist, and which of them currently have a recipe at all, is asked +/// of the dataset's own neutral pipeline API rather than re-derived from its +/// variant. `phase_axes` already withholds F1 from a dataset that is not truly +/// two-dimensional, and `axis_pipeline` already withholds it until the indirect +/// dimension has been transformed; a second derivation here would hand the +/// catalog steps the processing panel never shows and the user cannot navigate +/// to, and report writing to them as a success. +pub(crate) fn dataset_steps( + dataset: &Dataset, +) -> impl Iterator { + dataset + .phase_axes() + .iter() + .filter_map(move |&axis| Some((axis, dataset.axis_pipeline(axis)?))) + .flat_map(|(axis, pipeline)| pipeline.steps.iter().map(move |step| (axis, step))) +} + +/// Whether a dataset holds any component the property catalog can address. +/// +/// This is the admission question behind `CAP_PROPERTY_CATALOG`, asked once so +/// the capability and the target expansion cannot disagree about what "has +/// addressable components" means. +pub(crate) fn dataset_has_property_components(dataset: &Dataset) -> bool { + dataset_steps(dataset).next().is_some() +} + +/// Every processing-step component owned by one dataset resource. +/// +/// The provider sees only a stable `StepId`; axis placement is intentionally +/// resolved here as a one-shot lookup and never crosses an action or persistence +/// boundary as an index. +pub(crate) fn processing_step_targets(app: &PlotxApp, resource: &ResourceRef) -> Vec { + let Ok(dataset_id) = DatasetId::try_from(resource) else { + return Vec::new(); + }; + let Some(dataset) = app.doc.dataset_by_id(dataset_id) else { + return Vec::new(); + }; + dataset_steps(dataset) + .map(|(_, step)| TargetRef { + resource: resource.clone(), + component: Some(ComponentRef::ProcessingStep(step.id)), + }) + .collect() +} + +pub(crate) fn not_applicable_encoding( + definition: &'static PropertyDefinition, + encoding: &SeriesEncoding, +) -> PropertyError { + let expected = definition + .applicability + .encoding + .map(EncodingKind::as_str) + .unwrap_or("this target"); + PropertyError::NotApplicable(format!( + "{} applies to a {expected}, and this series is drawn as a {}", + definition.canonical_label, + EncodingKind::of(encoding).as_str() + )) +} + +pub(crate) fn resolved_schema( + definition: &'static PropertyDefinition, + capabilities: &FieldCapabilities, +) -> ResolvedSchema { + match definition.value_schema { + ValueSchema::Bool => ResolvedSchema::Bool, + ValueSchema::Int { min, max } => ResolvedSchema::Int { min, max }, + ValueSchema::Float { bounds, log, .. } => ResolvedSchema::Float { + bounds, + log, + unit: "", + }, + ValueSchema::Enum { .. } => ResolvedSchema::Enum { + variants: permitted_variants(&definition.value_schema, capabilities), + }, + ValueSchema::Color => ResolvedSchema::Color, + } +} diff --git a/crates/core/src/properties/tests.rs b/crates/core/src/properties/tests.rs index 73d9662..899bbcb 100644 --- a/crates/core/src/properties/tests.rs +++ b/crates/core/src/properties/tests.rs @@ -5,8 +5,9 @@ use crate::automation::{ TargetRef, }; use crate::state::{ - CONTOUR_BASE_FRACTION_OF_RANGE, CONTOUR_BASE_NOISE_FLOOR, CanvasDocument, Dataset, - Nmr2DDataset, ObjectFrame, PlotxApp, SeriesBinding, SeriesId, + CONTOUR_BASE_ABSOLUTE, CONTOUR_BASE_FRACTION_OF_RANGE, CONTOUR_BASE_NOISE_FLOOR, + CanvasDocument, Dataset, Nmr2DDataset, NmrDataset, ObjectFrame, PlotxApp, SeriesBinding, + SeriesId, }; /// The default plane: values running -7..8, so its noise estimate is an @@ -43,6 +44,21 @@ fn nmr2d_with(source: &str, values: &[f64]) -> plotx_io::NmrData2D { } } +fn nmr1d_with(source: &str) -> plotx_io::NmrData { + plotx_io::NmrData { + points: (0..32) + .map(|value| num_complex::Complex64::new(f64::from(value), 0.0)) + .collect(), + domain: plotx_io::Domain::Frequency, + spectral_width_hz: 4_000.0, + observe_freq_mhz: 400.0, + carrier_ppm: 0.0, + nucleus: "1H".to_owned(), + source: source.to_owned(), + group_delay: 0.0, + } +} + /// One page holding one plot bound to a true-2D spectrum, i.e. the exact shape /// the driving case has: a contour drawn from a signed scalar grid. pub(crate) fn contour_app() -> (PlotxApp, TargetRef) { @@ -130,6 +146,30 @@ fn stable_property_ids_are_unique() { assert_eq!(ids.len(), total, "catalog ids must be unique"); } +/// Provider modules are registered only through `GROUPS`. Keeping the catalog +/// derived from that one list means a new family cannot become searchable while +/// its reader/writer was forgotten, or vice versa. +#[test] +fn provider_groups_are_the_catalog_registration() { + let grouped: Vec = GROUPS + .iter() + .flat_map(|group| group.provider.definitions()) + .map(|definition| definition.id) + .collect(); + let catalogued: Vec = catalog().iter().map(|definition| definition.id).collect(); + assert_eq!(catalogued, grouped, "catalog entries come only from GROUPS"); + assert!( + catalogued.contains(&contour::COUNT), + "the contour provider must be registered through GROUPS" + ); + for id in grouped { + assert!( + provider_for(id).is_some(), + "{id} has a definition but no dispatch provider" + ); + } +} + /// Every entry must be reachable: a definition nothing can address is dead /// weight that still costs a panel row and a search hit. #[test] @@ -365,9 +405,9 @@ fn an_inapplicable_target_is_reported_rather_than_ignored() { assert_eq!(set.applicable_targets.len(), 1); assert_eq!(set.skipped_targets.len(), 1); assert!( - set.skipped_targets[0].1.contains("contour"), + set.skipped_targets[0].message.contains("contour"), "the reason names the mismatch: {}", - set.skipped_targets[0].1 + set.skipped_targets[0].message ); let commit = app @@ -661,3 +701,77 @@ fn the_typed_planner_names_the_rejected_value_and_the_bound() { "the bound is named: {message}" ); } + +#[cfg(test)] +#[path = "provider_tests.rs"] +mod provider_tests; + +/// A write that is valid for one target and refused by the next may not leave +/// the first one changed. +/// +/// The refusal has to arise *inside* the planner to mean anything: a value the +/// wire format already rejects never reaches a target at all, so a test that +/// fails at decoding proves nothing about the transaction. Here both values are +/// well-formed numbers and both series accept the property — the second series +/// is anchored to its noise floor, whose ceiling is a fact about that target's +/// current state, so only the planner can know the write is out of range. By +/// then the first series' working copy has already been modified. +#[test] +fn a_refusal_on_a_later_target_leaves_the_earlier_one_untouched() { + let (mut app, first) = contour_app(); + let object: crate::state::ObjectId = + first.resource.local_id.as_deref().unwrap().parse().unwrap(); + let second_id = { + let plot = app.doc.canvases[0] + .object_mut(object) + .and_then(|object| object.plot_mut()) + .expect("plot"); + let id = plot.allocate_series_id(); + let mut extra = plot.binding.series[0].clone(); + extra.id = id; + plot.binding.series.push(extra); + id + }; + let second = app.series_target(0, object, second_id).expect("target"); + + // The two series share one binding, which is exactly the case a per-target + // rollback exists for: the second target selects a working copy the first + // has already written to. + for (target, policy) in [ + (&first, CONTOUR_BASE_ABSOLUTE), + (&second, CONTOUR_BASE_NOISE_FLOOR), + ] { + let commit = app + .plan_property_write( + contour::BASE_POLICY, + std::slice::from_ref(target), + &PropertyValue::Enum(policy), + ) + .expect("both anchors are available on this field"); + app.commit_property(commit); + } + let before_first = contour_spec(&app, &first).positive.base.clone(); + let before_second = contour_spec(&app, &second).positive.base.clone(); + let revision = app.doc.automation_revision; + + // Well above any multiplier, well inside an absolute level. + let error = app + .plan_property_write( + contour::BASE_MAGNITUDE, + &[first.clone(), second.clone()], + &PropertyValue::Float(1.0e6), + ) + .expect_err("the noise-anchored series has a ceiling this value clears"); + assert!( + matches!(error, PropertyError::InvalidValue { .. }), + "an out-of-range value is a refusal, not a skip: {error}" + ); + + assert_eq!( + contour_spec(&app, &first).positive.base, + before_first, + "the first series was already written in the transaction and must be rolled back with it" + ); + assert_eq!(contour_spec(&app, &second).positive.base, before_second); + assert_eq!(app.doc.automation_revision, revision); +} diff --git a/crates/core/src/properties/transaction.rs b/crates/core/src/properties/transaction.rs new file mode 100644 index 0000000..4710e4b --- /dev/null +++ b/crates/core/src/properties/transaction.rs @@ -0,0 +1,247 @@ +//! The writable document snapshots a property provider selects. +//! +//! Providers decide which typed storage they need; the service merely executes +//! the completed action. That keeps a document property, a binding property, +//! and a future app preference from being dispatched by `ScopeKind` in the +//! planner. + +use crate::actions::{Action, DatasetProcessingState}; +use crate::state::{DataBinding, DatasetId, ObjectId, PlotxApp}; +use plotx_figure::FigureTypography; + +/// Working copies of all typed stores a catalog edit touches. +#[derive(Default)] +pub(crate) struct PropertyTransaction { + bindings: BindingPlan, + typography: Option<(FigureTypography, FigureTypography)>, + processing: Vec<(DatasetId, DatasetProcessingState, DatasetProcessingState)>, + /// The working-copy state a single provider edit started from. It is + /// deliberately per target, rather than one transaction-wide dirty bit: + /// two series can share a binding, and a later no-op edit must not inherit + /// the first series' change as a false success. + target_before: Vec, +} + +enum TargetSnapshot { + Binding { + canvas: usize, + object: ObjectId, + before: DataBinding, + }, + Typography(FigureTypography), + Processing { + dataset: DatasetId, + before: DatasetProcessingState, + }, +} + +impl PropertyTransaction { + /// Start measuring one provider operation. The service calls this around + /// every target so it can report a same-value write instead of claiming an + /// empty action was applied. + pub(crate) fn begin_target(&mut self) { + self.target_before.clear(); + } + + /// Whether the current provider operation changed one of the typed + /// working copies it selected. + pub(crate) fn target_changed(&self) -> bool { + self.target_before.iter().any(|snapshot| match snapshot { + TargetSnapshot::Binding { + canvas, + object, + before, + } => self + .bindings + .entries + .iter() + .find(|entry| entry.0 == *canvas && entry.1 == *object) + .is_some_and(|entry| entry.3 != *before), + TargetSnapshot::Typography(before) => { + self.typography.is_some_and(|(_, after)| after != *before) + } + TargetSnapshot::Processing { dataset, before } => self + .processing + .iter() + .find(|(candidate, _, _)| candidate == dataset) + .is_some_and(|(_, _, after)| after != before), + }) + } + + /// A provider may discover a target is inapplicable after selecting a + /// working copy. Restore that operation's local snapshot before the + /// service records its skip, so a failed target cannot leak a mutation + /// into another compatible target's atomic commit. + pub(crate) fn rollback_target(&mut self) { + for snapshot in &self.target_before { + match snapshot { + TargetSnapshot::Binding { + canvas, + object, + before, + } => { + if let Some(entry) = self + .bindings + .entries + .iter_mut() + .find(|entry| entry.0 == *canvas && entry.1 == *object) + { + entry.3 = before.clone(); + } + } + TargetSnapshot::Typography(before) => { + if let Some((_, after)) = self.typography.as_mut() { + *after = *before; + } + } + TargetSnapshot::Processing { dataset, before } => { + if let Some((_, _, after)) = self + .processing + .iter_mut() + .find(|(candidate, _, _)| candidate == dataset) + { + *after = before.clone(); + } + } + } + } + } + + /// Select the binding of one plot object for mutation. Repeated edits to + /// series on the same object share its one before/after snapshot. + pub(crate) fn data_binding( + &mut self, + app: &PlotxApp, + canvas: usize, + object: ObjectId, + ) -> Result<&mut DataBinding, super::PropertyError> { + let binding = self.bindings.entry(app, canvas, object)?; + if !self.target_before.iter().any(|snapshot| { + matches!(snapshot, TargetSnapshot::Binding { canvas: candidate_canvas, object: candidate_object, .. } if *candidate_canvas == canvas && *candidate_object == object) + }) { + self.target_before.push(TargetSnapshot::Binding { + canvas, + object, + before: binding.clone(), + }); + } + Ok(binding) + } + + /// Select the document's figure typography for mutation. The action stores + /// the complete typed value because that is the existing undo boundary. + pub(crate) fn figure_typography(&mut self, app: &PlotxApp) -> &mut FigureTypography { + let typography = &mut self + .typography + .get_or_insert_with(|| { + let value = app.doc.style_library.figure_typography; + (value, value) + }) + .1; + if !self + .target_before + .iter() + .any(|snapshot| matches!(snapshot, TargetSnapshot::Typography(_))) + { + self.target_before + .push(TargetSnapshot::Typography(*typography)); + } + typography + } + + /// Select one dataset's existing processing snapshot for mutation. The + /// provider chooses this store; the service never switches on scope to do + /// so. Multiple component edits to one dataset still become one action. + pub(crate) fn processing_state( + &mut self, + app: &PlotxApp, + dataset: DatasetId, + ) -> Result<&mut DatasetProcessingState, super::PropertyError> { + let index = if let Some(index) = self + .processing + .iter() + .position(|(candidate, _, _)| *candidate == dataset) + { + index + } else { + let current = app + .doc + .dataset_by_id(dataset) + .ok_or_else(|| super::PropertyError::UnknownTarget(dataset.to_string()))?; + let state = DatasetProcessingState::from_dataset(current); + self.processing.push((dataset, state.clone(), state)); + self.processing.len() - 1 + }; + if !self.target_before.iter().any(|snapshot| { + matches!(snapshot, TargetSnapshot::Processing { dataset: candidate, .. } if *candidate == dataset) + }) { + self.target_before.push(TargetSnapshot::Processing { + dataset, + before: self.processing[index].2.clone(), + }); + } + Ok(&mut self.processing[index].2) + } + + pub(crate) fn into_action(self) -> Action { + let mut actions = self.bindings.into_actions(); + if let Some((before, after)) = self.typography + && before != after + { + actions.push(Action::set_figure_typography(before, after)); + } + actions.extend( + self.processing + .into_iter() + .filter(|(_, before, after)| before != after) + .map(|(dataset, before, after)| { + Action::update_dataset_processing(dataset, before, after) + }), + ); + Action::Composite(actions) + } +} + +#[derive(Default)] +struct BindingPlan { + entries: Vec<(usize, ObjectId, DataBinding, DataBinding)>, +} + +impl BindingPlan { + fn entry( + &mut self, + app: &PlotxApp, + canvas: usize, + object: ObjectId, + ) -> Result<&mut DataBinding, super::PropertyError> { + if let Some(index) = self + .entries + .iter() + .position(|entry| entry.0 == canvas && entry.1 == object) + { + return Ok(&mut self.entries[index].3); + } + let binding = app + .doc + .canvases + .get(canvas) + .and_then(|canvas| canvas.object(object)) + .and_then(|object| object.plot()) + .map(|plot| plot.binding.clone()) + .ok_or_else(|| super::PropertyError::UnknownTarget(object.to_string()))?; + self.entries + .push((canvas, object, binding.clone(), binding)); + let index = self.entries.len() - 1; + Ok(&mut self.entries[index].3) + } + + fn into_actions(self) -> Vec { + self.entries + .into_iter() + .filter(|(_, _, before, after)| before != after) + .map(|(canvas, object, before, after)| { + Action::set_data_binding(canvas, object, before, after) + }) + .collect() + } +} diff --git a/crates/core/src/properties/typography.rs b/crates/core/src/properties/typography.rs new file mode 100644 index 0000000..1a6b365 --- /dev/null +++ b/crates/core/src/properties/typography.rs @@ -0,0 +1,119 @@ +//! Document-owned figure typography properties. + +use super::provider::PropertyProvider; +use super::target::require_document_target; +use super::{ + AggregateValue, Applicability, Availability, ComponentKind, DefaultPolicy, EditOp, FloatBounds, + PropertyAccess, PropertyAddress, PropertyDefinition, PropertyError, PropertyId, + PropertyTransaction, PropertyValue, ResolvedProperty, ResolvedSchema, ScopeKind, Tier, + ValueCopies, ValueSchema, definition, +}; +use crate::state::PlotxApp; + +pub const TICK_PT: PropertyId = PropertyId("document.figure.typography.tick_pt"); + +const POINT_BOUNDS: FloatBounds = FloatBounds::inclusive(1.0, 72.0); +/// A quarter point per drag notch: point sizes are chosen to a half point, and +/// the admissible range spans seventy-one of them, so the range says nothing +/// about how finely the value is usually set. +const POINT_STEP: f64 = 0.25; + +pub(crate) const DEFINITIONS: &[PropertyDefinition] = &[PropertyDefinition { + id: TICK_PT, + scope_kind: ScopeKind::Document, + value_schema: ValueSchema::Float { + bounds: POINT_BOUNDS, + log: false, + drag_step: Some(POINT_STEP), + }, + access: PropertyAccess::ReadWrite, + applicability: Applicability::component(ComponentKind::None), + default_policy: DefaultPolicy::Fixed(PropertyValue::Float(7.0)), + tier: Tier::Essential, + copies: ValueCopies::PerTarget, + canonical_label: "Figure tick-label size", + canonical_aliases: &["figure typography", "tick size", "font size", "points"], +}]; + +pub(crate) struct TypographyProvider; + +pub(crate) static PROVIDER: TypographyProvider = TypographyProvider; + +impl PropertyProvider for TypographyProvider { + fn definitions(&self) -> &'static [PropertyDefinition] { + DEFINITIONS + } + + fn read( + &self, + app: &PlotxApp, + address: &PropertyAddress, + ) -> Result { + let definition = definition(address.definition).ok_or_else(|| { + PropertyError::UnknownProperty(address.definition.as_str().to_owned()) + })?; + require_document_target(&address.target, definition)?; + Ok(ResolvedProperty { + address: address.clone(), + value: AggregateValue::Uniform(PropertyValue::Float(f64::from( + app.doc.style_library.figure_typography.tick_pt, + ))), + default_value: match definition.default_policy { + DefaultPolicy::Fixed(value) => Some(value), + DefaultPolicy::EncodingFactory + | DefaultPolicy::ProcessingFactory + | DefaultPolicy::None => None, + }, + availability: Availability::Editable, + schema: ResolvedSchema::Float { + bounds: POINT_BOUNDS, + log: false, + unit: "pt", + }, + }) + } + + fn edit( + &self, + app: &PlotxApp, + transaction: &mut PropertyTransaction, + address: &PropertyAddress, + operation: EditOp, + ) -> Result<(), PropertyError> { + let definition = definition(address.definition).ok_or_else(|| { + PropertyError::UnknownProperty(address.definition.as_str().to_owned()) + })?; + require_document_target(&address.target, definition)?; + let value = match operation { + EditOp::Set(PropertyValue::Float(value)) => { + POINT_BOUNDS.check(definition.id, "tick-label size", value)? + } + EditOp::Set(value) => { + return Err(PropertyError::InvalidValue { + property: definition.id, + message: format!("expected a number, got {}", value.kind()), + }); + } + EditOp::Reset => match definition.default_policy { + DefaultPolicy::Fixed(PropertyValue::Float(value)) => value, + DefaultPolicy::Fixed(_) + | DefaultPolicy::EncodingFactory + | DefaultPolicy::ProcessingFactory + | DefaultPolicy::None => { + return Err(PropertyError::InvalidValue { + property: definition.id, + message: "the default policy has no numeric value".to_owned(), + }); + } + }, + EditOp::Step(_) => { + return Err(PropertyError::InvalidValue { + property: definition.id, + message: "this setting has no step gesture".to_owned(), + }); + } + }; + transaction.figure_typography(app).tick_pt = value as f32; + Ok(()) + } +} diff --git a/crates/core/src/state/datasets.rs b/crates/core/src/state/datasets.rs index a895042..a5dba64 100644 --- a/crates/core/src/state/datasets.rs +++ b/crates/core/src/state/datasets.rs @@ -676,6 +676,31 @@ impl Dataset { } } + /// The recipe this dataset would be loaded with today, for one axis. + /// + /// Produced by re-running the very factory that built the live one, so "what + /// does reset give me" and "what does a new document get" stay one answer + /// rather than two derivations that agree only until one of them changes. + pub fn factory_pipeline(&self, axis: PhaseAxis) -> Option { + match self { + Dataset::Nmr(n) if axis == PhaseAxis::Direct => Some(match n.data.domain { + Domain::Time => AxisPipeline::default_1d(), + Domain::Frequency => AxisPipeline::frequency_1d(), + }), + Dataset::Nmr2D(n) => { + let params = match n.data.domain { + Domain::Time => Params2D::default_for(n.preset), + Domain::Frequency => Params2D::frequency_domain(n.preset), + }; + Some(match axis { + PhaseAxis::F1 => params.f1, + _ => params.f2, + }) + } + _ => None, + } + } + pub fn phase_params_mut(&mut self, axis: PhaseAxis) -> Option<&mut PhaseParams> { self.axis_pipeline_mut(axis).and_then(|pipe| { pipe.steps diff --git a/crates/core/src/state/ui_state.rs b/crates/core/src/state/ui_state.rs index 9a642c5..123c92a 100644 --- a/crates/core/src/state/ui_state.rs +++ b/crates/core/src/state/ui_state.rs @@ -277,6 +277,8 @@ pub struct UiState { pub page_layout_edit: Option, pub processing_edit: Option, pub processing_session: Option, + /// The continuous property-catalog control currently being dragged, if any. + pub property_gesture: Option, pub inspector_edit: Option, /// Pre-edit snapshot for a plot-local axis text/range gesture. pub axis_overrides_before: Option<(usize, ObjectId, AxisOverrides)>, @@ -451,6 +453,7 @@ impl Default for UiState { page_layout_edit: None, processing_edit: None, processing_session: None, + property_gesture: None, inspector_edit: None, axis_overrides_before: None, canvas_settings: None, diff --git a/crates/core/tests/slice2d.rs b/crates/core/tests/slice2d.rs index 3ec720a..5093250 100644 --- a/crates/core/tests/slice2d.rs +++ b/crates/core/tests/slice2d.rs @@ -8,11 +8,11 @@ use num_complex::Complex64; use plotx_core::automation::{ - CallerType, DocumentRevision, TOOL_RESET, TOOL_SET, TargetOutcome, TargetRef, TargetSelector, - ToolRequest, execute_tool, plan_tool, + CallerType, ComponentRef, DocumentRevision, ResourceRef, TOOL_RESET, TOOL_SET, TargetOutcome, + TargetRef, TargetSelector, ToolRequest, execute_tool, plan_tool, }; use plotx_core::build_stack_figure; -use plotx_core::properties::{PropertyId, PropertyValue, contour}; +use plotx_core::properties::{PropertyId, PropertyValue, apodization, contour}; use plotx_core::state::{ CanvasDocument, Dataset, Nmr2DDataset, ObjectFrame, ObjectId, PlotxApp, SeriesBinding, }; @@ -196,6 +196,35 @@ fn series_targets(app: &PlotxApp, object: ObjectId) -> Vec { app.series_targets(0, object) } +/// The default F2 apodization is a dataset-local component. The address carries +/// its stable `StepId`, not an axis index or a pipeline path. +fn apodization_target(app: &PlotxApp) -> TargetRef { + let Dataset::Nmr2D(dataset) = &app.doc.datasets[0] else { + panic!("the fixture owns a 2D NMR dataset"); + }; + let step = dataset + .params + .f2 + .steps + .iter() + .find(|step| matches!(step.kind, plotx_processing::StepKind::Apodize(_))) + .expect("the time-domain pipeline starts with apodization"); + TargetRef { + resource: ResourceRef::from(dataset.resource_id), + component: Some(ComponentRef::ProcessingStep(step.id)), + } +} + +fn processed_samples(app: &PlotxApp) -> Vec { + let Dataset::Nmr2D(dataset) = &app.doc.datasets[0] else { + panic!("the fixture owns a 2D NMR dataset"); + }; + let Processed2D::Ft(spectrum) = &dataset.processed else { + panic!("the HSQC fixture stays a true 2D spectrum"); + }; + spectrum.data.iter().take(32).copied().collect() +} + /// Every series of the plot, reduced to the state a property write owns. /// /// Comparing this whole rather than just the number that was written is what @@ -547,6 +576,64 @@ fn a_json_property_write_reaches_the_drawn_figure() { assert!(svg.contains("Ctrl+Z takes back the gesture +rather than one frame of it. + +### Changed values and reset + +A value that differs from the one the standard pipeline for this dataset puts in +the step is marked with a dot. Hover the dot to see the default; use the reset +button beside it to go back. + +A step you added yourself has no such default — nothing in the standard pipeline +corresponds to it — so it carries no marker and no reset button. Asking for a +reset through [Automation](/guides/automation/) reports that step as skipped +rather than turning a window you chose into *None*. + +### While auto-recompute is paused + +**Pause auto-recompute**, in the ⋮ menu at the top of the Processing panel under +**Advanced**, governs these rows as well. Change **Window**, **LB** or **GB** +while it is on and the recipe records the change without recomputing: the panel +shows **Changes pending** with an **Apply** button, and nothing is recalculated +until you press it. + +### Finding these settings + +- Ctrl+K (Cmd on macOS) searches settings as + well as commands and data. `LB`, `line broadening`, `apodization window` and + `gaussian broadening` all reach these rows; activating one opens the + Processing panel, expands the first step that carries the setting and + highlights it. See [Command palette](/reference/command-palette/). +- **Apodization settings** in the **Processing** group of the Ribbon's + **Process** tab goes to the same place. + ## Phase correction Automatic phase correction is enabled by default; you can switch methods or diff --git a/docs/src/content/docs/reference/command-palette.md b/docs/src/content/docs/reference/command-palette.md index fda1af7..fd3f1fa 100644 --- a/docs/src/content/docs/reference/command-palette.md +++ b/docs/src/content/docs/reference/command-palette.md @@ -37,16 +37,25 @@ Activating a setting does not change anything. It opens the panel the setting lives in, expands its section, scrolls to the row and highlights it briefly, so you can see the current value before editing it. +A setting that belongs to one processing step also opens the Processing panel +and expands the first step that actually carries it — a step whose window has no +**GB**, for instance, is passed over rather than opened onto a row that is not +there. + ## Availability Commands that don't apply in the current context are grayed out — for example, export commands without an active canvas, or align and distribute without enough selected objects. -Settings are grayed out the same way when nothing in the current selection can -receive them — no plot selected, or a selected series that draws something other -than a contour. They stay in the list so you can still find them by name; hover -one to see why it is unavailable. +Settings are grayed out the same way when nothing in the current context can +receive them — no plot selected, a selected series that draws something other +than a contour, or a dataset with no apodization step. They stay in the list so +you can still find them by name; hover one to see why it is unavailable. + +A setting that applies to the selection but cannot be changed there is grayed +out too. A locked plot reads *Unlock this plot to change its settings; it can +still be read while locked.* ## What's included @@ -62,13 +71,24 @@ Commands: - Arrange: grid, align, distribute, z-order, and *Tidy up frames*. - Applying themes and stacking data. - Switching to any tool. -- *Contour settings*, *Raise lowest level* and *Lower lowest level*. +- *Contour settings*, *Line settings*, *Figure typography settings*, + *Apodization settings*, *Raise lowest level* and *Lower lowest level*. Settings: -- The contour rows of the Object inspector — lowest level, anchor, levels, level - ratio, negative contours, colours, and line width. See +- The **Contour** rows of the Object inspector — lowest level, anchor, levels, + level ratio, negative contours, colours, and line width. See [Contour levels](/guides/contour-levels/). +- **Stroke width**, the width of a line series, in the Object inspector's + **Line** section. `line width`, `stroke width`, `trace thickness` and `line + thickness` all reach it. +- **Tick-label size** in the Object inspector's **Figure typography** section. + `font size`, `tick size`, `points` and `figure typography` reach it. See + [Layout and export](/guides/layout-and-export/). +- **Window**, **LB** and **GB** on an apodization step in the Processing panel. + `apodization`, `window function`, `exponential`, `gaussian`, `LB`, `line + broadening`, `GB` and `gaussian broadening` reach them. See + [Processing](/guides/processing/). Data: diff --git a/docs/src/content/docs/reference/ui-overview.md b/docs/src/content/docs/reference/ui-overview.md index eb99022..978c3f5 100644 --- a/docs/src/content/docs/reference/ui-overview.md +++ b/docs/src/content/docs/reference/ui-overview.md @@ -36,7 +36,16 @@ introduces the same regions in walkthrough form. - **Object inspector** — the properties panel for the selected board object: chart type, styling, geometry, and the display settings of what it draws, such as [contour levels](/guides/contour-levels/). Everyday settings are shown - directly; the rest are folded into **Advanced**. + directly; the rest are folded into **Advanced**. Its **Contour** and **Line** + sections appear only when the selection draws one; **Figure typography** + belongs to the document and is always shown. +- **Settings group** — a named set of related settings with one home. The Ribbon + carries a button per group that opens that home rather than repeating the + controls: **Contour settings**, **Line settings** and **Figure typography + settings** in **Figure → Style**, **Apodization settings** in + **Process → Processing**. The canvas right-click menu lists the same groups + that currently apply, as *Contour settings…* and so on, and + Ctrl+K finds the individual settings inside them. - **Data sheet** — the spreadsheet view of a data table, opened by double-clicking the table. - **Command palette** — the searchable list of commands, settings, and data on diff --git a/docs/src/content/docs/zh-cn/guides/automation.md b/docs/src/content/docs/zh-cn/guides/automation.md index af07d32..74de252 100644 --- a/docs/src/content/docs/zh-cn/guides/automation.md +++ b/docs/src/content/docs/zh-cn/guides/automation.md @@ -15,10 +15,11 @@ selection** 载入当前选择——再选择一个工具,点击 **Preflight** 哪些目标、哪些会被跳过。**Confirm and execute** 执行,整批操作合并为 一次 **Undo automation** 撤销。 -### 图的参数 +### 参数设置 -有三个工具能改到对象检查器(**Object inspector**)里的等高线参数,于是一个 -层级、一种颜色或一个线宽可以经一次可预检的批处理,应用到项目中的所有二维图: +有三个工具能改到对象检查器(**Object inspector**)和处理面板里的设置,于是 +一个层级、一种颜色、一个线宽、一个字号或一种窗函数,都可以经一次可预检的批 +处理应用到整个项目: | 工具 | 作用 | | --- | --- | @@ -26,11 +27,14 @@ selection** 载入当前选择——再选择一个工具,点击 **Preflight** | **Set a property** | 写入一个值 | | **Reset a property** | 按当前数据重新推导该值,与面板上的重置按钮一致 | -勾选要处理的图对象——不是页面,也不是数据集——然后在 **Parameters (JSON)** -中用 id 指定参数:**Set a property** 用 +在 **Parameters (JSON)** 中用 id 指定参数:**Set a property** 用 `{"key": "series.contour.count", "value": 12}`,另外两个用 `{"key": "series.contour.count"}`。 +三个工具都接受图对象、数据集,以及文档本身(列为 **PlotX document**)。该勾 +选哪一类由参数本身决定:等高线和线条参数在图对象上,切趾参数在数据集上,图形 +排印参数在文档上。 + | 对象检查器中的设置 | id | 取值 | | --- | --- | --- | | **Lowest level** | `series.contour.base.magnitude` | 大于 0 的数;究竟以什么计量取决于 **Anchor** | @@ -41,23 +45,68 @@ selection** 载入当前选择——再选择一个工具,点击 **Preflight** | **Positive colour** | `series.contour.positive_color` | `"#rrggbb"` | | **Negative colour** | `series.contour.negative_color` | `"#rrggbb"` | | **Line width** | `series.contour.line_width` | 0.05 到 10 | +| **Stroke width**(**Line** 区域) | `series.line.stroke_width` | 0.05 到 10 | +| **Tick-label size**(**Figure typography** 区域) | `document.figure.typography.tick_pt` | 1 到 72 | + +| 切趾步骤上的设置 | id | 取值 | +| --- | --- | --- | +| **Window** | `dataset.processing.apodization.kind` | `none`、`cosine_bell`、`exponential`、`gaussian` | +| **LB** | `dataset.processing.apodization.lb_hz` | −10000 到 10000 | +| **GB** | `dataset.processing.apodization.gb_hz` | 大于 0,最大 10000 | + +各等高线参数的含义、以及每种锚定方式需要什么样的数据,见 +[等高线层级](/zh-cn/guides/contour-levels/);切趾各行的说明见 +[数据处理](/zh-cn/guides/processing/)。 + +#### 勾选的目标会展开成什么 + +被勾选的资源往往并不是真正被写入的对象——展开成什么由参数决定:图对象展开 +为它的各条序列,数据集展开为它的各个处理步骤,文档就是它自己。预检和结果列 +表按展开后的部件逐行列出,因此一个含三条序列的图会给出三行,并指明是哪个图 +对象里的哪条序列。 + +参数够不到的部件会标为 **Skipped** 并给出原因,其余部件照常应用:等高线下方 +的热图、参数属于切趾时列表里的零填充步骤,或对一个窗函数为 *Exponential* 的 +步骤索取 **GB**,都是这种情况。完全没有可寻址内容的资源(文本框、没有管线的 +数据集)同样被跳过。 + +结果行和运行记录会在资源旁写出部件: + +```json +{ + "resource": { "id": "…", "kind": "plotx.dataset" }, + "component": { "kind": "processing_step", "id": 3 } +} +``` + +序列部件写作 `{"kind": "series", "id": 2}`。两种 id 都只在它所属的资源内有 +意义,换一个数据集,同一个步骤 id 就什么也不代表。 + +#### 为什么某个目标被跳过 + +每一条被跳过的行都带有一句供人阅读的说明。由写入本身跳过的行还带有 +`skip_reason`——一个稳定的标记,工作流据此分支即可,不必去匹配文字: + +| `skip_reason` | 含义 | +| --- | --- | +| `already_at_value` | 目标本来就是这个值,没有写入任何内容 | +| `not_applicable` | 该参数不适用于这个目标 | +| `target_missing` | 这个地址在文档里已经指不到任何东西 | -各参数的含义、以及每种锚定方式需要什么样的数据,见 -[等高线层级](/zh-cn/guides/contour-levels/)。 +在预检阶段就被排除的行只有那句说明。它们绝不会是「值本来就一样」这种情况, +因此没有这个标记本身就是区分。 -一个图可以包含多条序列,因此这些工具按序列处理:预检和结果列表为每条序列 -各列一行,并指明是哪个图对象里的哪条序列。参数够不到的序列——例如等高线 -下方的热图——会标为 **Skipped** 并给出原因,其余序列照常应用;没有可寻址 -内容的对象(如文本框)同样被跳过。 +如果 **Set a property** 的所有目标本来就是这个值,那么它什么也不会写入:每个 +目标都报告为跳过,文档版本号不变,撤销栈上也不会多出一步。 超出范围的值在 **Preflight** 阶段、也就是确认之前就会被拒绝,并同时给出你 -填的值和拒绝它的边界。过了这一步,列出的序列要么全部生效,要么全部不变, +填的值和拒绝它的边界。过了这一步,列出的部件要么全部生效,要么全部不变, 写入的改动合并为一次 **Undo automation** 撤销。 **Inspect a property** 把读数返回给调用方:作为工作流的一步运行时,读数会 进入运行记录,[命令行](/zh-cn/reference/cli/)写出的清单文件里也是同一份 -内容。在窗口里,数值显示在逐序列结果下方的 **Result value (JSON)** 区域: -每条序列一份读数,包含当前值、默认值和它接受的范围。 +内容。在窗口里,数值显示在逐部件结果下方的 **Result value (JSON)** 区域: +每个部件一份读数,包含当前值、默认值和它接受的范围。 ## External Inputs diff --git a/docs/src/content/docs/zh-cn/guides/layout-and-export.md b/docs/src/content/docs/zh-cn/guides/layout-and-export.md index 3bde734..96a024d 100644 --- a/docs/src/content/docs/zh-cn/guides/layout-and-export.md +++ b/docs/src/content/docs/zh-cn/guides/layout-and-export.md @@ -115,7 +115,22 @@ NMR 核素质量数。新数据集默认使用 89 × 60 mm 单栏画布:单个 这个手动范围。不显示坐标轴的图表没有轴设置;分类轴不提供范围控制。 - Figure Ribbon 选项卡的 **Figure Typography…** 一次设定文档内所有图的 坐标轴文字尺寸(刻度标签、轴标题与图标题),单位为绝对磅值——这是 - 文档级样式,缩放分图不会改变字号。 + 文档级样式,缩放分图不会改变字号。刻度标签取值 1 到 72 pt,轴标题与 + 图标题取值 4 到 24 pt。 +- 对象检查器的 **Figure typography** 区域中的 **Tick-label size** 就是同 + 一个刻度标签字号,范围同样是 1 到 72 pt,在两处中任何一处修改,改的都 + 是文档里的同一个值。它属于文档而不属于某个图,因此无论当前选中什么 + (包括什么都没选中)都会显示。 +- 对象检查器的 **Line** 区域中的 **Stroke width** 设定所选图中线条序列的 + 线宽,取值 0.05 到 10。只有当所选内容中有以线条绘制的序列时才会出现该 + 区域,标题处会数出即将改动的序列条数。选中多个图即一并编辑;什么都不选 + 时,作用于页面上的活动图。 + + 当所选序列的线宽并不一致时,控件显示破折号和 *mixed*,而不会把其中一 + 个值当成当前设置;此时写入一个值正是让它们重新一致的方式。所选序列中 + 以别的方式绘制的(例如等高线)会在状态栏里报告为跳过,其余序列照常生 + 效。若某个线宽不同于 PlotX 会为这份数据选择的值,该行会标上圆点,旁边 + 的重置按钮按当前数据重新推导默认值。 - **画布主题**携带配套字号——例如 Presentation Dark 主题会放大坐标轴 文字以适合投影演示。 diff --git a/docs/src/content/docs/zh-cn/guides/processing.md b/docs/src/content/docs/zh-cn/guides/processing.md index fa18fcc..eba3d92 100644 --- a/docs/src/content/docs/zh-cn/guides/processing.md +++ b/docs/src/content/docs/zh-cn/guides/processing.md @@ -58,6 +58,48 @@ PlotX 的处理是应用于原始数据的**有序步骤列表**。步骤可随 数字群延迟校正用于消除它;这是步骤列表旁的按数据集开关,在管线之前 应用。 +## 切趾 + +在步骤列表中点击 **Apodize** 一行即可展开它的设置。所有控件都直接显示在 +**Apodization** 标题下,不需要再展开任何折叠: + +- **Window**——*None*、*Cosine bell*、*Exponential* 或 *Gaussian*。 +- **LB**——线增宽,单位 Hz,窗函数为 Exponential 或 Gaussian 时显示。取值 + −10000 到 10000,允许负值:在高斯窗下,正的 LB 会让谱线变窄(洛伦兹到高斯 + 的分辨率增强),负的 LB 则进一步展宽。 +- **GB**——高斯增宽,单位 Hz,仅在窗函数为 Gaussian 时显示。它必须**大于 + 0**,最大 10000。取 0 时高斯项消失,剩下的部分只会无限增长而不再衰减,那 + 已经不是一个窗函数。 + +把某一步切换为 Exponential 或 Gaussian 时,它原本没有的增宽参数从 1 Hz 开始。 + +拖动 **LB** 或 **GB** 会连续预览,但整次拖拽只留下一条撤销记录,一次 +Ctrl+Z 撤销的是整个动作,而不是其中一帧。 + +### 已修改的值与重置 + +某个值若与该数据集的标准管线在这一步中放置的值不同,会标上一个圆点。悬停圆点 +可以看到默认值,旁边的重置按钮把它改回去。 + +你自己添加的步骤没有这样的默认值——标准管线里没有与它对应的步骤——因此它既 +没有圆点也没有重置按钮。通过[自动化](/zh-cn/guides/automation/)对这类步骤请求 +重置时,该步骤会被报告为跳过,而不会把你选定的窗函数变成 *None*。 + +### 暂停自动重算时 + +处理面板顶部 ⋮ 菜单中 **Advanced** 下的 **Pause auto-recompute** 同样管着这 +几行。开启它之后修改 **Window**、**LB** 或 **GB**,配方会记下改动但不重算: +面板显示 **Changes pending** 和一个 **Apply** 按钮,按下之前不会有任何重算。 + +### 从哪里找到这些设置 + +- Ctrl+K(macOS 上为 Cmd)除命令和数据外也 + 搜索设置。`LB`、`line broadening`、`apodization window`、`gaussian + broadening` 都能命中这几行;激活后会打开处理面板,展开第一个真正带有该设置 + 的步骤并高亮它。参见[命令面板](/zh-cn/reference/command-palette/)。 +- Ribbon **Process** 页签 **Processing** 组中的 **Apodization settings** 通向 + 同一处。 + ## 相位校正 自动相位校正默认启用;也可以切换算法,或手动调节 φ0 / φ1 并实时预览。 diff --git a/docs/src/content/docs/zh-cn/reference/command-palette.md b/docs/src/content/docs/zh-cn/reference/command-palette.md index 27e1240..5088db5 100644 --- a/docs/src/content/docs/zh-cn/reference/command-palette.md +++ b/docs/src/content/docs/zh-cn/reference/command-palette.md @@ -31,13 +31,21 @@ description: 用键盘搜索命令、设置与数据。 激活一个设置不会改变任何内容。它会打开该设置所在的面板、展开其分节、滚动到该行 并短暂高亮,让你先看清当前值再编辑。 +若某个设置属于某一个处理步骤,激活它还会打开处理面板,并展开第一个真正带有该 +设置的步骤——例如窗函数没有 **GB** 的步骤会被跳过,而不会展开到一行并不存在 +的控件上。 + ## 可用性 在当前上下文中不适用的命令会置灰——例如没有活动画布时的导出命令,或所选 对象不足时的对齐与分布。 -当前选择无法承载的设置同样会置灰——没有选中任何图,或选中的谱线画的不是等高线。 -它们仍留在列表中,以便你按名称找到;悬停即可看到不可用的原因。 +当前上下文无法承载的设置同样会置灰——没有选中任何图,选中的谱线画的不是等高线, +或数据集里没有切趾步骤。它们仍留在列表中,以便你按名称找到;悬停即可看到不可用 +的原因。 + +适用于当前选择、但在那里改不了的设置也会置灰。被锁定的图给出的原因是 +*Unlock this plot to change its settings; it can still be read while locked.* ## 收录范围 @@ -53,13 +61,24 @@ description: 用键盘搜索命令、设置与数据。 - 排列:网格、对齐、分布、层序与 *Tidy up frames*(一键整理)。 - 应用主题与堆叠数据。 - 切换到任意工具。 -- *Contour settings*(等高线设置)、*Raise lowest level*(提高最低层)与 +- *Contour settings*(等高线设置)、*Line settings*(线条设置)、 + *Figure typography settings*(图形排印设置)、*Apodization settings* + (切趾设置)、*Raise lowest level*(提高最低层)与 *Lower lowest level*(降低最低层)。 设置: -- 对象检查器中的等高线各行——最低层、锚定、层数、层间比值、负等高线、颜色与 - 线宽。参见[等高线层级](/zh-cn/guides/contour-levels/)。 +- 对象检查器 **Contour** 区域的各行——最低层、锚定、层数、层间比值、负等高线、 + 颜色与线宽。参见[等高线层级](/zh-cn/guides/contour-levels/)。 +- 对象检查器 **Line** 区域的 **Stroke width**,即线条序列的线宽。`line width`、 + `stroke width`、`trace thickness` 与 `line thickness` 都能命中它。 +- 对象检查器 **Figure typography** 区域的 **Tick-label size**。`font size`、 + `tick size`、`points` 与 `figure typography` 都能命中它。参见 + [版面与导出](/zh-cn/guides/layout-and-export/)。 +- 处理面板中切趾步骤的 **Window**、**LB** 与 **GB**。`apodization`、 + `window function`、`exponential`、`gaussian`、`LB`、`line broadening`、 + `GB` 与 `gaussian broadening` 都能命中它们。参见 + [数据处理](/zh-cn/guides/processing/)。 数据: diff --git a/docs/src/content/docs/zh-cn/reference/ui-overview.md b/docs/src/content/docs/zh-cn/reference/ui-overview.md index 2afc8ca..6f48943 100644 --- a/docs/src/content/docs/zh-cn/reference/ui-overview.md +++ b/docs/src/content/docs/zh-cn/reference/ui-overview.md @@ -33,7 +33,15 @@ PlotX 的界面为英文;手册中加粗的英文词即界面上的原文标 - **对象检查器(Object inspector)**——所选画板对象的属性面板:图表 类型、样式、几何,以及其绘制内容的显示设置,例如 [等高线层级](/zh-cn/guides/contour-levels/)。常用设置直接显示,其余收在 - **Advanced**(高级)折叠区中。 + **Advanced**(高级)折叠区中。其中 **Contour** 与 **Line** 两个区域只在 + 所选内容确实这样绘制时才出现;**Figure typography** 属于文档,始终显示。 +- **设置分组(settings group)**——一组同属一处的相关设置。Ribbon 为每个 + 分组提供一个按钮,它只负责打开这些设置的所在处,而不重复摆一套控件: + **Figure → Style** 中的 **Contour settings**、**Line settings** 与 + **Figure typography settings**,**Process → Processing** 中的 + **Apodization settings**。画布右键菜单会列出当前适用的同一批分组,形如 + *Contour settings…*;Ctrl+K 则能找到分组里的单项 + 设置。 - **数据表(Data sheet)**——数据表格的电子表格视图,双击表格打开。 - **命令面板(Command palette)**——Ctrl+K 打开 的可搜索列表,涵盖命令、设置与数据;见