diff --git a/.cursor/plans/fbx_external_texture_dependencies_2026-07-13.plan.md b/.cursor/plans/fbx_external_texture_dependencies_2026-07-13.plan.md new file mode 100644 index 0000000..8a705ee --- /dev/null +++ b/.cursor/plans/fbx_external_texture_dependencies_2026-07-13.plan.md @@ -0,0 +1,44 @@ +# FBX External Texture Dependencies + +**Issue:** Gitea #58 (`BS-JD-210`) + +## Goal + +Make FBX external texture discovery deterministic from import through runtime loading. Missing or +unsafe references must be reported before the Asset Browser queues Bevy loads, while an explicit +Authoring Override remains a supported untextured workflow. + +## Scope + +1. Add one path-safe FBX texture-reference resolver shared by the local `bevy_ufbx` loader and the + editor asset pipeline. Normalize separators, deduplicate references, retain relative `textures/` + and `.fbm/` layouts, and reject absolute or parent-traversing references. +2. Decode external images through the FBX `LoadContext`, reuse one handle per normalized path, and + consolidate unavailable dependency logging so duplicate ufbx texture records cannot create + repeated asset-server failures. +3. Generate static-mesh dependencies from parsed FBX texture references. Treat them as required for + Source Materials and as declared-but-optional for Authoring Override during read-only project + validation. +4. Replace the FBX-only file copy with a staged bundle transaction that preflights every referenced + file, preserves relative sidecar layout, backs up overwritten destinations, and rolls back a + partial commit. +5. Preflight source-material thumbnails, retain neutral direct FBX mesh/model previews, and expose a + stable failure reason plus missing dependency status in the Asset Browser. +6. Deliberately normalize the committed painted-chair fixture to Authoring Override rather than add + large absent 2K textures. + +## Verification + +- Unit tests for sibling `textures/`, `.fbm/`, duplicate references, missing files, path traversal, + destination symlink containment, transactional preservation, consolidated project findings, and + read-only validation. +- `cargo fmt`, focused crate tests, workspace Clippy, workspace tests, `validate-levels`, and + `validate-samples`. Packaged tests remain deferred by project-owner direction. +- Native editor QA for the painted chair and an intentionally broken Source Materials variant, + including log inspection and screenshots published as ordinary Gitea attachments. + +## Documentation + +- Record the cross-crate dependency/override contract in ADR 0044. +- Update editor architecture, Asset Browser workflow, root controls/checklist, documentation maps, + and an issue-specific evaluation record. diff --git a/README.md b/README.md index c1d9e10..f4f1e16 100644 --- a/README.md +++ b/README.md @@ -378,6 +378,7 @@ The `.vscode/` folder is preconfigured: - [ADR 0034: Registry-driven Authoring Components](docs/adr/0034-registry-driven-authoring-components.md) - [ADR 0037: Collaborative Authored-File Safety](docs/adr/0037-collaborative-authored-file-safety.md) - [ADR 0043: Content-Addressed Import Fingerprints](docs/adr/0043-content-addressed-import-fingerprints.md) +- [ADR 0044: Sandboxed FBX External Texture Dependencies](docs/adr/0044-sandboxed-fbx-external-texture-dependencies.md) ## Project Layout @@ -432,6 +433,9 @@ crates/ - [x] Deterministic imported-source fingerprints for models, textures, and audio; BLAKE3-backed static/animation manifests; byte-preserving equivalent refresh; and read-only validator checkout assertions ([ADR 0043](docs/adr/0043-content-addressed-import-fingerprints.md), [evaluation](docs/editor/evaluations/deterministic-asset-fingerprints/), [Gitea #56](https://git.spacetrainclubhouse.com/Falling-Metal-Interactive/Blacksite/issues/56)) +- [x] Sandboxed FBX external-texture discovery for sibling and `.fbm/` layouts, transactional + referenced-bundle import, manifest-first consolidated validation, deduplicated loader reads, and + stable Asset Browser dependency status ([ADR 0044](docs/adr/0044-sandboxed-fbx-external-texture-dependencies.md), [Gitea #58](https://git.spacetrainclubhouse.com/Falling-Metal-Interactive/Blacksite/issues/58)) - [x] Asset Browser expandable model subasset shelves, independent mesh/material/texture thumbnails, staged import/material details with shader-schema parameters, context actions, and trash-first file removal - [x] Audio clip catalog/import foundation for Ogg, WAV, MP3, and FLAC with dedicated filtering, file details, and stable runtime-resolvable asset references - [x] Audio source/listener authoring, non-dirty spatial audition, viewport icons/range gizmos, stable buses, PIE/runtime parity, device diagnostics, and shared release validation ([ADR 0030](docs/adr/0030-audio-authoring-and-bus-schema.md); production acceptance completed in [Gitea #47](https://git.spacetrainclubhouse.com/Falling-Metal-Interactive/Blacksite/issues/47)) @@ -479,8 +483,11 @@ crates/ - The editor asset browser is filesystem-backed with folder/tree navigation, grid/list views, texture and model thumbnails (glTF albedo fast-path; offscreen render studio for FBX and untextured models), material sphere thumbnails, search/filter/sort controls, expandable model - subasset shelves, and a staged details pane. **File → Import Assets** - accepts glTF/GLB and **FBX** (binary; copies sibling `.fbm` texture folders when present). + subasset shelves, and a staged details pane. **File -> Import Assets** + accepts glTF/GLB and binary **FBX**. FBX import parses and transactionally preserves every safe + referenced sibling `textures/` or `.fbm/` file while rejecting traversal and external absolute + paths before project content changes. Missing source textures appear once in validation and Asset + Browser dependency status; **Authoring Override** deliberately permits an untextured model. Model assets generate normalized model manifests under `assets/meshes/generated/`; drag/drop uses **Renderable Asset (Auto)**. Unrigged sources use `StaticMeshRenderer` with imported asset refs and optional separate static mesh colliders. Skin-bound or animated sources and their subasset placement use diff --git a/assets/.index/registry.ron b/assets/.index/registry.ron index 2400634..743f4c4 100644 --- a/assets/.index/registry.ron +++ b/assets/.index/registry.ron @@ -458,12 +458,16 @@ lod0_only: true, placement_mode: StaticAsset, hierarchy_mode: SingleActor, - material_policy: SourceMaterials, + material_policy: AuthoringOverride, static_mesh_manifest_path: Some("assets/meshes/generated/b98ef565-3500-49e7-9935-f685fa9b2594.static_mesh.ron"), animation_manifest_path: Some("assets/animations/generated/b98ef565-3500-49e7-9935-f685fa9b2594.animation.ron"), default_animation_clip_id: None, ), - dependencies: [], + dependencies: [ + "assets/models/textures/painted_wooden_chair_02_diff_2k.jpg", + "assets/models/textures/painted_wooden_chair_02_nor_gl_2k.exr", + "assets/models/textures/painted_wooden_chair_02_rough_2k.exr", + ], ), ( id: ("3f63f359-45eb-4cb2-8970-71921cbd7bd0"), @@ -769,4 +773,4 @@ ), dependencies: [], ), -] \ No newline at end of file +] diff --git a/assets/meshes/generated/b98ef565-3500-49e7-9935-f685fa9b2594.static_mesh.ron b/assets/meshes/generated/b98ef565-3500-49e7-9935-f685fa9b2594.static_mesh.ron index 530c1ee..7110e7d 100644 --- a/assets/meshes/generated/b98ef565-3500-49e7-9935-f685fa9b2594.static_mesh.ron +++ b/assets/meshes/generated/b98ef565-3500-49e7-9935-f685fa9b2594.static_mesh.ron @@ -9,7 +9,11 @@ byte_len: 59964, content_hash: "b12973a62dcb44589e380ea833eade726ae98a86c81084c842ee3801866d6a46", ), - dependencies: [], + dependencies: [ + "assets/models/textures/painted_wooden_chair_02_diff_2k.jpg", + "assets/models/textures/painted_wooden_chair_02_nor_gl_2k.exr", + "assets/models/textures/painted_wooden_chair_02_rough_2k.exr", + ], ), import: ( scale: 1.0, @@ -17,7 +21,7 @@ lod0_only: true, placement_mode: StaticAsset, hierarchy_mode: SingleActor, - material_policy: SourceMaterials, + material_policy: AuthoringOverride, ), metadata: ( mesh_count: 1, @@ -48,4 +52,4 @@ ), ], warnings: [], -) \ No newline at end of file +) diff --git a/crates/editor/src/assets/catalog.rs b/crates/editor/src/assets/catalog.rs index 05205a2..f2f40f0 100644 --- a/crates/editor/src/assets/catalog.rs +++ b/crates/editor/src/assets/catalog.rs @@ -1455,7 +1455,7 @@ pub fn import_external_assets(paths: &[PathBuf]) -> Result { }; let dest_dir = import_dir_for_extension(extension); if extension.eq_ignore_ascii_case("fbx") { - super::import::copy_fbx_with_sidecar(source, Path::new(dest_dir))?; + super::import::copy_fbx_bundle(source, Path::new(dest_dir))?; copied += 1; continue; } diff --git a/crates/editor/src/assets/import.rs b/crates/editor/src/assets/import.rs index bf70cec..4e333f5 100644 --- a/crates/editor/src/assets/import.rs +++ b/crates/editor/src/assets/import.rs @@ -1,71 +1,564 @@ -//! External asset import helpers (FBX sidecar folders, etc.). +//! External asset bundle inspection and transactional import. +use std::collections::BTreeMap; use std::fs; -use std::path::{Path, PathBuf}; +use std::path::{Component, Path, PathBuf}; -/// Copies an FBX file and its sibling `.fbm` embedded-texture folder when present. -pub fn copy_fbx_with_sidecar(source: &Path, dest_dir: &Path) -> Result<(), String> { - let Some(file_name) = source.file_name() else { - return Err("FBX import path has no file name".into()); - }; - fs::create_dir_all(dest_dir) - .map_err(|err| format!("could not create {}: {err}", dest_dir.display()))?; - let dest = dest_dir.join(file_name); - fs::copy(source, &dest).map_err(|err| { +use bevy_ufbx::texture::external_texture_paths; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct FbxDependencyInspection { + pub relative_paths: Vec, + pub resolved_paths: Vec, + pub missing_paths: Vec, +} + +/// Parses every external FBX texture reference without loading it through Bevy. +pub(crate) fn inspect_fbx_dependencies(source: &Path) -> Result { + let bytes = fs::read(source) + .map_err(|error| format!("could not read FBX {}: {error}", source.display()))?; + let filename_hint = source.to_string_lossy(); + let scene = ufbx::load_memory( + &bytes, + ufbx::LoadOpts { + target_unit_meters: 1.0, + target_axes: ufbx::CoordinateAxes::right_handed_y_up(), + filename: ufbx::StringOpt::Ref(&filename_hint), + ..Default::default() + }, + ) + .map_err(|error| format!("could not parse FBX {}: {error:?}", source.display()))?; + let relative_paths = external_texture_paths(&scene).map_err(|errors| { format!( - "could not copy {} to {}: {err}", + "unsafe FBX texture reference(s) in {}: {}", source.display(), - dest.display() + errors + .into_iter() + .map(|error| error.to_string()) + .collect::>() + .join("; ") ) })?; + let source_root = source.parent().unwrap_or_else(|| Path::new("")); + let canonical_root = fs::canonicalize(source_root).map_err(|error| { + format!( + "could not resolve FBX source directory {}: {error}", + source_root.display() + ) + })?; + let mut resolved_paths = Vec::new(); + let mut missing_paths = Vec::new(); + for relative_path in &relative_paths { + let resolved = source_root.join(relative_path); + if !resolved.is_file() { + missing_paths.push(resolved.clone()); + resolved_paths.push(resolved); + continue; + } + let canonical = fs::canonicalize(&resolved).map_err(|error| { + format!( + "could not resolve FBX dependency {}: {error}", + resolved.display() + ) + })?; + if !canonical.starts_with(&canonical_root) { + return Err(format!( + "FBX dependency {} resolves outside source directory {}", + resolved.display(), + source_root.display() + )); + } + resolved_paths.push(resolved); + } + missing_paths.sort(); + Ok(FbxDependencyInspection { + relative_paths, + resolved_paths, + missing_paths, + }) +} - if let Some(fbm_dir) = fbm_sidecar_dir(source) { - if fbm_dir.is_dir() { - let dest_fbm = dest_dir.join(fbm_dir.file_name().unwrap_or_default()); - copy_dir_recursive(&fbm_dir, &dest_fbm)?; +/// Returns one stable browser-facing error for all missing FBX source textures. +pub(crate) fn validate_fbx_dependencies(source: &Path) -> Result<(), String> { + let inspection = inspect_fbx_dependencies(source)?; + if inspection.missing_paths.is_empty() { + return Ok(()); + } + Err(format!( + "missing {} FBX source texture(s): {}", + inspection.missing_paths.len(), + inspection + .missing_paths + .iter() + .map(|path| path.to_string_lossy().replace('\\', "/")) + .collect::>() + .join(", ") + )) +} + +/// Copies an FBX and every referenced external texture as one staged filesystem transaction. +pub fn copy_fbx_bundle(source: &Path, dest_dir: &Path) -> Result<(), String> { + let inspection = inspect_fbx_dependencies(source)?; + if !inspection.missing_paths.is_empty() { + return Err(format!( + "FBX import is missing {} required texture(s): {}", + inspection.missing_paths.len(), + inspection + .missing_paths + .iter() + .map(|path| path.display().to_string()) + .collect::>() + .join(", ") + )); + } + let file_name = source + .file_name() + .ok_or_else(|| "FBX import path has no file name".to_string())?; + let mut entries = BTreeMap::new(); + entries.insert(PathBuf::from(file_name), source.to_path_buf()); + for (relative, resolved) in inspection + .relative_paths + .iter() + .zip(inspection.resolved_paths.iter()) + { + let relative = PathBuf::from(relative); + if let Some(existing) = entries.insert(relative.clone(), resolved.clone()) { + if existing != *resolved { + return Err(format!( + "FBX import maps multiple source files to {}", + relative.display() + )); + } } } - Ok(()) + copy_bundle_transactionally(&entries, dest_dir) } -fn fbm_sidecar_dir(source: &Path) -> Option { - let stem = source.file_stem()?.to_str()?; - let parent = source.parent()?; - Some(parent.join(format!("{stem}.fbm"))) -} +fn copy_bundle_transactionally( + entries: &BTreeMap, + dest_dir: &Path, +) -> Result<(), String> { + fs::create_dir_all(dest_dir) + .map_err(|error| format!("could not create {}: {error}", dest_dir.display()))?; + let canonical_dest_dir = fs::canonicalize(dest_dir).map_err(|error| { + format!( + "could not resolve FBX import destination {}: {error}", + dest_dir.display() + ) + })?; + for relative in entries.keys() { + validate_bundle_relative_path(relative)?; + validate_existing_destination_parents(dest_dir, &canonical_dest_dir, relative)?; + let target = dest_dir.join(relative); + if target.is_dir() { + return Err(format!( + "FBX import target {} is an existing directory", + target.display() + )); + } + } -fn copy_dir_recursive(source: &Path, dest: &Path) -> Result<(), String> { - fs::create_dir_all(dest) - .map_err(|err| format!("could not create {}: {err}", dest.display()))?; - for entry in fs::read_dir(source).map_err(|err| err.to_string())? { - let entry = entry.map_err(|err| err.to_string())?; - let src_path = entry.path(); - let dest_path = dest.join(entry.file_name()); - if src_path.is_dir() { - copy_dir_recursive(&src_path, &dest_path)?; - } else { - fs::copy(&src_path, &dest_path).map_err(|err| { + let stage_root = dest_dir.join(format!(".blacksite-import-{}", uuid::Uuid::new_v4())); + let staged_root = stage_root.join("new"); + let backup_root = stage_root.join("backup"); + let stage_result = (|| { + for (relative, source) in entries { + let staged = staged_root.join(relative); + if let Some(parent) = staged.parent() { + fs::create_dir_all(parent).map_err(|error| { + format!( + "could not create import staging {}: {error}", + parent.display() + ) + })?; + } + fs::copy(source, &staged).map_err(|error| { format!( - "could not copy {} to {}: {err}", - src_path.display(), - dest_path.display() + "could not stage FBX bundle file {} as {}: {error}", + source.display(), + relative.display() ) })?; } + commit_staged_bundle( + entries, + dest_dir, + &canonical_dest_dir, + &staged_root, + &backup_root, + ) + })(); + let cleanup_result = fs::remove_dir_all(&stage_root); + match (stage_result, cleanup_result) { + (Err(error), _) => Err(error), + (Ok(()), Ok(())) => Ok(()), + (Ok(()), Err(error)) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + (Ok(()), Err(error)) => { + bevy::log::warn!( + "FBX bundle imported but staging cleanup failed for {}: {error}", + stage_root.display() + ); + Ok(()) + } + } +} + +fn validate_bundle_relative_path(relative: &Path) -> Result<(), String> { + if relative.as_os_str().is_empty() + || !relative + .components() + .all(|component| matches!(component, Component::Normal(_))) + { + return Err(format!( + "FBX import target path {} is not a safe relative path", + relative.display() + )); } Ok(()) } +fn validate_existing_destination_parents( + dest_dir: &Path, + canonical_dest_dir: &Path, + relative: &Path, +) -> Result<(), String> { + let mut current = dest_dir.to_path_buf(); + let Some(parent) = relative.parent() else { + return Ok(()); + }; + for component in parent.components() { + let Component::Normal(segment) = component else { + return Err(format!( + "FBX import target path {} is not a safe relative path", + relative.display() + )); + }; + current.push(segment); + let metadata = match fs::symlink_metadata(¤t) { + Ok(metadata) => metadata, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => break, + Err(error) => { + return Err(format!( + "could not inspect FBX import target {}: {error}", + current.display() + )); + } + }; + if metadata.file_type().is_symlink() { + return Err(format!( + "FBX import target parent {} is a symbolic link", + current.display() + )); + } + if !metadata.is_dir() { + return Err(format!( + "FBX import target parent {} is not a directory", + current.display() + )); + } + let canonical = fs::canonicalize(¤t).map_err(|error| { + format!( + "could not resolve FBX import target parent {}: {error}", + current.display() + ) + })?; + if !canonical.starts_with(canonical_dest_dir) { + return Err(format!( + "FBX import target parent {} resolves outside destination {}", + current.display(), + dest_dir.display() + )); + } + } + Ok(()) +} + +fn commit_staged_bundle( + entries: &BTreeMap, + dest_dir: &Path, + canonical_dest_dir: &Path, + staged_root: &Path, + backup_root: &Path, +) -> Result<(), String> { + let mut installed = Vec::new(); + let mut backups = Vec::new(); + for relative in entries.keys() { + let target = dest_dir.join(relative); + let staged = staged_root.join(relative); + if let Some(parent) = target.parent() { + if let Err(error) = fs::create_dir_all(parent) { + return rollback_bundle( + &installed, + &backups, + format!( + "could not create import target {}: {error}", + parent.display() + ), + ); + } + let canonical_parent = match fs::canonicalize(parent) { + Ok(canonical_parent) => canonical_parent, + Err(error) => { + return rollback_bundle( + &installed, + &backups, + format!( + "could not resolve import target {}: {error}", + parent.display() + ), + ); + } + }; + if !canonical_parent.starts_with(canonical_dest_dir) { + return rollback_bundle( + &installed, + &backups, + format!( + "FBX import target {} resolves outside destination {}", + parent.display(), + dest_dir.display() + ), + ); + } + } + if target.exists() { + let backup = backup_root.join(relative); + if let Some(parent) = backup.parent() { + if let Err(error) = fs::create_dir_all(parent) { + return rollback_bundle( + &installed, + &backups, + format!( + "could not create import backup {}: {error}", + parent.display() + ), + ); + } + } + if let Err(error) = fs::rename(&target, &backup) { + return rollback_bundle( + &installed, + &backups, + format!( + "could not back up import target {}: {error}", + target.display() + ), + ); + } + backups.push((backup, target.clone())); + } + if let Err(error) = fs::rename(&staged, &target) { + return rollback_bundle( + &installed, + &backups, + format!( + "could not publish import target {}: {error}", + target.display() + ), + ); + } + installed.push(target); + } + Ok(()) +} + +fn rollback_bundle( + installed: &[PathBuf], + backups: &[(PathBuf, PathBuf)], + cause: String, +) -> Result<(), String> { + let mut rollback_errors = Vec::new(); + for target in installed.iter().rev() { + if let Err(error) = fs::remove_file(target) { + if error.kind() != std::io::ErrorKind::NotFound { + rollback_errors.push(format!("remove {}: {error}", target.display())); + } + } + } + for (backup, target) in backups.iter().rev() { + if let Err(error) = fs::rename(backup, target) { + rollback_errors.push(format!( + "restore {} to {}: {error}", + backup.display(), + target.display() + )); + } + } + if rollback_errors.is_empty() { + Err(cause) + } else { + Err(format!( + "{cause}; rollback also failed: {}", + rollback_errors.join("; ") + )) + } +} + #[cfg(test)] mod tests { - use super::fbm_sidecar_dir; - use std::path::Path; + use super::*; + + fn temp_root(label: &str) -> PathBuf { + std::env::temp_dir().join(format!("blacksite-fbx-{label}-{}", uuid::Uuid::new_v4())) + } + + fn committed_chair_bytes() -> Vec { + fs::read( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../assets/models/painted_wooden_chair_02_2k.fbx"), + ) + .unwrap() + } + + fn replace_equal_length(bytes: &mut [u8], from: &[u8], to: &[u8]) -> usize { + assert_eq!(from.len(), to.len()); + let mut count = 0; + let mut offset = 0; + while let Some(index) = bytes[offset..] + .windows(from.len()) + .position(|window| window == from) + { + let start = offset + index; + bytes[start..start + from.len()].copy_from_slice(to); + offset = start + from.len(); + count += 1; + } + count + } + + fn write_textures(root: &Path, folder: &str) { + let folder = root.join(folder); + fs::create_dir_all(&folder).unwrap(); + for name in [ + "painted_wooden_chair_02_diff_2k.jpg", + "painted_wooden_chair_02_nor_gl_2k.exr", + "painted_wooden_chair_02_rough_2k.exr", + ] { + fs::write(folder.join(name), name.as_bytes()).unwrap(); + } + } #[test] - fn fbm_sidecar_path() { + fn committed_chair_reports_one_stable_missing_dependency_state() { + let path = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../assets/models/painted_wooden_chair_02_2k.fbx"); + + let error = validate_fbx_dependencies(&path).unwrap_err(); + + assert!(error.starts_with("missing 3 FBX source texture(s):")); + assert_eq!(error.matches("painted_wooden_chair_02_").count(), 3); + } + + #[test] + fn sibling_texture_bundle_is_copied_with_relative_layout() { + let root = temp_root("sibling"); + let source_root = root.join("source"); + let destination = root.join("destination"); + fs::create_dir_all(&source_root).unwrap(); + let source = source_root.join("chair.fbx"); + fs::write(&source, committed_chair_bytes()).unwrap(); + write_textures(&source_root, "textures"); + + copy_fbx_bundle(&source, &destination).unwrap(); + + assert!(destination.join("chair.fbx").is_file()); + assert!(destination + .join("textures/painted_wooden_chair_02_diff_2k.jpg") + .is_file()); + assert!(!destination.read_dir().unwrap().any(|entry| entry + .unwrap() + .file_name() + .to_string_lossy() + .starts_with(".blacksite-import-"))); + fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn fbm_texture_bundle_is_copied_with_relative_layout() { + let root = temp_root("fbm"); + let source_root = root.join("source"); + let destination = root.join("destination"); + fs::create_dir_all(&source_root).unwrap(); + let source = source_root.join("chair.fbx"); + let mut bytes = committed_chair_bytes(); + assert!(replace_equal_length(&mut bytes, b"textures/", b"test.fbm/") > 0); + fs::write(&source, bytes).unwrap(); + write_textures(&source_root, "test.fbm"); + + copy_fbx_bundle(&source, &destination).unwrap(); + + assert!(destination + .join("test.fbm/painted_wooden_chair_02_diff_2k.jpg") + .is_file()); + fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn traversal_reference_is_rejected_before_destination_changes() { + let root = temp_root("traversal"); + let source_root = root.join("source"); + let destination = root.join("destination"); + fs::create_dir_all(&source_root).unwrap(); + fs::create_dir_all(&destination).unwrap(); + let source = source_root.join("chair.fbx"); + let mut bytes = committed_chair_bytes(); + assert!(replace_equal_length(&mut bytes, b"textures/", b"../evil//") > 0); + fs::write(&source, bytes).unwrap(); + fs::write(destination.join("chair.fbx"), b"original").unwrap(); + + let error = copy_fbx_bundle(&source, &destination).unwrap_err(); + + assert!(error.contains("parent traversal")); assert_eq!( - fbm_sidecar_dir(Path::new("/tmp/Character.fbx")), - Some(Path::new("/tmp/Character.fbm").to_path_buf()) + fs::read(destination.join("chair.fbx")).unwrap(), + b"original" ); + fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn missing_bundle_preserves_existing_destination() { + let root = temp_root("missing"); + let source_root = root.join("source"); + let destination = root.join("destination"); + fs::create_dir_all(&source_root).unwrap(); + fs::create_dir_all(&destination).unwrap(); + let source = source_root.join("chair.fbx"); + fs::write(&source, committed_chair_bytes()).unwrap(); + fs::write(destination.join("chair.fbx"), b"original").unwrap(); + + let error = copy_fbx_bundle(&source, &destination).unwrap_err(); + + assert!(error.contains("missing 3 required texture(s)")); + assert_eq!( + fs::read(destination.join("chair.fbx")).unwrap(), + b"original" + ); + fs::remove_dir_all(root).unwrap(); + } + + #[cfg(unix)] + #[test] + fn destination_symlink_escape_is_rejected_before_external_changes() { + use std::os::unix::fs::symlink; + + let root = temp_root("destination-symlink"); + let source_root = root.join("source"); + let destination = root.join("destination"); + let outside = root.join("outside"); + fs::create_dir_all(&source_root).unwrap(); + fs::create_dir_all(&destination).unwrap(); + fs::create_dir_all(&outside).unwrap(); + let source = source_root.join("chair.fbx"); + fs::write(&source, committed_chair_bytes()).unwrap(); + write_textures(&source_root, "textures"); + symlink(&outside, destination.join("textures")).unwrap(); + + let error = copy_fbx_bundle(&source, &destination).unwrap_err(); + + assert!(error.contains("symbolic link")); + assert!(!destination.join("chair.fbx").exists()); + assert!(!outside.join("painted_wooden_chair_02_diff_2k.jpg").exists()); + fs::remove_dir_all(root).unwrap(); } } diff --git a/crates/editor/src/assets/static_mesh.rs b/crates/editor/src/assets/static_mesh.rs index c64a6b7..32bece9 100644 --- a/crates/editor/src/assets/static_mesh.rs +++ b/crates/editor/src/assets/static_mesh.rs @@ -7,6 +7,7 @@ use bevy::gltf::GltfAssetLabel; use bevy::prelude::*; use bevy_ufbx::label::FbxAssetLabel; use bevy_ufbx::mesh::group_faces_by_material; +use bevy_ufbx::texture::external_texture_paths; use bevy_ufbx::utils::convert_matrix; use serde::{Deserialize, Serialize}; @@ -484,6 +485,7 @@ fn build_fbx_manifest( ufbx::LoadOpts { target_unit_meters: 1.0, target_axes: ufbx::CoordinateAxes::right_handed_y_up(), + filename: ufbx::StringOpt::Ref(&record.path), ..Default::default() }, ) @@ -563,7 +565,7 @@ fn build_fbx_manifest( path: record.path.clone(), format, fingerprint, - dependencies: fbx_dependencies(&record.path), + dependencies: fbx_dependencies(&record.path, &scene)?, }, import, metadata: StaticMeshMetadata { @@ -592,25 +594,26 @@ fn fbx_material_label(scene: &ufbx::Scene, element_id: u32) -> Option { .map(|index| FbxAssetLabel::Material(index).to_string()) } -fn fbx_dependencies(source_path: &str) -> Vec { +fn fbx_dependencies(source_path: &str, scene: &ufbx::Scene) -> Result, String> { let path = Path::new(source_path); - let Some(stem) = path.file_stem().and_then(|stem| stem.to_str()) else { - return Vec::new(); - }; - let Some(parent) = path.parent() else { - return Vec::new(); - }; - let sidecar = parent.join(format!("{stem}.fbm")); - if !sidecar.is_dir() { - return Vec::new(); - } - walkdir::WalkDir::new(sidecar) - .follow_links(false) - .into_iter() - .filter_map(Result::ok) - .filter(|entry| entry.file_type().is_file()) - .map(|entry| entry.path().to_string_lossy().replace('\\', "/")) - .collect() + let parent = path.parent().unwrap_or_else(|| Path::new("")); + external_texture_paths(scene) + .map_err(|errors| { + format!( + "unsafe FBX texture reference(s) in {source_path}: {}", + errors + .into_iter() + .map(|error| error.to_string()) + .collect::>() + .join("; ") + ) + }) + .map(|paths| { + paths + .into_iter() + .map(|relative| parent.join(relative).to_string_lossy().replace('\\', "/")) + .collect() + }) } fn source_format(path: &str) -> Result { @@ -823,4 +826,30 @@ mod tests { assert!(manifest.parts.iter().any(|part| part.skinned)); assert!(renderer.slots.is_empty()); } + + #[test] + fn committed_fbx_records_sibling_texture_dependencies() { + let path = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../assets/models/painted_wooden_chair_02_2k.fbx") + .to_string_lossy() + .into_owned(); + let record = AssetRecord { + id: AssetId::new(), + path, + label: "Painted Chair".into(), + kind_tag: "Model".into(), + source_fingerprint: None, + import_settings: ImportSettings::default(), + dependencies: Vec::new(), + }; + + let manifest = build_static_mesh_manifest(&record).unwrap(); + + assert_eq!(manifest.source.dependencies.len(), 3); + assert!(manifest + .source + .dependencies + .iter() + .all(|path| path.contains("assets/models/textures/painted_wooden_chair_02_"))); + } } diff --git a/crates/editor/src/assets/thumbnails/cache.rs b/crates/editor/src/assets/thumbnails/cache.rs index 2f4a2e7..66fbd60 100644 --- a/crates/editor/src/assets/thumbnails/cache.rs +++ b/crates/editor/src/assets/thumbnails/cache.rs @@ -1,7 +1,9 @@ //! Thumbnail cache for asset browser grid cells. use std::collections::{HashMap, HashSet}; +use std::path::Path; +use bevy::asset::LoadState; use bevy::prelude::*; use bevy_egui::{egui, EguiPrimaryContextPass, EguiTextureHandle, EguiUserTextures}; use egui_phosphor_icons::icons; @@ -79,6 +81,9 @@ impl AssetThumbnailCache { if self.failed.contains_key(&key) { return; } + if !self.texture_source_ready(&key, &path) { + return; + } let handle: Handle = asset_server.load(asset_server_path(&path)); self.pending.insert(key, handle); } @@ -161,7 +166,7 @@ impl AssetThumbnailCache { { return; } - if !self.model_source_ready(&key, &model_path) { + if !self.source_material_ready(&key, &model_path) { return; } if studio.enqueue_source( @@ -224,6 +229,32 @@ impl AssetThumbnailCache { true } + fn texture_source_ready(&mut self, key: &str, texture_path: &str) -> bool { + if Path::new(texture_path).is_file() { + return true; + } + self.mark_studio_failed(key, &format!("missing texture: {texture_path}"), false); + false + } + + fn source_material_ready(&mut self, key: &str, model_path: &str) -> bool { + if !self.model_source_ready(key, model_path) { + return false; + } + if model_path + .rsplit_once('.') + .is_some_and(|(_, extension)| extension.eq_ignore_ascii_case("fbx")) + { + if let Err(reason) = + crate::assets::import::validate_fbx_dependencies(std::path::Path::new(model_path)) + { + self.mark_studio_failed(key, &reason, false); + return false; + } + } + true + } + pub(crate) fn complete_studio_thumbnail( &mut self, key: &str, @@ -268,6 +299,11 @@ impl AssetThumbnailCache { .cloned() .collect(), failed_keys: self.failed.keys().cloned().collect(), + failure_reasons: self + .failed + .iter() + .map(|(key, failure)| (key.clone(), failure.reason.clone())) + .collect(), } } } @@ -277,6 +313,14 @@ fn register_loaded_thumbnails( mut cache: ResMut, mut textures: ResMut, ) { + let failed: Vec<(String, String)> = cache + .pending + .iter() + .filter_map(|(key, handle)| match asset_server.load_state(handle.id()) { + LoadState::Failed(error) => Some((key.clone(), error.to_string())), + _ => None, + }) + .collect(); let ready: Vec<(String, Handle)> = cache .pending .iter() @@ -284,6 +328,10 @@ fn register_loaded_thumbnails( .map(|(key, handle)| (key.clone(), handle.clone())) .collect(); + for (key, error) in failed { + cache.mark_studio_failed(&key, &format!("texture load failed: {error}"), false); + } + for (key, handle) in ready { cache.pending.remove(&key); cache.failed.remove(&key); @@ -397,7 +445,11 @@ pub fn draw_asset_cell_with( }, ); - response + if let Some(reason) = failed { + response.on_hover_text(reason) + } else { + response + } } #[derive(Clone)] @@ -405,6 +457,7 @@ pub struct ThumbnailCacheSnapshot { pub texture_ids: HashMap, pub pending_keys: Vec, pub failed_keys: Vec, + pub failure_reasons: HashMap, } impl ThumbnailCacheSnapshot { @@ -420,6 +473,10 @@ impl ThumbnailCacheSnapshot { self.failed_keys.iter().any(|failed| failed == key) } + pub fn failure_reason_for_key(&self, key: &str) -> Option<&str> { + self.failure_reasons.get(key).map(String::as_str) + } + pub fn texture_for(&self, asset: &EditorAsset) -> Option { self.texture_ids.get(&asset_cache_key(asset)).copied() } @@ -435,6 +492,10 @@ impl ThumbnailCacheSnapshot { .iter() .any(|key| key == &asset_cache_key(asset)) } + + pub fn failure_reason(&self, asset: &EditorAsset) -> Option<&str> { + self.failure_reason_for_key(&asset_cache_key(asset)) + } } pub fn prefetch_folder_thumbnails(world: &mut World, folder: &str) { @@ -534,3 +595,53 @@ pub fn invalidate_on_catalog_refresh(world: &mut World) { world.commands().entity(root).despawn(); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn missing_fbx_material_dependencies_are_one_stable_non_retryable_state() { + let model_path = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../assets/models/painted_wooden_chair_02_2k.fbx") + .to_string_lossy() + .into_owned(); + let mut cache = AssetThumbnailCache::default(); + + assert!(cache.model_source_ready("model", &model_path)); + assert!(!cache.source_material_ready("material", &model_path)); + let first = cache.state("material"); + assert!(!cache.source_material_ready("material", &model_path)); + + assert_eq!(cache.state("material"), first); + let Some(ThumbnailState::Failed { reason, retryable }) = first else { + panic!("missing FBX textures should produce one stable failure"); + }; + assert!(!retryable); + assert!(reason.starts_with("missing 3 FBX source texture(s):")); + assert!(cache.studio_pending.is_empty()); + } + + #[test] + fn missing_texture_is_one_stable_non_retryable_state() { + let path = std::env::temp_dir().join(format!( + "blacksite-missing-thumbnail-{}-{}.png", + std::process::id(), + std::thread::current().name().unwrap_or("test") + )); + let path = path.to_string_lossy().into_owned(); + let mut cache = AssetThumbnailCache::default(); + + assert!(!cache.texture_source_ready("texture", &path)); + let first = cache.state("texture"); + assert!(!cache.texture_source_ready("texture", &path)); + + assert_eq!(cache.state("texture"), first); + let Some(ThumbnailState::Failed { reason, retryable }) = first else { + panic!("missing texture should produce one stable failure"); + }; + assert!(!retryable); + assert_eq!(reason, format!("missing texture: {path}")); + assert!(cache.pending.is_empty()); + } +} diff --git a/crates/editor/src/assets/thumbnails/sources/fbx.rs b/crates/editor/src/assets/thumbnails/sources/fbx.rs index 8b90e1d..5b5c594 100644 --- a/crates/editor/src/assets/thumbnails/sources/fbx.rs +++ b/crates/editor/src/assets/thumbnails/sources/fbx.rs @@ -83,6 +83,72 @@ impl ThumbnailModelSource for FbxThumbnailSource { } } +pub(crate) fn spawn_fbx_mesh_subasset_preview( + commands: &mut Commands, + meshes: &mut Assets, + materials: &mut Assets, + root: Entity, + catalog_path: &str, + mesh_label: &str, +) -> bool { + let Some(encoded_index) = mesh_label + .strip_prefix("Mesh") + .and_then(|index| index.parse::().ok()) + else { + warn!("FBX thumbnail: unsupported mesh label {mesh_label}"); + return false; + }; + let node_index = encoded_index / 1000; + let material_index = encoded_index % 1000; + let Ok(bytes) = std::fs::read(catalog_path) else { + warn!("FBX thumbnail: could not read {catalog_path}"); + return false; + }; + let Ok(scene) = ufbx::load_memory( + &bytes, + ufbx::LoadOpts { + target_unit_meters: 1.0, + target_axes: ufbx::CoordinateAxes::right_handed_y_up(), + ..Default::default() + }, + ) else { + warn!("FBX thumbnail: failed to parse {catalog_path}"); + return false; + }; + let Some(node) = scene.nodes.as_ref().get(node_index) else { + warn!("FBX thumbnail: mesh label {mesh_label} has no source node"); + return false; + }; + let Some(mesh_ref) = node.mesh.as_ref() else { + warn!("FBX thumbnail: mesh label {mesh_label} has no source mesh"); + return false; + }; + let Some(indices) = group_faces_by_material(mesh_ref.as_ref()).remove(&material_index) else { + warn!("FBX thumbnail: mesh label {mesh_label} has no material group"); + return false; + }; + if indices.is_empty() { + return false; + } + + let mesh = meshes.add(build_mesh(mesh_ref.as_ref(), &indices)); + let material = materials.add(StandardMaterial { + base_color: Color::srgb(0.72, 0.72, 0.75), + perceptual_roughness: 0.65, + ..default() + }); + let transform = Transform::from_matrix(convert_matrix(&node.geometry_to_world)); + commands.entity(root).with_children(|parent| { + parent.spawn(( + RenderLayers::layer(THUMBNAIL_LAYER), + Mesh3d(mesh), + MeshMaterial3d(material), + transform, + )); + }); + true +} + fn build_mesh(ufbx_mesh: &ufbx::Mesh, indices: &[u32]) -> Mesh { let mut mesh = Mesh::new( PrimitiveTopology::TriangleList, diff --git a/crates/editor/src/assets/thumbnails/sources/mod.rs b/crates/editor/src/assets/thumbnails/sources/mod.rs index 470792f..9c75bc5 100644 --- a/crates/editor/src/assets/thumbnails/sources/mod.rs +++ b/crates/editor/src/assets/thumbnails/sources/mod.rs @@ -3,6 +3,7 @@ mod fbx; pub(crate) mod gltf; +pub(crate) use fbx::spawn_fbx_mesh_subasset_preview; pub use fbx::FbxThumbnailSource; pub use gltf::{gltf_skinned_primitive_labels, GltfThumbnailSource}; diff --git a/crates/editor/src/assets/thumbnails/studio.rs b/crates/editor/src/assets/thumbnails/studio.rs index 5c7f020..a4d1baa 100644 --- a/crates/editor/src/assets/thumbnails/studio.rs +++ b/crates/editor/src/assets/thumbnails/studio.rs @@ -15,7 +15,7 @@ use shared::{material_from_desc, standard_material_asset_path, ModelRef}; use super::cache::AssetThumbnailCache; use super::job::{ThumbnailJob, ThumbnailJobSource}; -use super::sources::{source_for_extension, uses_scene_root}; +use super::sources::{source_for_extension, spawn_fbx_mesh_subasset_preview, uses_scene_root}; use crate::assets::asset_server_path; use crate::infra::EditorOnly; @@ -27,6 +27,7 @@ const SCENE_FRAME_PADDING: f32 = 1.35; const MATERIAL_SPHERE_FRAME_PADDING: f32 = 1.28; const MESH_WARMUP_FRAMES: u8 = 12; const POST_ATTACH_FRAMES: u8 = 8; +const FIRST_RENDER_WARMUP_FRAMES: u8 = 120; const RENDER_FRAMES: u8 = 3; const COOLDOWN_FRAMES: u8 = 2; const LOAD_TIMEOUT_FRAMES: u32 = 240; @@ -51,6 +52,7 @@ pub struct ThumbnailStudio { queue: VecDeque, active: Option, cooldown_frames: u8, + render_pipeline_warmed: bool, } struct ActiveModelThumbnail { @@ -209,6 +211,7 @@ fn setup_thumbnail_studio(mut commands: Commands, mut images: ResMut spawn_mesh_subasset_preview( commands, + mesh_storage, materials, asset_server, root, - model_path, - mesh_label, - material_label.as_deref(), + MeshSubassetPreviewSpec { + model_path, + mesh_label, + material_label: material_label.as_deref(), + }, ), ThumbnailJobSource::SourceMaterial { model_path, @@ -452,18 +459,38 @@ fn spawn_thumbnail_job_content( } } +struct MeshSubassetPreviewSpec<'a> { + model_path: &'a str, + mesh_label: &'a str, + material_label: Option<&'a str>, +} + fn spawn_mesh_subasset_preview( commands: &mut Commands, + mesh_storage: &mut Assets, materials: &mut Assets, asset_server: &AssetServer, root: Entity, - model_path: &str, - mesh_label: &str, - material_label: Option<&str>, + preview: MeshSubassetPreviewSpec<'_>, ) -> bool { - let source_path = asset_server_path(model_path); - let mesh: Handle = asset_server.load(labeled_asset_path(&source_path, mesh_label)); - let material = if let Some(material_label) = material_label { + if preview + .model_path + .rsplit_once('.') + .is_some_and(|(_, extension)| extension.eq_ignore_ascii_case("fbx")) + { + return spawn_fbx_mesh_subasset_preview( + commands, + mesh_storage, + materials, + root, + preview.model_path, + preview.mesh_label, + ); + } + let source_path = asset_server_path(preview.model_path); + let mesh: Handle = + asset_server.load(labeled_asset_path(&source_path, preview.mesh_label)); + let material = if let Some(material_label) = preview.material_label { asset_server.load(standard_material_asset_path(&source_path, material_label)) } else { materials.add(StandardMaterial { @@ -509,6 +536,14 @@ fn studio_render_image() -> Image { Image::new_target_texture(THUMB_SIZE, THUMB_SIZE, TextureFormat::Rgba16Float, None) } +fn capture_frames(render_pipeline_warmed: bool) -> u8 { + if render_pipeline_warmed { + RENDER_FRAMES + } else { + FIRST_RENDER_WARMUP_FRAMES + } +} + fn finish_active_thumbnail( commands: &mut Commands, studio: &mut ThumbnailStudio, @@ -734,4 +769,10 @@ mod tests { assert!(material_distance < scene_distance * 0.6); assert!((0.75..0.82).contains(&projected_radius_fraction)); } + + #[test] + fn first_capture_keeps_camera_active_for_render_pipeline_warmup() { + assert_eq!(capture_frames(true), RENDER_FRAMES); + assert!(capture_frames(false) > capture_frames(true)); + } } diff --git a/crates/editor/src/ui/asset_browser/panel.rs b/crates/editor/src/ui/asset_browser/panel.rs index b19c4f7..2e1a673 100644 --- a/crates/editor/src/ui/asset_browser/panel.rs +++ b/crates/editor/src/ui/asset_browser/panel.rs @@ -49,7 +49,9 @@ use crate::assets::{ ThumbnailCacheSnapshot, ThumbnailStudio, }; use crate::ui::helpers::asset_label; -use crate::ui::theme::{panel_heading, ACCENT, BORDER, ELEVATED_BG, TEXT, TEXT_DIM, WIDGET_BG}; +use crate::ui::theme::{ + panel_heading, ACCENT, BORDER, ELEVATED_BG, ERROR, SUCCESS, TEXT, TEXT_DIM, WARNING, WIDGET_BG, +}; use crate::ui::widgets::{icon_button_small, tool_button}; const FOOTER_HEIGHT: f32 = 64.0; @@ -375,6 +377,10 @@ fn embedded_assets_for_asset(world: &World, asset: &EditorAsset) -> Vec Vec() @@ -1349,7 +1359,8 @@ fn draw_embedded_asset_cell( .and_then(|path| thumbnail_for_path(world, cache_snapshot, path)) }); let pending = cache_snapshot.is_pending_key(&embedded.thumbnail_key); - let failed = cache_snapshot.is_failed_key(&embedded.thumbnail_key); + let failure = cache_snapshot.failure_reason_for_key(&embedded.thumbnail_key); + let failed = failure.is_some(); let thumb_size = thumbnail_size.clamp(44.0, 64.0); let cell_size = egui::vec2(104.0, thumb_size + 50.0); let (rect, response) = ui.allocate_exact_size(cell_size, egui::Sense::click_and_drag()); @@ -1360,6 +1371,11 @@ fn draw_embedded_asset_cell( } else { response }; + let response = if let Some(reason) = failure { + response.on_hover_text(reason) + } else { + response + }; let fill = if selected { crate::ui::theme::SELECTION_BG_MUTED } else if response.hovered() { @@ -1883,9 +1899,53 @@ pub(crate) fn top_level_asset_details_panel( } if !record.dependencies.is_empty() { ui.separator(); - ui.label(panel_heading("Dependencies")); + let missing_count = record + .dependencies + .iter() + .filter(|dependency| !dependency_path(dependency).is_file()) + .count(); + ui.horizontal_wrapped(|ui| { + ui.label(panel_heading("Dependencies")); + if missing_count > 0 { + let optional = matches!( + record.import_settings.material_policy, + MaterialImportPolicy::AuthoringOverride + ); + ui.small( + egui::RichText::new(if optional { + format!("{missing_count} missing | override") + } else { + format!("{missing_count} missing") + }) + .color(if optional { + WARNING + } else { + ERROR + }), + ); + } + }); for dep in &record.dependencies { - ui.add(egui::Label::new(dep).truncate()); + let exists = dependency_path(dep).is_file(); + ui.horizontal(|ui| { + ui.label( + egui::RichText::new(if exists { + icons::CHECK_CIRCLE.as_str() + } else { + icons::WARNING_CIRCLE.as_str() + }) + .font(egui::FontId::new( + 12.0, + egui::FontFamily::Name(PHOSPHOR.into()), + )) + .color(if exists { + SUCCESS + } else { + WARNING + }), + ); + ui.add(egui::Label::new(dep).truncate()).on_hover_text(dep); + }); } } } @@ -2032,12 +2092,12 @@ fn subasset_details_panel( } } AssetSubAssetKind::Material => { - ui.small( - egui::RichText::new( - "Source materials are assigned through static mesh renderer slots.", - ) - .color(TEXT_DIM), - ); + let status = if embedded.detail.contains("Authoring Override") { + "Authoring Override is active; this source material is not assigned." + } else { + "Source material is assigned through static mesh renderer slots." + }; + ui.small(egui::RichText::new(status).color(TEXT_DIM)); } AssetSubAssetKind::Skeleton => { ui.small( @@ -2072,6 +2132,15 @@ fn subasset_details_panel( } } +fn dependency_path(reference: &str) -> PathBuf { + let path = Path::new(reference); + if path.starts_with("assets") { + path.to_path_buf() + } else { + Path::new("assets").join(path) + } +} + fn asset_action_buttons( world: &mut World, ui: &mut egui::Ui, diff --git a/crates/scene/src/project_validation.rs b/crates/scene/src/project_validation.rs index 0d05e34..8819c6b 100644 --- a/crates/scene/src/project_validation.rs +++ b/crates/scene/src/project_validation.rs @@ -410,15 +410,19 @@ fn validate_asset_registry( &format!("registry_{}", record.kind_tag.to_lowercase()), &record.path, ); - for dependency in &record.dependencies { - add_reference( - project_root, - report, - &record.path, - None, - "import_dependency", - dependency, - ); + let generated_static_manifest_owns_dependencies = record.kind_tag == "Model" + && record.import_settings.static_mesh_manifest_path.is_some(); + if !generated_static_manifest_owns_dependencies { + for dependency in &record.dependencies { + add_reference( + project_root, + report, + &record.path, + None, + "import_dependency", + dependency, + ); + } } if let Some(manifest) = record.import_settings.static_mesh_manifest_path.as_deref() { add_reference( @@ -791,6 +795,8 @@ struct StaticMeshManifestView { asset_id: String, source: StaticMeshSourceView, #[serde(default)] + import: StaticMeshImportView, + #[serde(default)] warnings: Vec, } @@ -798,11 +804,26 @@ struct StaticMeshManifestView { struct StaticMeshSourceView { path: String, #[serde(default)] + format: String, + #[serde(default)] fingerprint: AssetSourceFingerprint, #[serde(default)] dependencies: Vec, } +#[derive(Default, Deserialize)] +struct StaticMeshImportView { + #[serde(default)] + material_policy: StaticMeshMaterialPolicyView, +} + +#[derive(Default, Deserialize, PartialEq, Eq)] +enum StaticMeshMaterialPolicyView { + #[default] + SourceMaterials, + AuthoringOverride, +} + const STATIC_MESH_MANIFEST_SCHEMA_VERSION: u32 = 4; fn validate_static_mesh_manifests(project_root: &Path, report: &mut ProjectValidationReport) { @@ -855,16 +876,15 @@ fn validate_static_mesh_manifests(project_root: &Path, report: &mut ProjectValid "static_mesh.manifest_stale", "static mesh source", ); - for dependency in manifest.source.dependencies { - add_reference( - project_root, - report, - &source, - None, - "import_dependency", - &dependency, - ); - } + let optional_source_textures = manifest.source.format.eq_ignore_ascii_case("fbx") + && manifest.import.material_policy == StaticMeshMaterialPolicyView::AuthoringOverride; + add_import_dependencies( + project_root, + report, + &source, + &manifest.source.dependencies, + optional_source_textures, + ); for warning in manifest.warnings { report.findings.push(ProjectValidationFinding { severity: ValidationSeverity::Warning, @@ -881,6 +901,79 @@ fn validate_static_mesh_manifests(project_root: &Path, report: &mut ProjectValid } } +fn add_import_dependencies( + project_root: &Path, + report: &mut ProjectValidationReport, + owner_path: &str, + dependencies: &[String], + optional_source_textures: bool, +) { + let mut missing = Vec::new(); + for dependency in dependencies { + match resolve_reference(project_root, dependency) { + Some(path) if path.is_file() => add_reference( + project_root, + report, + owner_path, + None, + "import_dependency", + dependency, + ), + Some(_) => { + report.dependencies.push(ProjectDependency { + owner_path: owner_path.to_string(), + owner_actor_id: None, + kind: "import_dependency".into(), + reference: dependency.clone(), + }); + missing.push(dependency.clone()); + } + None => add_reference( + project_root, + report, + owner_path, + None, + "import_dependency", + dependency, + ), + } + } + if missing.is_empty() { + return; + } + missing.sort(); + missing.dedup(); + report.findings.push(ProjectValidationFinding { + severity: if optional_source_textures { + ValidationSeverity::Info + } else { + ValidationSeverity::Error + }, + code: "import.external_texture_missing".into(), + source_path: owner_path.to_string(), + owner_actor_id: None, + reference: Some(missing.join(", ")), + message: if optional_source_textures { + format!( + "{} FBX source texture(s) are unavailable and intentionally ignored by Authoring Override", + missing.len() + ) + } else { + format!( + "{} required imported texture dependency file(s) do not exist", + missing.len() + ) + }, + repair: if optional_source_textures { + "No action is required while Authoring Override remains active; restore the texture bundle before selecting Source Materials." + .into() + } else { + "Restore the referenced texture bundle and reimport, or select Authoring Override for a deliberately untextured asset." + .into() + }, + }); +} + fn validate_animation_manifests( project_root: &Path, report: &mut ProjectValidationReport, @@ -3192,6 +3285,118 @@ mod tests { std::fs::remove_dir_all(root).unwrap(); } + fn write_fbx_dependency_fixture( + root: &Path, + material_policy: &str, + dependencies: &[&str], + ) -> PathBuf { + let source_path = "assets/models/chair.fbx"; + std::fs::create_dir_all(root.join("assets/models")).unwrap(); + std::fs::write(root.join(source_path), b"model").unwrap(); + std::fs::write( + root.join("assets/levels/main.scn.ron"), + "(schema_version:4,resources:{},entities:{})", + ) + .unwrap(); + let artifact = root.join("assets/meshes/generated/chair.static_mesh.ron"); + std::fs::create_dir_all(artifact.parent().unwrap()).unwrap(); + let dependencies = dependencies + .iter() + .map(|path| format!("{path:?}")) + .collect::>() + .join(","); + std::fs::write( + &artifact, + format!( + "(schema_version:{STATIC_MESH_MANIFEST_SCHEMA_VERSION},asset_id:\"chair\",source:(path:\"{source_path}\",format:\"fbx\",fingerprint:(byte_len:5,content_hash:\"{}\"),dependencies:[{dependencies}]),import:(material_policy:{material_policy}),warnings:[])\n", + blake3::hash(b"model").to_hex() + ), + ) + .unwrap(); + artifact + } + + #[test] + fn source_materials_consolidate_missing_fbx_textures_as_one_blocker() { + let root = fixture_root(); + write_fbx_dependency_fixture( + &root, + "SourceMaterials", + &[ + "assets/models/textures/chair_diff.jpg", + "assets/models/textures/chair_normal.exr", + "assets/models/textures/chair_rough.exr", + ], + ); + + let report = validate_project(&root); + let findings = report + .findings + .iter() + .filter(|finding| finding.code == "import.external_texture_missing") + .collect::>(); + + assert_eq!(findings.len(), 1, "{:?}", report.findings); + assert_eq!(findings[0].severity, ValidationSeverity::Error); + assert!(findings[0].message.starts_with("3 required")); + assert_eq!( + report + .dependencies + .iter() + .filter(|dependency| dependency.kind == "import_dependency") + .count(), + 3 + ); + assert!(!report.is_release_ready()); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn authoring_override_reports_missing_fbx_textures_without_blocking_or_writing() { + let root = fixture_root(); + let artifact = write_fbx_dependency_fixture( + &root, + "AuthoringOverride", + &[ + "assets/models/textures/chair_diff.jpg", + "assets/models/textures/chair_normal.exr", + "assets/models/textures/chair_rough.exr", + ], + ); + let before = std::fs::read(&artifact).unwrap(); + + let report = validate_project(&root); + + let finding = report + .findings + .iter() + .find(|finding| finding.code == "import.external_texture_missing") + .expect("Authoring Override should retain one actionable dependency state"); + assert_eq!(finding.severity, ValidationSeverity::Info); + assert!(finding.message.contains("Authoring Override")); + assert!(report.is_release_ready(), "{:?}", report.findings); + assert_eq!(std::fs::read(&artifact).unwrap(), before); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn authoring_override_does_not_downgrade_unsafe_fbx_texture_paths() { + let root = fixture_root(); + write_fbx_dependency_fixture( + &root, + "AuthoringOverride", + &["assets/models/../outside/chair.jpg"], + ); + + let report = validate_project(&root); + + assert!(report.findings.iter().any(|finding| { + finding.code == "asset.unsafe_path" && finding.severity == ValidationSeverity::Error + })); + assert!(!report.is_release_ready()); + std::fs::remove_dir_all(root).unwrap(); + } + #[test] fn same_size_static_mesh_source_change_invalidates_content_hash() { let root = fixture_root(); diff --git a/docs/README.md b/docs/README.md index 0ca45d3..3e15c25 100644 --- a/docs/README.md +++ b/docs/README.md @@ -58,6 +58,7 @@ Immutable-style log of significant decisions. Add a new numbered ADR when changi | [0041](adr/0041-transactional-editor-physics-placement.md) | Paused editor physics ownership and transactional gravity placement | | [0042](adr/0042-guarded-editor-shutdown-and-document-savepoints.md) | Guarded native editor exit and canonical per-document clean checkpoints | | [0043](adr/0043-content-addressed-import-fingerprints.md) | Content-addressed imported-source identity and byte-preserving artifact publication | +| [0044](adr/0044-sandboxed-fbx-external-texture-dependencies.md) | Sandboxed FBX texture bundles, validation policy, and deduplicated loading | ## Editor framework diff --git a/docs/adr/0044-sandboxed-fbx-external-texture-dependencies.md b/docs/adr/0044-sandboxed-fbx-external-texture-dependencies.md new file mode 100644 index 0000000..76fab6f --- /dev/null +++ b/docs/adr/0044-sandboxed-fbx-external-texture-dependencies.md @@ -0,0 +1,68 @@ +# ADR 0044: Sandboxed FBX External Texture Dependencies + +## Status + +Accepted + +## Context + +FBX texture references may use sibling folders such as `textures/`, exporter-created `.fbm/` +folders, absolute workstation paths, Windows separators, or repeated ufbx texture elements that +resolve to the same file. Blacksite previously discovered only files already present in a sibling +`{model}.fbm/` directory. The committed painted-chair FBX instead declares three files under +`textures/`; none were committed, so source-material thumbnails queued the same missing Bevy image +paths more than once. + +Import, manifest generation, project validation, editor previews, and the runtime loader must agree +on one dependency graph. That graph also crosses a security boundary: an imported source must not +read or copy a parent-traversing path or an arbitrary absolute path outside its bundle. A missing +source texture is required when Source Materials is active, but it can be an intentional authoring +condition when an asset explicitly uses Authoring Override. + +## Decision + +The local `bevy_ufbx` compatibility crate owns FBX external-texture path discovery and +normalization. It converts separators to `/`, preserves safe sibling and `.fbm/` layouts, +deduplicates normalized paths, and rejects parent traversal, asset-path syntax, and absolute paths. +An absolute exporter path may be portably rebased only when it contains a `.fbm/` suffix; only that +sidecar-relative suffix is retained. + +The FBX loader performs a discovery parse, drops the non-`Send` ufbx scene, reads every unique safe +dependency through Bevy's `LoadContext`, and then performs normal synchronous scene processing. +External images become FBX labeled assets from those bytes. Repeated texture elements share one +image handle. Missing, rejected, or undecodable references are skipped with one consolidated +loader warning instead of deferred asset-server requests. + +Static-mesh manifests are the authoritative model import dependency graph. Their existing +`source.dependencies` list records every parsed FBX external texture, including unavailable files. +Project validation checks that list without loading native assets. Missing FBX textures are a +blocking consolidated finding under Source Materials and an informational consolidated finding +under Authoring Override. Unsafe paths remain blocking under either policy. Model registry records +mirror the list for editor details, but validation does not duplicate it when a generated static +manifest exists. + +File -> Import Assets treats an FBX plus its referenced textures as one bundle. The importer +preflights every source path and canonical containment before touching the project, rejects +destination paths with symlinked ancestors, stages only the referenced files, preserves their +normalized relative layout, backs up overwritten destinations, and rolls back a partial publish. +Unreferenced `.fbm` contents are not copied. + +The Asset Browser renders FBX model and mesh thumbnails through the neutral direct mesh path. +Source-material thumbnails preflight the authoritative dependency resolver first and cache one +non-retryable actionable failure when required textures are unavailable. Dependency rows expose +present/missing state. The committed painted-chair fixture uses Authoring Override deliberately and +remains untextured; its three source paths stay visible in manifests, validation, and asset details. + +## Consequences + +- FBX sibling `textures/` and `.fbm/` bundles import and validate with identical normalized paths. +- Missing texture files are visible before native loading and cannot create repeated Bevy + asset-server errors. +- Source Materials fails release validation when its FBX textures are absent. Authoring Override + remains release-valid but retains an informational dependency finding. +- Absolute exporter paths, traversal attempts, and destination symlink redirects cannot read or + copy arbitrary host files. +- FBX files with external textures are parsed twice during loading so no non-`Send` ufbx scene + crosses an async read. Files without external textures retain a single parse. +- External images are labeled children of the FBX and reload the owning FBX through loader + dependency tracking. Per-image import metadata is not yet exposed. diff --git a/docs/editor/README.md b/docs/editor/README.md index fb908b7..6c2983b 100644 --- a/docs/editor/README.md +++ b/docs/editor/README.md @@ -65,7 +65,7 @@ Docs for the in-process egui editor (`crates/editor/`). Update this index when a | `assets/materials.rs` / `ui/material_library.rs` / `viewport/material_drop.rs` / `shared::renderer_material` | Shared material discovery, docked catalog/usage UI, exact reversible surface drops, stable renderer slots, and draw binding | material-system.md, ADR 0035 | | `blacksite_surface` / `game_hot::rendering::solari` / `third_party/bevy_solari` | Surface ABI packing, raster composition, Solari evaluator dispatch, and deformation eligibility | material-system.md, rendering.md, ADR 0036 | | `assets/fingerprint.rs` / `shared::AssetSourceFingerprint` | Content-addressed imported-source identity and byte-preserving registry/manifest publication | ADR 0043, evaluations/deterministic-asset-fingerprints/ | -| `assets/` | Catalog, asset DB, static mesh artifacts, `thumbnails/`, `materials.rs`, prefab overrides v2 | this file (below), prefab-authoring.md, ADR 0017, ADR 0027 | +| `assets/` | Catalog, transactional import bundles, asset DB, static mesh artifacts, `thumbnails/`, `materials.rs`, prefab overrides v2 | this file (below), prefab-authoring.md, ADR 0017, ADR 0027, ADR 0044 | | `shared::prefab_overrides` | Versioned stable override schema and editor-independent runtime application | prefab-authoring.md, ADR 0027 | | `project/` | Workspace, settings UI, user prefs, support diagnostics | roadmap Phase 1 | | `project/collaboration.rs` | Guarded authored writes, asynchronous Git status, and optional ownership providers | collaborative-file-safety.md, ADR 0037 | @@ -92,7 +92,7 @@ Docs for the in-process egui editor (`crates/editor/`). Update this index when a - **Dedicated egui `Camera2d`** at full window — never attach `PrimaryEguiContext` to a viewport-cropped 3D camera (NaN layout panic). - **Unified viewport** uses one render-to-texture target for both the editor fly camera and the possessed player camera so HDR/atmosphere is not broken by sub-viewport cropping. - **PIE:** F8 possess/eject while sim runs; **F6** pauses/resumes simulation in Play; project settings drive shared rendering for the active viewport camera. -- **Asset browser** mirrors the on-disk `assets/` directory with project tree, breadcrumb, search/filter/sort, grid/list views, texture/model/material thumbnails, a details pane, and context-aware row/menu actions; narrow docks prioritize content, keep the root panel fixed, switch list view to a compact single-column layout, and hide tree/details panes when they would crowd the content area. The footer stays pinned while only the project tree, asset content, and details panels scroll. **Built-ins** holds spawnable primitives and lights. **Materials** folder scans `assets/materials/*.ron`, renders material thumbnails on a sphere using `MaterialDesc`, and exposes shader-schema-driven parameters/textures in the details editor; **Shaders** holds shader schema RON files. **Audio** accepts Bevy-supported Ogg/Vorbis and Speex (`.ogg`, `.oga`, `.spx`), WAV, MP3, and FLAC clips under `assets/audio/`, with a dedicated filter, waveform icon, format/file details, and stable registry-backed references that retain a runtime source path. glTF/GLB/FBX rows can expand into a shelf of normalized embedded mesh, material, and texture subassets with independent generated thumbnails. Mesh subassets can be selected, dragged into the viewport, or placed from details/context menus; material subassets render source-material spheres; texture subassets can be applied to the selected actor. Model import settings are staged with **Apply** / **Revert**, asset context menus can regenerate thumbnails, material asset details edit shared `MaterialAsset` fields, and file asset deletion moves sources/generated artifacts into `assets/.trash/`. Before thumbnail loading, glTF sources preflight local external buffers/images and show a stable non-retryable failure state when a dependency is missing instead of repeatedly invoking the asset loader. +- **Asset browser** mirrors the on-disk `assets/` directory with project tree, breadcrumb, search/filter/sort, grid/list views, texture/model/material thumbnails, a details pane, and context-aware row/menu actions; narrow docks prioritize content, keep the root panel fixed, switch list view to a compact single-column layout, and hide tree/details panes when they would crowd the content area. The footer stays pinned while only the project tree, asset content, and details panels scroll. **Built-ins** holds spawnable primitives and lights. **Materials** folder scans `assets/materials/*.ron`, renders material thumbnails on a sphere using `MaterialDesc`, and exposes shader-schema-driven parameters/textures in the details editor; **Shaders** holds shader schema RON files. **Audio** accepts Bevy-supported Ogg/Vorbis and Speex (`.ogg`, `.oga`, `.spx`), WAV, MP3, and FLAC clips under `assets/audio/`, with a dedicated filter, waveform icon, format/file details, and stable registry-backed references that retain a runtime source path. glTF/GLB/FBX rows can expand into a shelf of normalized embedded mesh, material, and texture subassets with independent generated thumbnails. Mesh subassets can be selected, dragged into the viewport, or placed from details/context menus; material subassets render source-material spheres; texture subassets can be applied to the selected actor. Model import settings are staged with **Apply** / **Revert**, asset context menus can regenerate thumbnails, material asset details edit shared `MaterialAsset` fields, and file asset deletion moves sources/generated artifacts into `assets/.trash/`. Before thumbnail loading, glTF sources preflight local external buffers/images. FBX source-material previews use the same sandboxed resolver as import and validation, expose one cached hover-visible dependency failure, and never enqueue known-missing texture paths; neutral model/mesh previews remain available. Asset details mark present/missing dependencies and identify missing textures intentionally ignored by Authoring Override. FBX bundle import preserves referenced sibling or `.fbm/` layout transactionally. See [ADR 0044](../adr/0044-sandboxed-fbx-external-texture-dependencies.md). - **Imported-source fingerprints** use exact byte length plus lowercase BLAKE3 for model, texture, and audio registry records and for generated static-mesh/animation manifests. Filesystem timestamps are scan hints only; equivalent refresh preserves the exact committed RON bytes and registry publication order is normalized by project path. See [ADR 0043](../adr/0043-content-addressed-import-fingerprints.md) and the [acceptance record](evaluations/deterministic-asset-fingerprints/). - **Material Library** is a dockable bottom-panel catalog for cross-folder Material and direct-base Material Instance authoring. It provides search, type and scene-usage filters, grid/list thumbnails, dependency health, usage counts, creation, guarded details editing, and first-class drag sources. Viewport drops resolve an exact renderer slot, primitive, or brush face under the pointer, preview transiently, reject incompatible/read-only targets explicitly, restore on target change/cancel, and commit one typed undo step on release. - **Static/skinned renderer split** — model drag/drop uses normalized artifacts under `assets/meshes/generated/`. Unrigged, non-animated sources create `ActorKind::StaticMesh + StaticMeshRenderer`; skin-bound or animated sources create `ActorKind::SkinnedMesh + SkinnedMeshRenderer` and preserve the imported hierarchy. Static slots never contain marked skinned primitives or geometry from animated sources. `SceneInstance` placement keeps `ImportedModel + ModelRef` for generic full-source scenes. See [ADR 0033](../adr/0033-dedicated-skinned-mesh-renderer.md). diff --git a/docs/editor/architecture.md b/docs/editor/architecture.md index 719d51f..51f7b0e 100644 --- a/docs/editor/architecture.md +++ b/docs/editor/architecture.md @@ -220,7 +220,11 @@ the Edit-to-Play boundary restore the complete runtime snapshot. See ## Model import (glTF + FBX) -- **Import:** File → Import Assets copies glTF/GLB/FBX into `assets/models/`. FBX imports also copy a sibling `{name}.fbm/` folder when present (embedded textures). +- **Import:** File -> Import Assets copies glTF/GLB/FBX into `assets/models/`. An FBX import + parses every safe external texture reference, including sibling `textures/` and `.fbm/` layouts, + then stages and publishes the complete referenced bundle transactionally. Absolute non-sidecar + paths, parent traversal, and dependencies that canonically escape the source folder are rejected + before project files change; unreferenced sidecar contents are not copied. - **Processing:** the asset registry generates normalized model manifests under `assets/meshes/generated/`. The manifests store stable part IDs, glTF/FBX mesh/material subasset labels, whether each part is skin-bound, source metadata, dependencies, and import settings. @@ -229,13 +233,22 @@ the Edit-to-Play boundary restore the complete runtime snapshot. See equivalent refresh, so checkout mtimes and formatting do not dirty project content. See [ADR 0043](../adr/0043-content-addressed-import-fingerprints.md). These artifacts are hidden from the Asset Browser catalog. +- **FBX dependencies:** the local `bevy_ufbx` resolver owns separator normalization, safe `.fbm/` + rebasing, deduplication, and sandbox rejection. Static-mesh manifests record every declared + external texture even when it is absent. Project validation reports one blocking finding for + missing Source Materials textures, or one informational finding when Authoring Override + deliberately leaves the model untextured. The runtime loader reads unique dependencies through + `LoadContext` and creates labeled images directly, so missing paths cannot fan out into repeated + asset-server errors. See [ADR 0044](../adr/0044-sandboxed-fbx-external-texture-dependencies.md). - **Browser subassets:** model rows can expand into a content shelf backed by the generated manifest. Unrigged mesh subassets place as independent `StaticMeshRenderer` actors. A skinned subasset places its owning source through `SkinnedMeshRenderer`, retaining joints and inverse bind poses. Material subassets expose source defaults, and texture dependencies can be applied to selected actors. The thumbnail studio renders each generated model/subasset/material preview into its own render target before registering it in the cache so later thumbnails cannot overwrite - earlier cache entries. + earlier cache entries. FBX model and mesh previews use neutral direct geometry; source-material + previews preflight external textures and retain one stable hover-visible failure reason instead + of enqueueing a known-missing path. - **Default placement:** **Renderable Asset (Auto)** creates `ActorKind::StaticMesh + StaticMeshRenderer` for unrigged, non-animated models and `ActorKind::SkinnedMesh + SkinnedMeshRenderer` when any part is skin-bound or the source contains diff --git a/docs/editor/roadmap.md b/docs/editor/roadmap.md index 5070166..fb0668b 100644 --- a/docs/editor/roadmap.md +++ b/docs/editor/roadmap.md @@ -120,8 +120,8 @@ Phased work for the in-process editor (`crates/editor`). Status reflects the cur | Item | Status | Notes | |------|--------|-------| | `asset_db.rs` registry | Done | Stable UUIDs by path/content, deterministic model/texture/audio BLAKE3 fingerprints, normalized publication order, and import settings in details | -| Model import formats | Done | glTF/GLB + FBX (binary); `.fbm` sidecar copy on import | -| Thumbnails | Done | Textures via asset load; glTF albedo fast-path; FBX/untextured models via offscreen studio | +| Model import formats | Done | glTF/GLB + binary FBX; sandboxed `textures/`/`.fbm/` discovery and transactional referenced-bundle copy | +| Thumbnails | Done | Textures via asset load; glTF albedo fast-path; direct neutral FBX geometry; preflighted source-material dependency state | | Import settings per asset | Done | Scale/collider/LOD in registry + details pane | | Prefab workflow | Done | Shared stable nested property/component/structural overrides, recursive validation, committed variant fixtures, transactional source Apply, conflict recovery, and undoable outer unpack/recursive local conversion passed headless, packaged-runtime, and live editor acceptance in Gitea #43 | | Curated editor regression samples | Done | Versioned five-area manifest, dedicated brush/material labs, strengthened terrain/physics/rendering fixtures, **File > Open Sample**, typed scene coverage, and `validate-samples` release gate in Gitea #32 | diff --git a/third_party/bevy_ufbx/src/lib.rs b/third_party/bevy_ufbx/src/lib.rs index 617c8ec..0ea93bb 100644 --- a/third_party/bevy_ufbx/src/lib.rs +++ b/third_party/bevy_ufbx/src/lib.rs @@ -25,6 +25,7 @@ pub mod material; pub mod mesh; pub mod node; pub mod scene; +pub mod texture; pub mod types; pub mod utils; diff --git a/third_party/bevy_ufbx/src/loader.rs b/third_party/bevy_ufbx/src/loader.rs index 803a36a..e230f96 100644 --- a/third_party/bevy_ufbx/src/loader.rs +++ b/third_party/bevy_ufbx/src/loader.rs @@ -5,11 +5,12 @@ use crate::material::process_materials; use crate::mesh::process_meshes; use crate::node::{process_nodes, process_skins}; use crate::scene::build_scene; +use crate::texture::external_texture_path_report; use crate::types::{Fbx, FbxAxisSystem, FbxMeta, Handedness}; use bevy::asset::{io::Reader, AssetLoader, LoadContext, RenderAssetUsages}; use bevy::prelude::*; use serde::{Deserialize, Serialize}; -use std::collections::HashMap; +use std::collections::{BTreeSet, HashMap}; /// Settings for FBX file loading. /// @@ -74,77 +75,128 @@ impl AssetLoader for FbxLoader { return Err(FbxError::InvalidData("FBX file too small".to_string())); } - // Parse with ufbx - // Provide the asset path as a filename hint so ufbx can compute relative_filename let filename_hint = load_context.path().path().to_string_lossy().to_string(); - let root = ufbx::load_memory( - &bytes, - ufbx::LoadOpts { - target_unit_meters: 1.0, - target_axes: ufbx::CoordinateAxes::right_handed_y_up(), - filename: ufbx::StringOpt::Ref(&filename_hint), - ..Default::default() - }, - ) - .map_err(|e| FbxError::UfbxError(format!("{:?}", e)))?; - let scene: &ufbx::Scene = &*root; + let root = parse_fbx(&bytes, &filename_hint)?; + let texture_report = external_texture_path_report(&root); + if texture_report.paths.is_empty() { + if !texture_report.errors.is_empty() { + warn_texture_failures( + load_context.path(), + texture_report.errors.iter().map(ToString::to_string), + ); + } + return process_scene(&root, settings, load_context, &HashMap::new()); + } - // Process meshes - let (meshes, named_meshes, mesh_transforms, mesh_material_info) = - process_meshes(scene, settings, load_context)?; + // `ufbx::Scene` is not Send, so finish discovery and drop it before awaiting asset I/O. + drop(root); + let mut failures = texture_report + .errors + .iter() + .map(ToString::to_string) + .collect::>(); + let mut external_texture_bytes = HashMap::new(); + let asset_path = load_context.path().clone(); + for relative_path in texture_report.paths { + let resolved_path = match asset_path.resolve_embed_str(&relative_path) { + Ok(path) => path, + Err(error) => { + failures.insert(format!("{relative_path} ({error})")); + continue; + } + }; + match load_context.read_asset_bytes(resolved_path.clone()).await { + Ok(bytes) => { + external_texture_bytes.insert(relative_path, bytes); + } + Err(error) => { + failures.insert(format!("{resolved_path} ({error})")); + } + } + } + if !failures.is_empty() { + warn_texture_failures(load_context.path(), failures); + } - // Process materials and textures - let (materials, named_materials) = if !settings.load_materials.is_empty() { - process_materials(scene, settings, load_context)? - } else { - (Vec::new(), HashMap::new()) - }; - - // Process nodes and hierarchy - let (nodes, named_nodes, node_map) = process_nodes(scene, &meshes, load_context)?; - - // Process skins - let (skins, named_skins) = process_skins(scene, &node_map, load_context)?; - - // Build scene - let scene_handle = build_scene( - scene, - &meshes, - &materials, - &named_materials, - &mesh_transforms, - &mesh_material_info, - settings, - load_context, - )?; - - // Extract metadata - let metadata = FbxMeta::default(); - - // Build final FBX asset - Ok(Fbx { - scenes: vec![scene_handle.clone()], - named_scenes: HashMap::new(), - meshes, - named_meshes, - materials, - named_materials, - nodes, - named_nodes, - skins, - named_skins, - default_scene: Some(scene_handle), - axis_system: FbxAxisSystem { - up: Vec3::Y, - front: Vec3::Z, - handedness: Handedness::Right, - }, - unit_scale: 1.0, - metadata, - }) + let root = parse_fbx(&bytes, &filename_hint)?; + process_scene(&root, settings, load_context, &external_texture_bytes) } fn extensions(&self) -> &[&str] { &["fbx"] } } + +fn parse_fbx(bytes: &[u8], filename_hint: &str) -> Result { + ufbx::load_memory( + bytes, + ufbx::LoadOpts { + target_unit_meters: 1.0, + target_axes: ufbx::CoordinateAxes::right_handed_y_up(), + filename: ufbx::StringOpt::Ref(filename_hint), + ..Default::default() + }, + ) + .map_err(|error| FbxError::UfbxError(format!("{error:?}"))) +} + +fn process_scene( + scene: &ufbx::Scene, + settings: &FbxLoaderSettings, + load_context: &mut LoadContext<'_>, + external_texture_bytes: &HashMap>, +) -> Result { + let (meshes, named_meshes, mesh_transforms, mesh_material_info) = + process_meshes(scene, settings, load_context)?; + let (materials, named_materials) = if !settings.load_materials.is_empty() { + process_materials(scene, settings, load_context, external_texture_bytes)? + } else { + (Vec::new(), HashMap::new()) + }; + let (nodes, named_nodes, node_map) = process_nodes(scene, &meshes, load_context)?; + let (skins, named_skins) = process_skins(scene, &node_map, load_context)?; + let scene_handle = build_scene( + scene, + &meshes, + &materials, + &named_materials, + &mesh_transforms, + &mesh_material_info, + settings, + load_context, + )?; + + Ok(Fbx { + scenes: vec![scene_handle.clone()], + named_scenes: HashMap::new(), + meshes, + named_meshes, + materials, + named_materials, + nodes, + named_nodes, + skins, + named_skins, + default_scene: Some(scene_handle), + axis_system: FbxAxisSystem { + up: Vec3::Y, + front: Vec3::Z, + handedness: Handedness::Right, + }, + unit_scale: 1.0, + metadata: FbxMeta::default(), + }) +} + +fn warn_texture_failures( + asset_path: &bevy::asset::AssetPath<'_>, + failures: impl IntoIterator, +) { + let failures = failures.into_iter().collect::>(); + warn!( + "bevy_ufbx: skipped {} unavailable external texture reference(s) for {}: {}", + failures.len(), + asset_path, + failures.join("; ") + ); +} diff --git a/third_party/bevy_ufbx/src/material.rs b/third_party/bevy_ufbx/src/material.rs index d8342d7..5b64cd1 100644 --- a/third_party/bevy_ufbx/src/material.rs +++ b/third_party/bevy_ufbx/src/material.rs @@ -3,19 +3,21 @@ use crate::error::FbxError; use crate::label::FbxAssetLabel; use crate::loader::FbxLoaderSettings; +use crate::texture::external_texture_reference; use crate::utils::convert_texture_uv_transform; use bevy::asset::{Handle, LoadContext}; use bevy::pbr::StandardMaterial; use bevy::prelude::*; use bevy_material::AlphaMode; use bevy_image::{CompressedImageFormats, ImageSampler, ImageType}; -use std::collections::HashMap; +use std::collections::{BTreeSet, HashMap}; /// Process all materials from the FBX scene. pub fn process_materials( scene: &ufbx::Scene, settings: &FbxLoaderSettings, - load_context: &mut LoadContext, + load_context: &mut LoadContext<'_>, + external_texture_bytes: &HashMap>, ) -> Result< ( Vec>, @@ -25,7 +27,12 @@ pub fn process_materials( > { let mut materials = Vec::new(); let mut named_materials = HashMap::new(); - let texture_handles = process_textures(scene, settings, load_context)?; + let texture_handles = process_textures( + scene, + settings, + load_context, + external_texture_bytes, + )?; for (index, ufbx_material) in scene.materials.as_ref().iter().enumerate() { if ufbx_material.element.element_id == 0 { @@ -60,18 +67,15 @@ pub fn process_materials( pub fn process_textures( scene: &ufbx::Scene, settings: &FbxLoaderSettings, - load_context: &mut LoadContext, + load_context: &mut LoadContext<'_>, + external_texture_bytes: &HashMap>, ) -> Result>, FbxError> { let mut texture_handles = HashMap::new(); + let mut external_handles = HashMap::>::new(); + let mut unavailable = BTreeSet::new(); + let asset_path = load_context.path().clone(); for (index, texture) in scene.textures.as_ref().iter().enumerate() { - let asset_path = load_context.path().clone(); - let fbx_dir = asset_path - .path() - .parent() - .unwrap_or_else(|| std::path::Path::new("")) - .to_path_buf(); - // Priority 1: Embedded texture data if !texture.content.is_empty() { let ext = extract_extension(&texture.filename).unwrap_or("png"); @@ -91,39 +95,62 @@ pub fn process_textures( texture_handles.insert(texture.element.element_id, handle); continue; } - Err(e) => { - eprintln!("bevy_ufbx: failed to load embedded texture {index}: {e}"); - // Fall through to try external paths + Err(error) => { + warn!( + "bevy_ufbx: skipped embedded texture {index} for {asset_path}: {error}" + ); + continue; } } } - // Priority 2: relative_filename from ufbx (relative to FBX file) - if !texture.relative_filename.is_empty() { - let rel = texture.relative_filename.as_ref(); - let is_absolute = rel.starts_with('/') - || (rel.len() >= 3 && rel.chars().nth(1) == Some(':')); - if !is_absolute { - let texture_path = fbx_dir.join(rel).to_string_lossy().to_string(); - let image_handle = load_context.load(texture_path); - texture_handles.insert(texture.element.element_id, image_handle); + let reference = match external_texture_reference(index, texture) { + Ok(Some(reference)) => reference, + Ok(None) => continue, + Err(error) => { + unavailable.insert(error.to_string()); continue; } + }; + if let Some(handle) = external_handles.get(&reference.relative_path) { + texture_handles.insert(reference.element_id, handle.clone()); + continue; } - // Priority 3: Extract relative path from filename / absolute_filename - // Preserves .fbm folder structure if present (e.g., "model.fbm/texture.jpg") - let relative_path = extract_relative_texture_path(texture); + let Some(bytes) = external_texture_bytes.get(&reference.relative_path) else { + continue; + }; + let extension = std::path::Path::new(&reference.relative_path) + .extension() + .and_then(|extension| extension.to_str()) + .unwrap_or("png"); + let image = match Image::from_buffer( + &bytes, + ImageType::Extension(extension), + CompressedImageFormats::NONE, + true, + ImageSampler::Default, + settings.load_materials, + ) { + Ok(image) => image, + Err(error) => { + unavailable.insert(format!("{} ({error})", reference.relative_path)); + continue; + } + }; + let handle = load_context + .add_labeled_asset(FbxAssetLabel::Texture(index).to_string(), image); + external_handles.insert(reference.relative_path, handle.clone()); + texture_handles.insert(reference.element_id, handle); + } - if !relative_path.is_empty() { - let texture_path = fbx_dir - .join(relative_path) - .to_string_lossy() - .to_string(); - - let image_handle = load_context.load(texture_path); - texture_handles.insert(texture.element.element_id, image_handle); - } + if !unavailable.is_empty() { + warn!( + "bevy_ufbx: skipped {} unavailable external texture reference(s) for {}: {}", + unavailable.len(), + asset_path, + unavailable.into_iter().collect::>().join("; ") + ); } Ok(texture_handles) @@ -140,48 +167,6 @@ fn extract_extension(filename: &ufbx::String) -> Option<&str> { .filter(|ext| !ext.contains('/') && !ext.contains('\\')) } -/// Extract a relative texture path from a ufbx Texture's filename or absolute_filename. -/// -/// Handles absolute paths by looking for `.fbm` folders (FBX's standard embedded texture -/// directory) and extracting from there, or falling back to just the filename. -fn extract_relative_texture_path<'a>(texture: &'a ufbx::Texture) -> &'a str { - if !texture.filename.is_empty() { - let filename = texture.filename.as_ref(); - - let is_absolute = filename.starts_with('/') - || (filename.len() >= 3 && filename.chars().nth(1) == Some(':')); - - if is_absolute { - extract_from_absolute(filename) - } else { - filename - } - } else if !texture.absolute_filename.is_empty() { - extract_from_absolute(texture.absolute_filename.as_ref()) - } else { - "" - } -} - -/// Extract a relative path from an absolute path, preserving `.fbm` folder structure. -fn extract_from_absolute(path: &str) -> &str { - if let Some(fbm_pos) = path.rfind(".fbm/").or_else(|| path.rfind(".fbm\\")) { - let before_fbm = &path[..fbm_pos]; - let folder_start = before_fbm - .rfind(&['/', '\\'][..]) - .map(|p| p + 1) - .unwrap_or(0); - &path[folder_start..] - } else { - let last_slash = path.rfind(&['/', '\\'][..]); - if let Some(pos) = last_slash { - &path[pos + 1..] - } else { - path - } - } -} - /// Create a StandardMaterial from ufbx material. pub fn create_standard_material( ufbx_material: &ufbx::Material, diff --git a/third_party/bevy_ufbx/src/texture.rs b/third_party/bevy_ufbx/src/texture.rs new file mode 100644 index 0000000..c175e0f --- /dev/null +++ b/third_party/bevy_ufbx/src/texture.rs @@ -0,0 +1,211 @@ +//! External FBX texture reference discovery and path normalization. + +use std::collections::BTreeSet; +use std::fmt; +use std::path::{Component, Path}; + +/// One safe external texture reference declared by an FBX texture element. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FbxExternalTextureReference { + pub texture_index: usize, + pub element_id: u32, + pub relative_path: String, +} + +/// A texture reference that cannot be resolved inside the FBX asset source. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FbxTexturePathError { + pub texture_index: usize, + pub raw_path: String, + pub reason: String, +} + +/// Deterministic external texture discovery result, including safe and rejected references. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct FbxExternalTexturePathReport { + pub paths: Vec, + pub errors: Vec, +} + +impl fmt::Display for FbxTexturePathError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + formatter, + "texture {} path `{}` is unsafe: {}", + self.texture_index, self.raw_path, self.reason + ) + } +} + +/// Returns the normalized external path for one texture, or `None` for embedded/no-file textures. +pub fn external_texture_reference( + texture_index: usize, + texture: &ufbx::Texture, +) -> Result, FbxTexturePathError> { + if !texture.content.is_empty() { + return Ok(None); + } + + let Some(raw_path) = texture_path_candidate(texture) else { + return Ok(None); + }; + let relative_path = normalize_external_texture_path(raw_path).map_err(|reason| { + FbxTexturePathError { + texture_index, + raw_path: raw_path.to_string(), + reason, + } + })?; + Ok(Some(FbxExternalTextureReference { + texture_index, + element_id: texture.element.element_id, + relative_path, + })) +} + +/// Returns every distinct safe external texture path in deterministic order. +pub fn external_texture_paths( + scene: &ufbx::Scene, +) -> Result, Vec> { + let report = external_texture_path_report(scene); + if report.errors.is_empty() { + Ok(report.paths) + } else { + Err(report.errors) + } +} + +/// Discovers safe and rejected references without discarding either side of the result. +pub fn external_texture_path_report(scene: &ufbx::Scene) -> FbxExternalTexturePathReport { + let mut paths = BTreeSet::new(); + let mut errors = Vec::new(); + for (texture_index, texture) in scene.textures.as_ref().iter().enumerate() { + match external_texture_reference(texture_index, texture) { + Ok(Some(reference)) => { + paths.insert(reference.relative_path); + } + Ok(None) => {} + Err(error) => errors.push(error), + } + } + errors.sort_by(|left, right| { + (left.texture_index, &left.raw_path).cmp(&(right.texture_index, &right.raw_path)) + }); + errors.dedup(); + FbxExternalTexturePathReport { + paths: paths.into_iter().collect(), + errors, + } +} + +fn texture_path_candidate(texture: &ufbx::Texture) -> Option<&str> { + if !texture.relative_filename.is_empty() { + return Some(texture.relative_filename.as_ref()); + } + if !texture.filename.is_empty() { + return Some(texture.filename.as_ref()); + } + if !texture.absolute_filename.is_empty() { + return Some(texture.absolute_filename.as_ref()); + } + None +} + +fn normalize_external_texture_path(raw_path: &str) -> Result { + let normalized = raw_path.trim().replace('\\', "/"); + if normalized.is_empty() { + return Err("path is empty".into()); + } + if normalized.contains('\0') { + return Err("path contains a NUL byte".into()); + } + if normalized.contains('#') || normalized.contains('?') || normalized.contains("://") { + return Err("path contains asset-source, label, or query syntax".into()); + } + + let normalized = if is_absolute_like(&normalized) { + extract_fbm_suffix(&normalized).ok_or_else(|| { + "absolute paths are rejected unless they identify a portable .fbm sidecar".to_string() + })? + } else { + normalized + }; + + let mut components = Vec::new(); + for component in Path::new(&normalized).components() { + match component { + Component::Normal(segment) => { + let segment = segment.to_str().ok_or_else(|| "path is not UTF-8".to_string())?; + if segment.contains(':') { + return Err("path contains a platform or asset-source prefix".into()); + } + components.push(segment); + } + Component::CurDir => {} + Component::ParentDir => return Err("parent traversal is not allowed".into()), + Component::RootDir | Component::Prefix(_) => { + return Err("absolute paths are not allowed".into()); + } + } + } + if components.is_empty() { + return Err("path does not identify a file".into()); + } + Ok(components.join("/")) +} + +fn is_absolute_like(path: &str) -> bool { + path.starts_with('/') + || path.starts_with("//") + || path.as_bytes().get(1).is_some_and(|byte| *byte == b':') +} + +fn extract_fbm_suffix(path: &str) -> Option { + let lower = path.to_ascii_lowercase(); + let fbm_end = lower.rfind(".fbm/")? + ".fbm/".len(); + let fbm_start = path[..fbm_end - 1] + .rfind('/') + .map(|index| index + 1) + .unwrap_or(0); + Some(path[fbm_start..].to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn sibling_texture_folder_is_preserved() { + assert_eq!( + normalize_external_texture_path("textures\\chair_diff.jpg").unwrap(), + "textures/chair_diff.jpg" + ); + } + + #[test] + fn absolute_fbm_path_is_portably_rebased() { + assert_eq!( + normalize_external_texture_path("C:\\Build\\Chair.fbm\\chair_diff.jpg").unwrap(), + "Chair.fbm/chair_diff.jpg" + ); + } + + #[test] + fn absolute_non_sidecar_path_is_rejected() { + let error = normalize_external_texture_path("C:\\Build\\textures\\chair_diff.jpg") + .unwrap_err(); + assert!(error.contains("absolute paths are rejected")); + } + + #[test] + fn parent_traversal_is_rejected() { + let error = normalize_external_texture_path("../shared/chair_diff.jpg").unwrap_err(); + assert!(error.contains("parent traversal")); + } + + #[test] + fn asset_path_syntax_is_rejected() { + assert!(normalize_external_texture_path("remote://chair_diff.jpg").is_err()); + assert!(normalize_external_texture_path("textures/chair.jpg#Image0").is_err()); + } +}