docs/plans/wgpu-egui-architecture-spike.md

@@ -0,0 +1,124 @@
+# WGPU/egui Interactive App Architecture Spike
+
+> Goal: introduce an interactive OpenVistaPro app without destabilizing the existing CLI, importer pipeline, or deterministic CPU reference renderer.
+
+## Decision
+
+Keep the repository as a single Rust crate for now.
+
+The current boundary set is still moving, but it is already useful:
+
+- `src/terrain.rs`, `src/terrain_gen.rs`, `src/import.rs`, `src/scene.rs`, `src/scene_file.rs`, `src/script.rs`, `src/script_exec.rs`, `src/path.rs`, `src/render.rs`, and `src/colormap.rs` are the clean-room core.
+- `src/cli.rs` and `src/main.rs` are the command-line adapter.
+- `src/app_state.rs`, `src/ui_shell.rs`, `src/app.rs`, and `src/bin/openvistapro_app.rs` are the optional interactive shell.
+
+A workspace split into `openvistapro-core`, `openvistapro-cli`, and `openvistapro-app` would add maintenance overhead before there is a second renderer/backend boundary that truly justifies it. The better near-term move is to keep a monolith and enforce boundaries with modules, feature flags, and tests.
+
+Revisit a workspace split only when at least one of these becomes true:
+
+1. the GPU renderer needs materially different dependencies from the CLI/reference renderer,
+2. the core API stabilizes enough that the app shell can depend on it as a published internal crate boundary, or
+3. build times / dependency fanout make the single-crate layout painful.
+
+## Integration recommendation
+
+Use egui as the interactive shell now, and treat WGPU as the rendering backend that arrives later behind a narrow viewport abstraction.
+
+Recommended path:
+
+1. Keep `eframe` for the windowing / egui host while the shell is still mostly controls and state.
+2. Keep the current CPU renderer as the canonical reference output for tests and CLI smoke runs.
+3. Introduce a tiny viewport trait or adapter boundary before any full GPU renderer lands.
+4. Add a WGPU-backed viewport only once the render pipeline actually needs GPU surfaces, textures, and custom passes.
+
+Why this order:
+
+- `eframe` already gives a working egui host with the least glue.
+- CLI builds stay GPU-free because the app stays behind the `app` feature.
+- The CPU renderer remains the reference implementation for determinism and testability.
+- WGPU can then become an implementation detail of the viewport, not a rewrite of the app state model.
+
+## Module boundaries
+
+Keep the following responsibilities separate:
+
+### Core domain
+
+- `terrain.rs`: immutable height grid and sampling rules.
+- `terrain_gen.rs`: deterministic procedural generation inputs and output grid creation.
+- `import.rs`: import boundary and source metadata.
+- `scene.rs`: camera, lighting, hydrology, and palette-linked scene state.
+- `scene_file.rs`: `.ovp.toml` persistence.
+- `script.rs` / `script_exec.rs`: project-owned script language and executor.
+- `path.rs`: MakePath-style camera path generation.
+- `render.rs`: deterministic CPU render outputs.
+- `colormap.rs`: palette / band mapping.
+
+### CLI adapter
+
+- `cli.rs` should stay focused on argument parsing and orchestration.
+- Keep it thin enough that the same core operations can be called from future automation or from the app shell.
+
+### Interactive shell state
+
+- `app_state.rs` should remain pure state plus reducers / actions.
+- `ui_shell.rs` should own section ordering, labels, and placeholder metadata.
+- Avoid leaking egui types into either file.
+
+### egui view/controller
+
+- `app.rs` should remain the presentation layer that turns `AppData` into panels, menus, and a preview texture.
+- The shell should consume state snapshots and call backend actions, not embed render or import logic inline.
+
+### Future GPU viewport
+
+- Introduce a narrow backend boundary for viewport rendering before the first custom WGPU renderer lands.
+- Keep surface creation, texture upload, and draw passes out of `AppData`.
+- Prefer a dedicated GPU adapter module over scattering WGPU setup across the UI code.
+
+## Minimal first app shell
+
+The smallest durable shell is:
+
+- a stable top command bar,
+- a left or right dock for section-specific controls,
+- a central viewport that can show the CPU preview now and a GPU surface later,
+- a status bar for file paths, script status, and render feedback,
+- a tiny about/help dialog for orientation.
+
+That is already enough to validate layout, interaction, and state management without pretending the GPU renderer exists yet.
+
+## Testing strategy
+
+Keep the test surface split the same way as the code.
+
+### Unit tests
+
+- `AppData` reducers and state transitions.
+- `UiShellState` navigation order and section metadata.
+- render-mode selection and preview-state plumbing.
+- scene / script / path / importer behavior.
+
+### Smoke commands
+
+Use existing CLI and app entry points as smoke tests for the architecture:
+
+- `cargo run -- info`
+- `cargo run -- render --preset fractal --seed 1337 --width 64 --height 64 --output /tmp/openvistapro-fractal.png`
+- `cargo run --features app --bin openvistapro_app` when a display is available
+
+### Later integration tests
+
+Once the GPU viewport exists, add lightweight screenshot or frame-smoke coverage around the viewport adapter rather than baking WGPU assumptions into the core state model.
+
+## Next tasks
+
+1. Add a small viewport abstraction so the egui shell can switch between CPU preview and a future GPU backend without changing `AppData`.
+2. Keep the CPU renderer as the reference implementation for deterministic validation.
+3. Add a dedicated GPU adapter module only when the WGPU surface truly exists.
+4. Revisit a workspace split after the GPU path forces a second hard boundary.
+5. Keep `docs/knowledgebase/architecture-notes.md` and `docs/knowledgebase/ui-panel-map.md` aligned with any later shell or viewport changes.
+
+## Recommendation summary
+
+Stay monolithic now, keep egui as the host shell, and treat WGPU as a future viewport backend rather than the reason to split the crate today. That gives OpenVistaPro the fastest path to a usable interactive app while protecting the CLI and deterministic renderer from churn.

src/terrain_gen.rs

@@ -1,29 +1,104 @@
 use crate::terrain::{HeightGrid, TerrainError};
 
-const OCTAVE_COUNT: usize = 4;
-const BASE_FREQUENCY: f32 = 2.0;
-const LACUNARITY: f32 = 2.0;
-const GAIN: f32 = 0.5;
+const DEFAULT_OCTAVE_COUNT: usize = 4;
+const DEFAULT_BASE_FREQUENCY: f32 = 2.0;
+const DEFAULT_LACUNARITY: f32 = 2.0;
+const DEFAULT_GAIN: f32 = 0.5;
 const LATTICE_SEED_STEP: u64 = 0x9E37_79B9_7F4A_7C15;
 const LATTICE_X_MUL: u64 = 0x9E37_79B9_7F4A_7C15;
 const LATTICE_Y_MUL: u64 = 0xC2B2_AE3D_27D4_EB4F;
 const FINALIZER_MUL1: u64 = 0xBF58_476D_1CE4_E5B9;
 const FINALIZER_MUL2: u64 = 0x94D0_49BB_1331_11EB;
 
+fn sanitize_positive(value: f32, fallback: f32) -> f32 {
+    if value.is_finite() && value > 0.0 {
+        value
+    } else {
+        fallback
+    }
+}
+
+fn sanitize_gain(value: f32) -> f32 {
+    if value.is_finite() {
+        value.clamp(0.0, 1.0)
+    } else {
+        DEFAULT_GAIN
+    }
+}
+
+/// Tunable fractal-noise controls for procedural terrain generation.
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub struct TerrainGenerationSettings {
+    octave_count: usize,
+    base_frequency: f32,
+    lacunarity: f32,
+    gain: f32,
+}
+
+impl TerrainGenerationSettings {
+    /// Build a settings bundle, normalizing obviously invalid inputs.
+    pub fn new(octave_count: usize, base_frequency: f32, lacunarity: f32, gain: f32) -> Self {
+        Self {
+            octave_count: octave_count.max(1),
+            base_frequency: sanitize_positive(base_frequency, DEFAULT_BASE_FREQUENCY),
+            lacunarity: sanitize_positive(lacunarity, DEFAULT_LACUNARITY),
+            gain: sanitize_gain(gain),
+        }
+    }
+
+    pub fn octave_count(&self) -> usize {
+        self.octave_count
+    }
+
+    pub fn base_frequency(&self) -> f32 {
+        self.base_frequency
+    }
+
+    pub fn lacunarity(&self) -> f32 {
+        self.lacunarity
+    }
+
+    pub fn gain(&self) -> f32 {
+        self.gain
+    }
+}
+
+impl Default for TerrainGenerationSettings {
+    fn default() -> Self {
+        Self::new(
+            DEFAULT_OCTAVE_COUNT,
+            DEFAULT_BASE_FREQUENCY,
+            DEFAULT_LACUNARITY,
+            DEFAULT_GAIN,
+        )
+    }
+}
+
 /// Describes a terrain generation request.
 ///
 /// Specs are immutable once constructed; identical specs always yield
 /// identical [`HeightGrid`]s from a given [`TerrainGenerator`].
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq)]
 pub struct TerrainGenerationSpec {
     seed: u64,
     width: u32,
     height: u32,
+    settings: TerrainGenerationSettings,
 }
 
 impl TerrainGenerationSpec {
     /// Creates a spec, rejecting zero-sized grids with [`TerrainError::ZeroDimension`].
     pub fn new(seed: u64, width: u32, height: u32) -> Result<Self, TerrainError> {
+        Self::with_settings(seed, width, height, TerrainGenerationSettings::default())
+    }
+
+    /// Creates a spec with explicit fractal settings.
+    pub fn with_settings(
+        seed: u64,
+        width: u32,
+        height: u32,
+        settings: TerrainGenerationSettings,
+    ) -> Result<Self, TerrainError> {
         if width == 0 || height == 0 {
             return Err(TerrainError::ZeroDimension);
         }
@@ -31,9 +106,28 @@ impl TerrainGenerationSpec {
             seed,
             width,
             height,
+            settings,
         })
     }
 
+    /// Convenience constructor for callers that want to override the default settings inline.
+    pub fn with_parameters(
+        seed: u64,
+        width: u32,
+        height: u32,
+        octave_count: usize,
+        base_frequency: f32,
+        lacunarity: f32,
+        gain: f32,
+    ) -> Result<Self, TerrainError> {
+        Self::with_settings(
+            seed,
+            width,
+            height,
+            TerrainGenerationSettings::new(octave_count, base_frequency, lacunarity, gain),
+        )
+    }
+
     pub fn seed(&self) -> u64 {
         self.seed
     }
@@ -45,6 +139,10 @@ impl TerrainGenerationSpec {
     pub fn height(&self) -> u32 {
         self.height
     }
+
+    pub fn settings(&self) -> TerrainGenerationSettings {
+        self.settings
+    }
 }
 
 /// Produces a [`HeightGrid`] from a [`TerrainGenerationSpec`].
@@ -116,18 +214,18 @@ fn value_noise(seed: u64, x: f32, y: f32) -> f32 {
 }
 
 #[inline]
-fn fbm_height(seed: u64, x: f32, y: f32) -> f32 {
+fn fbm_height(seed: u64, x: f32, y: f32, settings: TerrainGenerationSettings) -> f32 {
     let mut total = 0.0;
     let mut amplitude = 1.0;
-    let mut frequency = BASE_FREQUENCY;
+    let mut frequency = settings.base_frequency();
     let mut amplitude_sum = 0.0;
 
-    for octave in 0..OCTAVE_COUNT {
+    for octave in 0..settings.octave_count() {
         let octave_seed = seed.wrapping_add((octave as u64).wrapping_mul(LATTICE_SEED_STEP));
         total += amplitude * value_noise(octave_seed, x * frequency, y * frequency);
         amplitude_sum += amplitude;
-        amplitude *= GAIN;
-        frequency *= LACUNARITY;
+        amplitude *= settings.gain();
+        frequency *= settings.lacunarity();
     }
 
     total / amplitude_sum
@@ -142,7 +240,7 @@ impl TerrainGenerator for DeterministicTerrainGenerator {
             let ny = normalized_coord(y, height);
             for x in 0..width {
                 let nx = normalized_coord(x, width);
-                samples.push(fbm_height(spec.seed(), nx, ny));
+                samples.push(fbm_height(spec.seed(), nx, ny, spec.settings()));
             }
         }
         HeightGrid::new(width, height, samples)
@@ -165,6 +263,28 @@ mod tests {
         );
     }
 
+    #[test]
+    fn settings_normalize_invalid_inputs() {
+        let settings = TerrainGenerationSettings::new(0, -1.0, 0.0, 2.0);
+        assert_eq!(settings.octave_count(), 1);
+        assert_eq!(settings.base_frequency(), DEFAULT_BASE_FREQUENCY);
+        assert_eq!(settings.lacunarity(), DEFAULT_LACUNARITY);
+        assert_eq!(settings.gain(), 1.0);
+    }
+
+    #[test]
+    fn with_parameters_exposes_custom_settings() {
+        let spec = TerrainGenerationSpec::with_parameters(7, 8, 6, 2, 1.5, 3.0, 0.25).unwrap();
+        let settings = spec.settings();
+        assert_eq!(spec.seed(), 7);
+        assert_eq!(spec.width(), 8);
+        assert_eq!(spec.height(), 6);
+        assert_eq!(settings.octave_count(), 2);
+        assert_eq!(settings.base_frequency(), 1.5);
+        assert_eq!(settings.lacunarity(), 3.0);
+        assert_eq!(settings.gain(), 0.25);
+    }
+
     #[test]
     fn generate_matches_spec_dimensions() {
         let spec = TerrainGenerationSpec::new(99, 8, 5).unwrap();
@@ -185,4 +305,35 @@ mod tests {
             .unwrap();
         assert_ne!(a.sample(0, 0), b.sample(0, 0));
     }
+
+    #[test]
+    fn generate_is_bounded_and_deterministic_for_exact_samples() {
+        let spec = TerrainGenerationSpec::new(1337, 4, 4).unwrap();
+        let generator = DeterministicTerrainGenerator::new();
+        let first = generator.generate(&spec).unwrap();
+        let second = generator.generate(&spec).unwrap();
+
+        let expected = [
+            [0.706_783_f32, 0.390_681, 0.571_295, 0.454_375],
+            [0.667_478, 0.549_327, 0.476_506, 0.335_594],
+            [0.603_516, 0.451_503, 0.516_397, 0.357_081],
+            [0.308_838, 0.295_558, 0.570_510, 0.666_683],
+        ];
+
+        for y in 0..4 {
+            for x in 0..4 {
+                let sample = first.sample(x, y).unwrap();
+                assert!((0.0..=1.0).contains(&sample));
+                assert_eq!(sample, second.sample(x, y).unwrap());
+                assert!(
+                    (sample - expected[y as usize][x as usize]).abs() < 1e-6,
+                    "mismatch at ({x}, {y})"
+                );
+            }
+        }
+
+        let (min, max) = first.min_max().unwrap();
+        assert!(min >= 0.0);
+        assert!(max <= 1.0);
+    }
 }