Bevy Rewrite, attempt 2 #46

Merged
Schmarni-Dev merged 18 commits from bevy_for_real_this_time into dev 2025-06-30 05:04:00 -04:00
8 changed files with 5250 additions and 936 deletions
Showing only changes of commit a7e8f84841 - Show all commits

5132
Cargo.lock generated

File diff suppressed because it is too large Load Diff

View File

@@ -21,37 +21,29 @@ name = "stardust-xr-server"
path = "src/main.rs"
[features]
default = ["wayland"]
default = []
wayland = ["dep:smithay", "dep:wayland-scanner", "dep:wayland-backend"]
coderabbitai[bot] commented 2025-06-28 14:53:14 -04:00 (Migrated from github.com)
Review

⚠️ Potential issue

Breaking change: Wayland no longer in default features.

Removing wayland from default features might break existing users. Consider documenting this change prominently or keeping it as default if it's commonly used.

🤖 Prompt for AI Agents
In Cargo.toml at line 24, the default features list has been changed to exclude
"wayland," which is a breaking change. To fix this, either add "wayland" back to
the default features array if it is commonly used or update the documentation
clearly highlighting this removal so users are aware of the change and can
adjust their dependencies accordingly.
_⚠️ Potential issue_ **Breaking change: Wayland no longer in default features.** Removing `wayland` from default features might break existing users. Consider documenting this change prominently or keeping it as default if it's commonly used. <details> <summary>🤖 Prompt for AI Agents</summary> ``` In Cargo.toml at line 24, the default features list has been changed to exclude "wayland," which is a breaking change. To fix this, either add "wayland" back to the default features array if it is commonly used or update the documentation clearly highlighting this removal so users are aware of the change and can adjust their dependencies accordingly. ``` </details> <!-- This is an auto-generated comment by CodeRabbit --> <!-- fingerprinting:phantom:medusa:lion -->
profile_tokio = ["dep:console-subscriber", "tokio/tracing"]
profile_app = ["dep:tracing-tracy"]
local_deps = ["stereokit-rust/force-local-deps"]
[package.metadata.appimage]
auto_link = true
auto_link_exclude_list = [
"libc*",
"libdl*",
"libpthread*",
"ld-linux*",
"libGL*",
"libEGL*",
]
auto_link_exclude_list = ["libc*", "libdl*", "libpthread*", "ld-linux*"]
[profile.dev.package."*"]
opt-level = 3
debug = true
strip = false
debug-assertions = true
overflow-checks = true
[profile.release]
opt-level = 3
debug = "line-tables-only"
strip = true
debug-assertions = true
overflow-checks = false
lto = "thin"
# [profile.release]
# opt-level = 3
# debug = "line-tables-only"
# strip = true
# debug-assertions = true
# overflow-checks = false
# lto = "thin"
[patch.crates-io]
bevy_mod_openxr = { git = "https://github.com/Schmarni-Dev/bevy_openxr", branch = "non_default_wait_frame_system" }
bevy_mod_xr = { git = "https://github.com/Schmarni-Dev/bevy_openxr", branch = "non_default_wait_frame_system" }
[dependencies]
# small utility thingys
@@ -63,6 +55,7 @@ send_wrapper = "0.6.0"
slotmap = "1.0.7"
global_counter = "=0.2.2"
parking_lot = "0.12.3"
dashmap = "6.1.0"
# rust errors/logging
color-eyre = { version = "0.6.3", default-features = false }
@@ -83,6 +76,17 @@ glam = { version = "0.29.0", features = ["mint", "serde"] }
mint = "0.5.9"
tokio = { version = "1.39.2", features = ["rt-multi-thread", "signal", "time"] }
coderabbitai[bot] commented 2025-06-28 14:53:14 -04:00 (Migrated from github.com)
Review

⚠️ Potential issue

Local path dependency will cause issues for other developers.

The local path dependency bevy_sk.path = "../bevy_sk" won't work for other developers or in CI/CD.

Consider using:

  • A git repository with a specific commit
  • Publishing to crates.io
  • A workspace member if it's part of this project
🤖 Prompt for AI Agents
In Cargo.toml at line 89, the local path dependency bevy_sk.path = "../bevy_sk"
should be replaced because it causes issues for other developers and CI/CD.
Instead, specify bevy_sk as a git dependency with a repository URL and a
specific commit hash, or publish the crate to crates.io and reference it by
version, or if bevy_sk is part of this project, include it as a workspace member
in Cargo.toml.
_⚠️ Potential issue_ **Local path dependency will cause issues for other developers.** The local path dependency `bevy_sk.path = "../bevy_sk"` won't work for other developers or in CI/CD. Consider using: - A git repository with a specific commit - Publishing to crates.io - A workspace member if it's part of this project <details> <summary>🤖 Prompt for AI Agents</summary> ``` In Cargo.toml at line 89, the local path dependency bevy_sk.path = "../bevy_sk" should be replaced because it causes issues for other developers and CI/CD. Instead, specify bevy_sk as a git dependency with a repository URL and a specific commit hash, or publish the crate to crates.io and reference it by version, or if bevy_sk is part of this project, include it as a workspace member in Cargo.toml. ``` </details> <!-- This is an auto-generated comment by CodeRabbit --> <!-- fingerprinting:phantom:medusa:lion -->
# bevy
bevy = { version = "0.16", features = ["wayland", "bevy_remote"] }
bevy_mod_xr = "0.3"
bevy_mod_openxr = "0.3"
bevy_mod_meshtext.git = "https://github.com/Schmarni-Dev/bevy_mod_meshtext"
# bevy_sk.git = "https://github.com/MalekiRe/bevy_sk"
# bevy_sk = { git = "https://github.com/Schmarni-Dev/bevy_sk", branch = "fix_mat_stuff" }
bevy_sk.path = "../bevy_sk"
openxr = "0.19"
# linux stuffs
input-event-codes = "6.2.0"
zbus = { version = "5.0.0", default-features = false, features = ["tokio"] }
@@ -92,7 +96,7 @@ xkbcommon-rs = "0.1.0"
# wayland
wayland-backend = { version = "0.3.7", optional = true, default-features = false }
wayland-scanner = { version = "0.31.4", optional = true }
dashmap = "6.1.0"
[dependencies.smithay]
git = "https://github.com/smithay/smithay.git"
@@ -100,7 +104,6 @@ default-features = false
features = ["desktop", "backend_drm", "renderer_gl", "wayland_frontend"]
optional = true
[dependencies.stereokit-rust]
git = "https://github.com/mvvvv/StereoKit-rust.git"
rev = "73ffaae6f42aa369e599a6ea0391f77840d682d8"

View File

@@ -26,7 +26,7 @@ use std::{
time::Instant,
};
use tokio::{net::UnixStream, sync::watch, task::JoinHandle};
use tracing::info;
use tracing::{info, warn};
lazy_static! {
pub static ref CLIENTS: OwnedRegistry<Client> = OwnedRegistry::new();
@@ -201,7 +201,9 @@ impl Client {
std::fs::read_link(cwd_proc_path).ok()
}
pub async fn save_state(&self) -> Option<ClientStateParsed> {
println!("start save state");
let internal = self.root.get()?.save_state().await.ok()?;
println!("finished save state");
Some(ClientStateParsed::from_deserialized(self, internal))
}

View File

@@ -10,11 +10,39 @@ use crate::core::destroy_queue;
use crate::nodes::items::camera;
use crate::nodes::{audio, drawable, input};
use bevy::MinimalPlugins;
use bevy::a11y::AccessibilityPlugin;
use bevy::app::{App, ScheduleRunnerPlugin, TerminalCtrlCHandlerPlugin};
use bevy::asset::{AssetMetaCheck, UnapprovedPathMode};
use bevy::audio::AudioPlugin;
use bevy::core_pipeline::CorePipelinePlugin;
use bevy::gizmos::GizmoPlugin;
use bevy::gltf::GltfPlugin;
use bevy::input::{InputPlugin, InputSystem};
use bevy::pbr::PbrPlugin;
use bevy::remote::RemotePlugin;
use bevy::remote::http::RemoteHttpPlugin;
use bevy::render::{RenderDebugFlags, RenderPlugin};
use bevy::scene::ScenePlugin;
use bevy::text::FontLoader;
use bevy::winit::{WakeUp, WinitPlugin};
use bevy_mod_meshtext::MeshTextPlugin;
use bevy_mod_openxr::add_xr_plugins;
use bevy_mod_openxr::exts::OxrExtensions;
use bevy_mod_openxr::features::overlay::OxrOverlaySettings;
use bevy_mod_openxr::init::OxrInitPlugin;
use bevy_mod_openxr::reference_space::OxrReferenceSpacePlugin;
use bevy_mod_openxr::resources::OxrSessionConfig;
use bevy_mod_openxr::types::AppInfo;
use bevy_mod_xr::hand_debug_gizmos::HandGizmosPlugin;
use clap::Parser;
use core::client::{Client, tick_internal_client};
use core::task;
use directories::ProjectDirs;
use nodes::drawable::model::ModelNodePlugin;
use nodes::spatial::SpatialNodePlugin;
use objects::ServerObjects;
use openxr::{EnvironmentBlendMode, ReferenceSpaceType};
use session::{launch_start, save_session};
use stardust_xr::schemas::dbus::object_registry::ObjectRegistry;
use stardust_xr::server;
@@ -38,6 +66,8 @@ use tracing_subscriber::{EnvFilter, fmt, prelude::*};
use zbus::Connection;
use zbus::fdo::ObjectManager;
use bevy::prelude::*;
#[derive(Debug, Clone, Parser)]
#[clap(author, version, about, long_about = None)]
struct CliArgs {
@@ -176,7 +206,7 @@ async fn main() {
let cli_args = cli_args.clone();
let dbus_connection = dbus_connection.clone();
move || {
stereokit_loop(
bevy_loop(
sk_ready_notifier,
project_dirs,
cli_args,
@@ -209,6 +239,129 @@ async fn main() {
static DEFAULT_SKYTEX: OnceLock<Tex> = OnceLock::new();
static DEFAULT_SKYLIGHT: OnceLock<SphericalHarmonics> = OnceLock::new();
fn bevy_loop(
ready_notifier: Arc<Notify>,
project_dirs: Option<ProjectDirs>,
args: CliArgs,
dbus_connection: Connection,
object_registry: ObjectRegistry,
) {
let mut app = App::new();
app.add_plugins(AssetPlugin {
meta_check: AssetMetaCheck::Never,
unapproved_path_mode: UnapprovedPathMode::Allow,
..default()
});
let mut plugins = MinimalPlugins
.build()
.disable::<ScheduleRunnerPlugin>()
.add(TransformPlugin)
.add(InputPlugin)
/* .add(AccessibilityPlugin) */;
coderabbitai[bot] commented 2025-06-28 14:53:14 -04:00 (Migrated from github.com)
Review

⚠️ Potential issue

Fix undefined wayland variable in xr_step function.

The wayland variable is referenced but not defined in this function scope. This will cause compilation errors when the wayland feature is enabled.

The wayland instance needs to be made accessible to the xr_step function. Consider these options:

  1. Store it as a Bevy resource:
#[cfg(feature = "wayland")]
#[derive(Resource)]
struct WaylandResource(wayland::Wayland);
  1. Or remove these calls if wayland support is not yet implemented (as mentioned in the PR description).
🤖 Prompt for AI Agents
In src/main.rs between lines 433 and 473, the xr_step function references the
wayland variable without defining or accessing it, causing compilation errors
when the wayland feature is enabled. To fix this, make the wayland instance
accessible inside xr_step by storing it as a Bevy resource (e.g., define a
WaylandResource struct wrapping wayland::Wayland and insert it into the world
resources), then retrieve and use it within xr_step. Alternatively, if wayland
support is not implemented yet, remove or comment out the wayland-related calls
to avoid undefined variable errors.

Addressed in commits d2afa00 to aec875a

_⚠️ Potential issue_ **Fix undefined `wayland` variable in xr_step function.** The `wayland` variable is referenced but not defined in this function scope. This will cause compilation errors when the wayland feature is enabled. The wayland instance needs to be made accessible to the xr_step function. Consider these options: 1. Store it as a Bevy resource: ```rust #[cfg(feature = "wayland")] #[derive(Resource)] struct WaylandResource(wayland::Wayland); ``` 2. Or remove these calls if wayland support is not yet implemented (as mentioned in the PR description). <details> <summary>🤖 Prompt for AI Agents</summary> ``` In src/main.rs between lines 433 and 473, the xr_step function references the wayland variable without defining or accessing it, causing compilation errors when the wayland feature is enabled. To fix this, make the wayland instance accessible inside xr_step by storing it as a Bevy resource (e.g., define a WaylandResource struct wrapping wayland::Wayland and insert it into the world resources), then retrieve and use it within xr_step. Alternatively, if wayland support is not implemented yet, remove or comment out the wayland-related calls to avoid undefined variable errors. ``` </details> <!-- This is an auto-generated comment by CodeRabbit --> <!-- fingerprinting:phantom:medusa:lion --> ✅ Addressed in commits d2afa00 to aec875a
coderabbitai[bot] commented 2025-06-28 14:53:14 -04:00 (Migrated from github.com)
Review

💡 Verification agent

🧩 Analysis chain

Verify the extremely small near plane value.

The near plane is set to 0.003 meters (3mm), which is unusually small and may cause z-fighting or depth precision issues.

Consider using a more typical value like 0.01 or 0.05 meters for VR applications. The small value might be intentional for specific use cases, but it's worth verifying.


🌐 Web query:

What is the recommended near plane distance for VR/XR applications to avoid z-fighting and maintain depth precision?

💡 Result:

In VR/XR applications, the recommended near plane distance is typically set to a value that balances depth precision and minimizes z-fighting.

A common guideline is to set the near plane distance to around 0.1 meters (10 centimeters) or slightly above. This is because:

  • Setting the near plane too close (e.g., less than 0.01 meters) severely reduces depth buffer precision, causing z-fighting at farther distances.
  • Setting it too far can clip nearby objects that are important to the VR experience.

For VR, which often requires accurate depth perception at close range, 0.1 meters is a good starting point. Some developers go as close as 0.05 meters if their engine and hardware support higher precision depth buffers (like using 24 or 32-bit depth buffers), but going below that usually causes depth fighting.

To sum up:

  • Recommended near plane distance: ~0.1 meters (10 cm)
  • Ensure a far plane distance is not excessively large to maximize depth buffer precision.
  • Use a 24-bit or 32-bit depth buffer to improve precision.
  • Adjust based on your scene scale and hardware capabilities.

If you want, I can also look up the latest best practices or guidelines from specific VR platforms or engines. Would you like me to do that?


Re-evaluate the near‐plane setting (0.003 m)

A 3 mm near plane is well below common VR/XR guidelines (typically ≥ 0.05–0.1 m) and can severely degrade depth‐buffer precision, leading to z-fighting. Please confirm whether such an ultra-close near plane is required for your use case, or consider increasing it to a more typical value (e.g., 0.05–0.1 m).

• Location: src/main.rs, lines 415–431
• All three branches set near = 0.003 (Perspective, Orthographic, and XrProjection)

Suggested change example:

--- a/src/main.rs
+++ b/src/main.rs
@@ fn update_cameras(mut camera: Query<&mut Projection, (With<Camera3d>,)>) {
-           Projection::Perspective(p) => p.near = 0.003,
+           Projection::Perspective(p) => p.near = 0.1,     // consider 0.05–0.1 m for VR/XR
@@
-           Projection::Orthographic(o) => o.near = 0.003,
+           Projection::Orthographic(o) => o.near = 0.1,
@@
-                   xr.near = 0.003
+                   xr.near = 0.1
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

fn update_cameras(mut camera: Query<&mut Projection, (With<Camera3d>,)>) {
    for mut projection in &mut camera {
        match projection.deref_mut() {
            // consider 0.05–0.1 m for VR/XR instead of 0.003 m
            Projection::Perspective(perspective_projection) => perspective_projection.near = 0.1,
            Projection::Orthographic(orthographic_projection) => {
                orthographic_projection.near = 0.1
            }
            Projection::Custom(custom_projection) => {
                if let Some(xr) = custom_projection.get_mut::<XrProjection>() {
                    xr.near = 0.1
                } else {
                    error_once!("unknown custom camera projection");
                }
            }
        }
    }
}
🤖 Prompt for AI Agents
In src/main.rs around lines 415 to 431, the near plane for all camera
projections is set to 0.003, which is unusually close and can cause depth
precision issues. Review your application's requirements to determine if such a
small near plane is necessary; if not, increase the near plane distance to a
more standard value between 0.05 and 0.1 meters by updating the assignments in
all branches of the match statement accordingly.

Addressed in commits d2afa00 to aec875a

_💡 Verification agent_ <details> <summary>🧩 Analysis chain</summary> **Verify the extremely small near plane value.** The near plane is set to 0.003 meters (3mm), which is unusually small and may cause z-fighting or depth precision issues. Consider using a more typical value like 0.01 or 0.05 meters for VR applications. The small value might be intentional for specific use cases, but it's worth verifying. --- 🌐 Web query: ```web What is the recommended near plane distance for VR/XR applications to avoid z-fighting and maintain depth precision? ``` 💡 Result: In VR/XR applications, the recommended near plane distance is typically set to a value that balances depth precision and minimizes z-fighting. A common guideline is to set the near plane distance to around **0.1 meters (10 centimeters)** or slightly above. This is because: - Setting the near plane too close (e.g., less than 0.01 meters) severely reduces depth buffer precision, causing z-fighting at farther distances. - Setting it too far can clip nearby objects that are important to the VR experience. For VR, which often requires accurate depth perception at close range, 0.1 meters is a good starting point. Some developers go as close as 0.05 meters if their engine and hardware support higher precision depth buffers (like using 24 or 32-bit depth buffers), but going below that usually causes depth fighting. To sum up: - **Recommended near plane distance:** ~0.1 meters (10 cm) - Ensure a far plane distance is not excessively large to maximize depth buffer precision. - Use a 24-bit or 32-bit depth buffer to improve precision. - Adjust based on your scene scale and hardware capabilities. If you want, I can also look up the latest best practices or guidelines from specific VR platforms or engines. Would you like me to do that? --- </details> **Re-evaluate the near‐plane setting (0.003 m)** A 3 mm near plane is well below common VR/XR guidelines (typically ≥ 0.05–0.1 m) and can severely degrade depth‐buffer precision, leading to z-fighting. Please confirm whether such an ultra-close near plane is required for your use case, or consider increasing it to a more typical value (e.g., 0.05–0.1 m). • Location: `src/main.rs`, lines 415–431 • All three branches set `near = 0.003` (Perspective, Orthographic, and `XrProjection`) Suggested change example: ```diff --- a/src/main.rs +++ b/src/main.rs @@ fn update_cameras(mut camera: Query<&mut Projection, (With<Camera3d>,)>) { - Projection::Perspective(p) => p.near = 0.003, + Projection::Perspective(p) => p.near = 0.1, // consider 0.05–0.1 m for VR/XR @@ - Projection::Orthographic(o) => o.near = 0.003, + Projection::Orthographic(o) => o.near = 0.1, @@ - xr.near = 0.003 + xr.near = 0.1 ``` <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion fn update_cameras(mut camera: Query<&mut Projection, (With<Camera3d>,)>) { for mut projection in &mut camera { match projection.deref_mut() { // consider 0.05–0.1 m for VR/XR instead of 0.003 m Projection::Perspective(perspective_projection) => perspective_projection.near = 0.1, Projection::Orthographic(orthographic_projection) => { orthographic_projection.near = 0.1 } Projection::Custom(custom_projection) => { if let Some(xr) = custom_projection.get_mut::<XrProjection>() { xr.near = 0.1 } else { error_once!("unknown custom camera projection"); } } } } } ``` </details> <!-- suggestion_end --> <details> <summary>🤖 Prompt for AI Agents</summary> ``` In src/main.rs around lines 415 to 431, the near plane for all camera projections is set to 0.003, which is unusually close and can cause depth precision issues. Review your application's requirements to determine if such a small near plane is necessary; if not, increase the near plane distance to a more standard value between 0.05 and 0.1 meters by updating the assignments in all branches of the match statement accordingly. ``` </details> <!-- This is an auto-generated comment by CodeRabbit --> <!-- fingerprinting:phantom:medusa:lion --> ✅ Addressed in commits d2afa00 to aec875a
// TODO: figure out headless
{
plugins = plugins.add(WindowPlugin::default()).add({
let mut winit = WinitPlugin::<WakeUp>::default();
winit.run_on_any_thread = true;
winit
});
}
plugins = plugins
.add(TerminalCtrlCHandlerPlugin)
// bevy_mod_openxr will replace this, TODO: figure out how to mix this with
// bevy-dmabuf
.add(RenderPlugin::default())
.add(ImagePlugin::default())
.add(CorePipelinePlugin)
// theoretically we shouldn't need this because of bevy_sk, but everything is tangled in
// there and idk what we actually need to run
.add(PbrPlugin {
// this seems to only apply to StandardMaterial, we don't use that
prepass_enabled: true,
add_default_deferred_lighting_plugin: false,
use_gpu_instance_buffer_builder: true,
debug_flags: RenderDebugFlags::default(),
})
// required for gltf
.add(ScenePlugin)
.add(GltfPlugin::default())
.add(AudioPlugin::default())
.add(GizmoPlugin)
.add(AccessibilityPlugin);
app.add_plugins(
add_xr_plugins(plugins)
.set(OxrInitPlugin {
app_info: AppInfo {
name: "Stardust XR".into(),
version: bevy_mod_openxr::types::Version(0, 44, 1),
},
exts: {
// all OpenXR extensions can be requested here
let mut exts = OxrExtensions::default();
exts.enable_hand_tracking();
if args.overlay_priority.is_some() {
exts.enable_extx_overlay();
}
exts
},
..default()
})
.set(OxrReferenceSpacePlugin {
default_primary_ref_space: ReferenceSpaceType::LOCAL,
}),
);
app.init_asset::<Font>().init_asset_loader::<FontLoader>();
if let Some(priority) = args.overlay_priority {
app.insert_resource(OxrOverlaySettings {
session_layer_placement: priority,
..default()
});
}
app.add_plugins((
bevy_sk::hand::HandPlugin,
bevy_sk::vr_materials::SkMaterialPlugin {
replace_standard_material: true,
},
bevy_sk::skytext::SphericalHarmonicsPlugin,
));
app.add_plugins(HandGizmosPlugin);
app.add_plugins(MeshTextPlugin);
app.insert_resource(OxrSessionConfig {
blend_modes: Some(vec![
EnvironmentBlendMode::ALPHA_BLEND,
EnvironmentBlendMode::ADDITIVE,
EnvironmentBlendMode::OPAQUE,
]),
..default()
});
app.insert_resource(ClearColor(Color::BLACK.with_alpha(0.0)));
app.add_plugins((RemotePlugin::default(), RemoteHttpPlugin::default()));
app.add_plugins((SpatialNodePlugin, ModelNodePlugin));
ready_notifier.notify_waiters();
app.add_systems(PreUpdate, main_loop_system.after(InputSystem));
app.run();
}
fn main_loop_system(world: &mut World) {
// camera::update(token);
#[cfg(feature = "wayland")]
wayland.frame_event();
destroy_queue::clear();
// objects.update(&sk, token, &dbus_connection, &object_registry);
// input::process_input();
let time = world.resource::<bevy::prelude::Time>().delta_secs_f64();
nodes::root::Root::send_frame_events(time);
// Wait
tick_internal_client();
#[cfg(feature = "wayland")]
wayland.update();
// drawable::draw(token);
// audio::update();
}
fn stereokit_loop(
sk_ready_notifier: Arc<Notify>,
project_dirs: Option<ProjectDirs>,

View File

@@ -23,7 +23,6 @@ use stereokit_rust::{sk::MainThreadToken, system::Renderer, tex::SHCubemap};
// #[instrument(level = "debug", skip(sk))]
pub fn draw(token: &MainThreadToken) {
lines::draw_all(token);
model::draw_all(token);
text::draw_all(token);
match QUEUED_SKYTEX.lock().take() {
Some(Some(skytex)) => {

View File

@@ -6,128 +6,350 @@ use crate::core::registry::Registry;
use crate::core::resource::get_resource_file;
use crate::nodes::Node;
use crate::nodes::alias::{Alias, AliasList};
use crate::nodes::spatial::Spatial;
use crate::nodes::spatial::{Spatial, SpatialNode};
use bevy::prelude::*;
use bevy_sk::vr_materials::PbrMaterial;
use color_eyre::eyre::eyre;
use glam::{Mat4, Vec2, Vec3};
use parking_lot::Mutex;
use rustc_hash::FxHashMap;
use rustc_hash::{FxHashMap, FxHasher};
use stardust_xr::values::ResourceID;
use std::ffi::OsStr;
use std::hash::{Hash, Hasher};
use std::sync::{Arc, LazyLock, OnceLock, Weak};
use stereokit_rust::material::Transparency;
use std::path::PathBuf;
use std::sync::{Arc, OnceLock, Weak};
use stereokit_rust::maths::Bounds;
use stereokit_rust::sk::MainThreadToken;
use stereokit_rust::{material::Material, model::Model as SKModel, tex::Tex, util::Color128};
use tokio::sync::mpsc;
pub struct MaterialWrapper(pub Material);
impl Drop for MaterialWrapper {
fn drop(&mut self) {
MATERIAL_REGISTRY.remove(self);
static LOAD_MODEL: OnceLock<mpsc::UnboundedSender<(Arc<Model>, PathBuf)>> = OnceLock::new();
pub struct ModelNodePlugin;
impl Plugin for ModelNodePlugin {
fn build(&self, app: &mut App) {
let (tx, rx) = mpsc::unbounded_channel();
LOAD_MODEL.set(tx).unwrap();
app.insert_resource(MpscReceiver(rx));
app.add_systems(Update, load_models);
app.add_systems(PostUpdate, (gen_model_parts, apply_materials).chain());
}
}
impl Hash for MaterialWrapper {
fn hash<H: Hasher>(&self, state: &mut H) {
self.0.get_shader().0.as_ptr().hash(state);
for param in self.0.get_all_param_info() {
param.name.hash(state);
param.to_string().hash(state);
}
self.0.get_chain().map(MaterialWrapper).hash(state)
}
}
impl PartialEq for MaterialWrapper {
fn eq(&self, other: &Self) -> bool {
if self.0.get_shader().0.as_ptr() != other.0.get_shader().0.as_ptr() {
return false;
}
if self.0.get_all_param_info().count() != other.0.get_all_param_info().count() {
return false;
}
for self_param in self.0.get_all_param_info() {
let Some(other_param) = other
.0
.get_all_param_info()
.get_data(self_param.get_name(), self_param.get_type())
else {
return false;
};
if self_param.to_string() != other_param.to_string() {
return false;
}
}
self.0.get_chain().map(MaterialWrapper) == other.0.get_chain().map(MaterialWrapper)
}
}
impl Eq for MaterialWrapper {}
unsafe impl Send for MaterialWrapper {}
unsafe impl Sync for MaterialWrapper {}
#[derive(Resource)]
struct MpscReceiver<T>(mpsc::UnboundedReceiver<T>);
#[derive(Component)]
struct ModelNode(Weak<Model>);
#[derive(Default)]
struct MaterialRegistry(Mutex<FxHashMap<u64, Weak<MaterialWrapper>>>);
impl MaterialRegistry {
fn add_or_get(&self, material: Arc<MaterialWrapper>) -> Arc<MaterialWrapper> {
let hash = {
use std::hash::{Hash, Hasher};
let mut hasher = std::collections::hash_map::DefaultHasher::new();
material.hash(&mut hasher);
hasher.finish()
fn load_models(
asset_server: Res<AssetServer>,
mut cmds: Commands,
mut mpsc_receiver: ResMut<MpscReceiver<(Arc<Model>, PathBuf)>>,
) {
while let Ok((model, path)) = mpsc_receiver.0.try_recv() {
// idk of the asset label is the correct approach here
let handle = asset_server.load(GltfAssetLabel::Scene(0).from_asset(path));
let entity = cmds
.spawn((
SceneRoot(handle),
ModelNode(Arc::downgrade(&model)),
SpatialNode(Arc::downgrade(&model.space)),
))
.id();
model.bevy_scene_entity.set(entity).unwrap();
}
}
fn apply_materials(
mut query: Query<&mut MeshMaterial3d<PbrMaterial>>,
mut material_registry: Local<MaterialRegistry>,
asset_server: Res<AssetServer>,
mut materials: ResMut<Assets<PbrMaterial>>,
) -> bevy::prelude::Result {
for model_part in MODEL_REGISTRY
.get_valid_contents()
.iter()
.filter_map(|p| p.parts.get())
.flatten()
{
let mut mesh_mat = query.get_mut(*model_part.mesh_entity.get().unwrap())?;
if let Some(material) = model_part.pending_material_replacement.lock().take() {
let pbr_mat = material.to_pbr_mat(&asset_server);
let handle = material_registry.get_handle(pbr_mat, &mut materials);
mesh_mat.0 = handle;
}
for (param_name, param) in model_part.pending_material_parameters.lock().drain() {
let mut new_mat = materials.get(&mesh_mat.0).unwrap().clone();
param.apply_to_material(
&model_part.space.node().unwrap().get_client().unwrap(),
&mut new_mat,
&param_name,
&asset_server,
);
let handle = material_registry.get_handle(new_mat, &mut materials);
mesh_mat.0 = handle;
}
}
Ok(())
}
fn gen_model_parts(
coderabbitai[bot] commented 2025-06-28 14:53:15 -04:00 (Migrated from github.com)
Review

🛠️ Refactor suggestion

Handle GLTF files with multiple scenes.

The code assumes scene index 0 exists, which may not be true for all GLTF files. Consider making this configurable or loading the default scene.

-let handle = asset_server.load(GltfAssetLabel::Scene(0).from_asset(path));
+// Use the default scene if available, otherwise scene 0
+let handle = asset_server.load::<Scene>(path.clone());

Alternatively, load the GLTF asset first to inspect available scenes:

let gltf_handle = asset_server.load::<Gltf>(path);
// Then in another system, check gltf.default_scene or iterate gltf.scenes
🤖 Prompt for AI Agents
In src/nodes/drawable/model.rs at line 68, the code currently loads a GLTF asset
assuming scene index 0 exists, which may not be valid for all GLTF files. To fix
this, first load the GLTF asset without specifying a scene, then in a separate
system or step, inspect the loaded GLTF to determine the default scene or
available scenes. Adjust the scene index dynamically based on this inspection
rather than hardcoding 0.
_🛠️ Refactor suggestion_ **Handle GLTF files with multiple scenes.** The code assumes scene index 0 exists, which may not be true for all GLTF files. Consider making this configurable or loading the default scene. ```diff -let handle = asset_server.load(GltfAssetLabel::Scene(0).from_asset(path)); +// Use the default scene if available, otherwise scene 0 +let handle = asset_server.load::<Scene>(path.clone()); ``` Alternatively, load the GLTF asset first to inspect available scenes: ```rust let gltf_handle = asset_server.load::<Gltf>(path); // Then in another system, check gltf.default_scene or iterate gltf.scenes ``` <details> <summary>🤖 Prompt for AI Agents</summary> ``` In src/nodes/drawable/model.rs at line 68, the code currently loads a GLTF asset assuming scene index 0 exists, which may not be valid for all GLTF files. To fix this, first load the GLTF asset without specifying a scene, then in a separate system or step, inspect the loaded GLTF to determine the default scene or available scenes. Adjust the scene index dynamically based on this inspection rather than hardcoding 0. ``` </details> <!-- This is an auto-generated comment by CodeRabbit --> <!-- fingerprinting:phantom:medusa:lion -->
scenes: Res<Assets<Scene>>,
query: Query<(Entity, &SceneRoot, &ModelNode, &Children)>,
children_query: Query<&Children>,
part_query: Query<(&Name, Option<&Children>, &Transform), Without<Mesh3d>>,
has_mesh: Query<Has<Mesh3d>>,
mut cmds: Commands,
) {
for (entity, scene_root, model_node, model_children) in query.iter() {
let Some(model) = model_node.0.upgrade() else {
cmds.entity(entity).despawn();
return;
};
let mut lock = self.0.lock();
if let Some(mat) = lock.get(&hash) {
if let Some(mat) = mat.upgrade() {
return mat;
}
if model.parts.get().is_some() {
continue;
}
lock.insert(hash, Arc::downgrade(&material));
material
}
fn remove(&self, material: &MaterialWrapper) {
let hash = {
use std::hash::{Hash, Hasher};
let mut hasher = std::collections::hash_map::DefaultHasher::new();
material.hash(&mut hasher);
hasher.finish()
};
let mut lock = self.0.lock();
lock.remove(&hash);
if scenes.get(scene_root.0.id()).is_none() {
continue;
}
let mut parts = Vec::new();
for entity in model_children
.iter()
.filter_map(|e| children_query.get(e).ok())
.flat_map(|c| c.iter())
{
gen_path(
entity,
&part_query,
None,
&mut |entity, name, transform, parent| {
let path = parent
.as_ref()
.map(|p| format!("{}/{}", &p.path, name.as_str()))
.unwrap_or_else(|| name.to_string());
let parent_spatial = parent
.as_ref()
.map(|p| p.space.clone())
.unwrap_or_else(|| model.space.clone());
let client = model.space.node()?.get_client()?;
let (spatial, model_part) =
match model.pre_bound_parts.lock().iter().find(|v| v.path == path) {
None => {
let node =
client.scenegraph.add_node(Node::generate(&client, false));
let spatial = Spatial::add_to(
&node,
Some(parent_spatial),
transform.compute_matrix(),
false,
);
let model_part = node.add_aspect(ModelPart {
entity: OnceLock::new(),
mesh_entity: OnceLock::new(),
path,
space: spatial.clone(),
model: Arc::downgrade(&model),
pending_material_parameters: Mutex::default(),
pending_material_replacement: Mutex::default(),
aliases: AliasList::default(),
});
(spatial, model_part)
}
Some(part) => {
part.space
.set_spatial_parent(Some(&parent_spatial))
.unwrap();
(part.space.clone(), part.clone())
}
};
_ = spatial.bounding_box_calc.set(|_| {
// TODO: actually impl aabb
Bounds::default()
});
cmds.entity(entity)
.insert(SpatialNode(Arc::downgrade(&spatial)));
let mesh_entity = children_query
.get(entity)
.iter()
.flat_map(|v| v.iter())
.find(|e| has_mesh.get(*e).unwrap_or(false))?;
_ = model_part.entity.set(entity);
_ = model_part.mesh_entity.set(mesh_entity);
parts.push(model_part.clone());
Some(model_part)
},
);
}
_ = model.parts.set(parts);
}
}
static MATERIAL_REGISTRY: LazyLock<MaterialRegistry> = LazyLock::new(MaterialRegistry::default);
fn gen_path(
current_entity: Entity,
part_query: &Query<(&Name, Option<&Children>, &Transform), Without<Mesh3d>>,
parent: Option<Arc<ModelPart>>,
func: &mut dyn FnMut(
Entity,
&Name,
&Transform,
Option<Arc<ModelPart>>,
) -> Option<Arc<ModelPart>>,
) {
let Ok((name, children, transform)) = part_query.get(current_entity) else {
return;
};
let Some(parent) = func(current_entity, name, transform, parent) else {
return;
};
for e in children.iter().flat_map(|c| c.iter()) {
gen_path(e, part_query, Some(parent.clone()), func);
}
}
#[derive(PartialEq, Deref, DerefMut, Clone, Copy, Eq, PartialOrd, Ord, Hash)]
struct HashedPbrMaterial(u64);
impl HashedPbrMaterial {
fn new(material: &PbrMaterial) -> Self {
let mut hasher = FxHasher::default();
Self::hash_pbr_mat(material, &mut hasher);
Self(hasher.finish())
}
fn hash_pbr_mat<H: Hasher>(mat: &PbrMaterial, state: &mut H) {
hash_color(mat.color, state);
hash_color(mat.emission_factor, state);
state.write_u32(mat.metallic.to_bits());
state.write_u32(mat.roughness.to_bits());
mat.use_stereokit_uvs.hash(state);
match mat.alpha_mode {
AlphaMode::Opaque => state.write_u8(0),
AlphaMode::Mask(v) => {
state.write_u8(1);
state.write_u32(v.to_bits());
}
AlphaMode::Blend => state.write_u8(2),
AlphaMode::Premultiplied => state.write_u8(3),
AlphaMode::AlphaToCoverage => state.write_u8(4),
AlphaMode::Add => state.write_u8(5),
AlphaMode::Multiply => state.write_u8(6),
}
state.write_u8(mat.double_sided as u8);
mat.diffuse_texture.hash(state);
mat.emission_texture.hash(state);
mat.metal_texture.hash(state);
mat.occlusion_texture.hash(state);
// should always be the same, TODO: make the spherical harmonics buffer a per mesh instance thing
mat.spherical_harmonics.hash(state);
}
}
fn hash_color<H: Hasher>(color: Color, state: &mut H) {
match color {
Color::Srgba(srgba) => {
state.write_u8(0);
state.write(&srgba.to_u8_array());
}
Color::LinearRgba(linear_rgba) => {
state.write_u8(1);
state.write(&linear_rgba.to_u8_array());
}
Color::Hsla(hsla) => {
state.write_u8(2);
hsla.to_f32_array()
.iter()
.for_each(|v| state.write_u32(v.to_bits()));
}
Color::Hsva(hsva) => {
state.write_u8(3);
hsva.to_f32_array()
.iter()
.for_each(|v| state.write_u32(v.to_bits()));
}
Color::Hwba(hwba) => {
state.write_u8(4);
hwba.to_f32_array()
.iter()
.for_each(|v| state.write_u32(v.to_bits()));
}
Color::Laba(laba) => {
state.write_u8(5);
laba.to_f32_array()
.iter()
.for_each(|v| state.write_u32(v.to_bits()));
}
Color::Lcha(lcha) => {
state.write_u8(6);
lcha.to_f32_array()
.iter()
.for_each(|v| state.write_u32(v.to_bits()));
}
Color::Oklaba(oklaba) => {
state.write_u8(7);
oklaba
.to_f32_array()
.iter()
.for_each(|v| state.write_u32(v.to_bits()));
}
Color::Oklcha(oklcha) => {
state.write_u8(8);
oklcha
.to_f32_array()
.iter()
.for_each(|v| state.write_u32(v.to_bits()));
}
Color::Xyza(xyza) => {
state.write_u8(9);
xyza.to_f32_array()
.iter()
.for_each(|v| state.write_u32(v.to_bits()));
}
}
}
static MODEL_REGISTRY: Registry<Model> = Registry::new();
static HOLDOUT_MATERIAL: OnceLock<Arc<MaterialWrapper>> = OnceLock::new();
impl MaterialParameter {
fn apply_to_material(&self, client: &Client, material: &Material, parameter_name: &str) {
let mut params = material.get_all_param_info();
fn apply_to_material(
&self,
client: &Client,
mat: &mut PbrMaterial,
parameter_name: &str,
asset_server: &AssetServer,
) {
match self {
MaterialParameter::Bool(val) => {
params.set_bool(parameter_name, *val);
MaterialParameter::Bool(val) => match parameter_name {
"double_sided" => mat.double_sided = *val,
v => {
error!("unknown param_name ({v}) for color")
}
},
MaterialParameter::Int(_val) => {
// nothing uses an int
}
MaterialParameter::Int(val) => {
params.set_int(parameter_name, &[*val]);
}
MaterialParameter::UInt(val) => {
params.set_uint(parameter_name, &[*val]);
MaterialParameter::UInt(_val) => {
// nothing uses an uint
}
MaterialParameter::Float(val) => {
params.set_float(parameter_name, *val);
match parameter_name {
"metallic" => mat.metallic = *val,
"roughness" => mat.roughness = *val,
// we probably don't want to expose tex_scale
// "tex_scale" => mat.tex_scale = *val,
v => {
error!("unknown param_name ({v}) for float")
}
}
}
MaterialParameter::Vec2(val) => {
params.set_vec2(parameter_name, Vec2::from(*val));
MaterialParameter::Vec2(_val) => {
// nothing uses a Vec2
}
MaterialParameter::Vec3(val) => {
params.set_vec3(parameter_name, Vec3::from(*val));
MaterialParameter::Vec3(_val) => {
// nothing uses a Vec3
}
MaterialParameter::Color(val) => {
params.set_color(
parameter_name,
Color128::new(val.c.r, val.c.g, val.c.b, val.a),
);
let rgba = Srgba::new(val.c.r, val.c.g, val.c.b, val.a);
match parameter_name {
"color" => mat.color = rgba.into(),
"emission_factor" => mat.emission_factor = rgba.into(),
v => {
error!("unknown param_name ({v}) for color")
}
}
}
MaterialParameter::Texture(resource) => {
let Some(texture_path) =
@@ -135,180 +357,103 @@ impl MaterialParameter {
else {
return;
};
if let Ok(tex) = Tex::from_file(texture_path, true, None) {
params.set_texture(parameter_name, &tex);
info!(texture_param = parameter_name, path = ?texture_path);
let handle = asset_server.load(texture_path);
match parameter_name {
"diffuse" => mat.diffuse_texture = Some(handle),
"emission" => mat.emission_texture = Some(handle),
"metal" => mat.metal_texture = Some(handle),
"occlusion" => mat.occlusion_texture = Some(handle),
v => {
error!("unknown param_name ({v}) for texture");
return;
}
}
mat.alpha_mode = AlphaMode::AlphaToCoverage;
mat.use_stereokit_uvs = true;
}
}
}
}
pub struct Material {
pub color: Color,
pub emission_factor: Color,
pub metallic: f32,
pub roughness: f32,
pub alpha_mode: AlphaMode,
pub double_sided: bool,
pub diffuse_texture: Option<PathBuf>,
pub emission_texture: Option<PathBuf>,
pub metal_texture: Option<PathBuf>,
pub occlusion_texture: Option<PathBuf>,
}
impl Material {
fn to_pbr_mat(&self, asset_server: &AssetServer) -> PbrMaterial {
PbrMaterial {
color: self.color,
emission_factor: self.emission_factor,
metallic: self.metallic,
roughness: self.roughness,
alpha_mode: self.alpha_mode,
double_sided: self.double_sided,
diffuse_texture: self
.diffuse_texture
.as_ref()
.map(|p| asset_server.load(p.as_path())),
emission_texture: self
.emission_texture
.as_ref()
.map(|p| asset_server.load(p.as_path())),
metal_texture: self
.metal_texture
.as_ref()
.map(|p| asset_server.load(p.as_path())),
occlusion_texture: self
.occlusion_texture
.as_ref()
.map(|p| asset_server.load(p.as_path())),
spherical_harmonics: bevy_sk::skytext::SPHERICAL_HARMONICS_HANDLE,
use_stereokit_uvs: true,
}
}
}
pub struct ModelPart {
id: i32,
entity: OnceLock<Entity>,
mesh_entity: OnceLock<Entity>,
path: String,
space: Arc<Spatial>,
model: Weak<Model>,
material: Mutex<Option<Arc<MaterialWrapper>>>,
pending_material_parameters: Mutex<FxHashMap<String, MaterialParameter>>,
pending_material_replacement: Mutex<Option<Arc<MaterialWrapper>>>,
pending_material_replacement: Mutex<Option<Material>>,
aliases: AliasList,
}
impl ModelPart {
fn create_for_model(model: &Arc<Model>, sk_model: &SKModel) {
HOLDOUT_MATERIAL.get_or_init(|| {
let mut mat = Material::copy(&Material::unlit());
mat.transparency(Transparency::None);
mat.color_tint(Color128::BLACK_TRANSPARENT);
Arc::new(MaterialWrapper(mat))
});
let nodes = sk_model.get_nodes();
for part in nodes.all() {
ModelPart::create(model, &part);
}
}
fn create(model: &Arc<Model>, part: &stereokit_rust::model::ModelNode) -> Option<Arc<Self>> {
let mut parts = model.parts.lock();
let parent_part = part
.get_parent()
.and_then(|part| parts.iter().find(|p| p.id == *part.get_id()));
let stardust_model_part = model.space.node()?;
let client = stardust_model_part.get_client()?;
let mut part_path = parent_part
.map(|n| n.path.clone() + "/")
.unwrap_or_default();
part_path += part.get_name().unwrap();
let node = client.scenegraph.add_node(Node::generate(&client, false));
let spatial_parent = parent_part
.map(|n| n.space.clone())
.unwrap_or_else(|| model.space.clone());
let local_transform = unsafe { part.get_local_transform().m };
let space = Spatial::add_to(
&node,
Some(spatial_parent),
Mat4::from_cols_array(&local_transform),
false,
);
let _ = space.bounding_box_calc.set(|node| {
let Ok(model_part) = node.get_aspect::<ModelPart>() else {
return Bounds::default();
};
let Some(model) = model_part.model.upgrade() else {
return Bounds::default();
};
let Some(sk_model) = model.sk_model.get() else {
return Bounds::default();
};
let model_nodes = sk_model.get_nodes();
let Some(model_node) = model_nodes.get_index(model_part.id) else {
return Bounds::default();
};
let Some(sk_mesh) = model_node.get_mesh() else {
return Bounds::default();
};
sk_mesh.get_bounds()
});
let model_part = Arc::new(ModelPart {
id: *part.get_id(),
path: part_path,
space,
model: Arc::downgrade(model),
pending_material_parameters: Mutex::new(FxHashMap::default()),
pending_material_replacement: Mutex::new(None),
aliases: AliasList::default(),
material: Mutex::new(part.get_material().map(MaterialWrapper).map(Arc::new)),
});
node.add_aspect_raw(model_part.clone());
parts.push(model_part.clone());
Some(model_part)
}
pub fn replace_material(&self, replacement: Arc<MaterialWrapper>) {
let shared_material = MATERIAL_REGISTRY.add_or_get(replacement);
pub fn replace_material(&self, replacement: Material) {
self.pending_material_replacement
.lock()
.replace(shared_material);
}
/// only to be run on the main thread
pub fn replace_material_now(&self, replacement: &Material) {
let Some(model) = self.model.upgrade() else {
return;
};
let Some(sk_model) = model.sk_model.get() else {
return;
};
let nodes = sk_model.get_nodes();
let Some(mut part) = nodes.get_index(self.id) else {
return;
};
let shared_material =
MATERIAL_REGISTRY.add_or_get(Arc::new(MaterialWrapper(replacement.copy())));
let mut lock = self.material.lock();
part.material(&shared_material.0);
lock.replace(shared_material);
}
fn update(&self) {
let Some(model) = self.model.upgrade() else {
return;
};
let Some(sk_model) = model.sk_model.get() else {
return;
};
let Some(node) = model.space.node() else {
return;
};
let nodes = sk_model.get_nodes();
let Some(mut part) = nodes.get_index(self.id) else {
return;
};
part.model_transform(Spatial::space_to_space_matrix(
Some(&self.space),
Some(&model.space),
));
let Some(client) = node.get_client() else {
return;
};
if let Some(material_replacement) = self.pending_material_replacement.lock().take() {
let mut lock = self.material.lock();
part.material(&material_replacement.0);
lock.replace(material_replacement);
}
'mat_params: {
let mut material_parameters = self.pending_material_parameters.lock();
if !material_parameters.is_empty() {
let Some(material) = part.get_material() else {
break 'mat_params;
};
let new_material = material.copy();
for (parameter_name, parameter_value) in material_parameters.drain() {
parameter_value.apply_to_material(&client, &new_material, &parameter_name);
}
let shared_material =
MATERIAL_REGISTRY.add_or_get(Arc::new(MaterialWrapper(new_material)));
let mut lock = self.material.lock();
part.material(&shared_material.0);
lock.replace(shared_material);
}
}
.replace(replacement);
}
}
impl ModelPartAspect for ModelPart {
#[doc = "Set this model part's material to one that cuts a hole in the world. Often used for overlays/passthrough where you want to show the background through an object."]
fn apply_holdout_material(node: Arc<Node>, _calling_client: Arc<Client>) -> Result<()> {
let model_part = node.get_aspect::<ModelPart>()?;
model_part.replace_material(HOLDOUT_MATERIAL.get().unwrap().clone());
model_part.replace_material(Material {
color: Color::BLACK.with_alpha(0.0),
emission_factor: Color::BLACK,
metallic: 0.0,
roughness: 1.0,
alpha_mode: AlphaMode::Opaque,
double_sided: false,
diffuse_texture: None,
emission_texture: None,
metal_texture: None,
occlusion_texture: None,
});
Ok(())
}
@@ -328,12 +473,37 @@ impl ModelPartAspect for ModelPart {
Ok(())
}
}
#[derive(Default)]
struct MaterialRegistry(FxHashMap<HashedPbrMaterial, Handle<PbrMaterial>>);
impl MaterialRegistry {
/// returns strong handle for PbrMaterial elminitating duplications
fn get_handle(
&mut self,
material: PbrMaterial,
materials: &mut ResMut<Assets<PbrMaterial>>,
) -> Handle<PbrMaterial> {
let hash = HashedPbrMaterial::new(&material);
match self
.0
.get(&hash)
.and_then(|v| materials.get_strong_handle(v.id()))
{
Some(v) => v,
None => {
let handle = materials.add(material);
self.0.insert(hash, handle.clone_weak());
handle
}
}
}
}
pub struct Model {
space: Arc<Spatial>,
_resource_id: ResourceID,
sk_model: OnceLock<SKModel>,
parts: Mutex<Vec<Arc<ModelPart>>>,
bevy_scene_entity: OnceLock<Entity>,
parts: OnceLock<Vec<Arc<ModelPart>>>,
pre_bound_parts: Mutex<Vec<Arc<ModelPart>>>,
}
impl Model {
pub fn add_to(node: &Arc<Node>, resource_id: ResourceID) -> Result<Arc<Model>> {
@@ -347,42 +517,21 @@ impl Model {
let model = Arc::new(Model {
space: node.get_aspect::<Spatial>().unwrap().clone(),
_resource_id: resource_id,
sk_model: OnceLock::new(),
parts: Mutex::new(Vec::default()),
bevy_scene_entity: OnceLock::new(),
pre_bound_parts: Mutex::default(),
parts: OnceLock::new(),
});
LOAD_MODEL
.get()
.unwrap()
.send((model.clone(), pending_model_path))
.unwrap();
MODEL_REGISTRY.add_raw(&model);
// technically doing this in anything but the main thread isn't a good idea but dangit we need those model nodes ASAP
let sk_model = SKModel::copy(SKModel::from_file(
pending_model_path.to_str().unwrap(),
None,
)?);
ModelPart::create_for_model(&model, &sk_model);
let _ = model.sk_model.set(sk_model);
node.add_aspect_raw(model.clone());
Ok(model)
}
fn draw(&self, token: &MainThreadToken) {
let Some(sk_model) = self.sk_model.get() else {
return;
};
let parts = self.parts.lock();
for model_node in &*parts {
model_node.update();
}
drop(parts);
if let Some(node) = self.space.node() {
if node.enabled() {
sk_model.draw(token, self.space.global_transform(), None, None);
}
}
}
}
// TODO: proper hread safety in stereokit_rust (probably just bind stereokit directly)
unsafe impl Send for Model {}
unsafe impl Sync for Model {}
impl ModelAspect for Model {
#[doc = "Bind a model part to the node with the ID input."]
fn bind_model_part(
@@ -392,10 +541,44 @@ impl ModelAspect for Model {
part_path: String,
) -> Result<()> {
let model = node.get_aspect::<Model>()?;
let parts = model.parts.lock();
let Some(part) = parts.iter().find(|p| p.path == part_path) else {
let paths = parts.iter().map(|p| &p.path).collect::<Vec<_>>();
bail!("Couldn't find model part at path {part_path}, all available paths: {paths:?}",);
let part = match model
.parts
.get()
.map(|v| v.iter().find(|p| p.path == part_path))
{
Some(Some(part)) => part.clone(),
Some(None) => {
let paths = model
.parts
.get()
.unwrap()
.iter()
.map(|p| &p.path)
.collect::<Vec<_>>();
bail!(
"Couldn't find model part at path {part_path}, all available paths: {paths:?}",
);
}
None => {
let part_node = calling_client
.scenegraph
.add_node(Node::generate(&calling_client, false));
let model = node.get_aspect::<Model>()?;
let spatial =
Spatial::add_to(&part_node, Some(model.space.clone()), Mat4::IDENTITY, false);
let part = part_node.add_aspect(ModelPart {
entity: OnceLock::new(),
mesh_entity: OnceLock::new(),
path: part_path,
space: spatial,
model: Arc::downgrade(&model),
pending_material_parameters: Mutex::default(),
pending_material_replacement: Mutex::default(),
aliases: AliasList::default(),
});
model.pre_bound_parts.lock().push(part.clone());
part
}
};
Alias::create_with_id(
&part.space.node().unwrap(),
@@ -412,9 +595,3 @@ impl Drop for Model {
MODEL_REGISTRY.remove(self);
}
}
pub fn draw_all(token: &MainThreadToken) {
for model in MODEL_REGISTRY.get_valid_contents() {
model.draw(token);
}
}

View File

@@ -9,10 +9,7 @@ use crate::{
core::{client::Client, registry::Registry, scenegraph::MethodResponseSender},
nodes::{
Message, Node,
drawable::{
model::{MaterialWrapper, ModelPart},
shaders::UNLIT_SHADER_BYTES,
},
drawable::{model::ModelPart, shaders::UNLIT_SHADER_BYTES},
items::TypeInfo,
spatial::{Spatial, Transform},
},
@@ -68,7 +65,6 @@ pub struct CameraItem {
space: Arc<Spatial>,
frame_info: Mutex<FrameInfo>,
sk_tex: OnceLock<TexWrapper>,
sk_mat: OnceLock<Arc<MaterialWrapper>>,
applied_to: Registry<ModelPart>,
apply_to: Registry<ModelPart>,
}
@@ -82,7 +78,6 @@ impl CameraItem {
px_size,
}),
sk_tex: OnceLock::new(),
sk_mat: OnceLock::new(),
applied_to: Registry::new(),
apply_to: Registry::new(),
});
@@ -139,15 +134,15 @@ impl CameraItem {
TexFormat::RGBA32Linear,
))
});
let sk_mat = self.sk_mat.get_or_init(|| {
let shader = Shader::from_memory(UNLIT_SHADER_BYTES).unwrap();
let mut mat = Material::new(&shader, None);
mat.get_all_param_info().set_texture("diffuse", &sk_tex.0);
mat.transparency(Transparency::Blend);
Arc::new(MaterialWrapper(mat))
});
// let sk_mat = self.sk_mat.get_or_init(|| {
// let shader = Shader::from_memory(UNLIT_SHADER_BYTES).unwrap();
// let mut mat = Material::new(&shader, None);
// mat.get_all_param_info().set_texture("diffuse", &sk_tex.0);
// mat.transparency(Transparency::Blend);
// Arc::new(MaterialWrapper(mat))
// });
for model_part in self.apply_to.take_valid_contents() {
model_part.replace_material(sk_mat.clone())
// model_part.replace_material(sk_mat.clone())
}
if !self.applied_to.is_empty() {

View File

@@ -9,16 +9,85 @@ use crate::core::client::Client;
use crate::core::error::Result;
use crate::core::registry::Registry;
use crate::nodes::{Node, OWNED_ASPECT_ALIAS_INFO};
use bevy::ecs::entity_disabling::Disabled;
use bevy::prelude::Transform as BevyTransform;
use bevy::prelude::*;
use color_eyre::eyre::OptionExt;
use glam::{Mat4, Quat, Vec3, vec3a};
use mint::Vector3;
use parking_lot::Mutex;
use rustc_hash::FxHashMap;
use std::fmt::Debug;
coderabbitai[bot] commented 2025-06-28 14:53:15 -04:00 (Migrated from github.com)
Review

🛠️ Refactor suggestion

Consider implementing entity cleanup for dead spatial references.

The comment raises a valid concern. When the spatial weak reference can't be upgraded, the entity is orphaned and should be despawned to prevent resource leaks.

 let Some(spatial) = spatial_node.0.upgrade() else {
-    // should we despawn the entity?
+    cmds.entity(entity).despawn_recursive();
     return;
 };

Note: This would require adding Commands to the system parameters.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/nodes/spatial/mod.rs around lines 41 to 44, the code currently returns
early when the spatial weak reference cannot be upgraded, potentially leaving
orphaned entities. To fix this, modify the system to accept Commands as a
parameter and use it to despawn the entity when the spatial reference is dead,
ensuring proper cleanup and preventing resource leaks.
_🛠️ Refactor suggestion_ **Consider implementing entity cleanup for dead spatial references.** The comment raises a valid concern. When the spatial weak reference can't be upgraded, the entity is orphaned and should be despawned to prevent resource leaks. ```diff let Some(spatial) = spatial_node.0.upgrade() else { - // should we despawn the entity? + cmds.entity(entity).despawn_recursive(); return; }; ``` Note: This would require adding `Commands` to the system parameters. > Committable suggestion skipped: line range outside the PR's diff. <details> <summary>🤖 Prompt for AI Agents</summary> ``` In src/nodes/spatial/mod.rs around lines 41 to 44, the code currently returns early when the spatial weak reference cannot be upgraded, potentially leaving orphaned entities. To fix this, modify the system to accept Commands as a parameter and use it to despawn the entity when the spatial reference is dead, ensuring proper cleanup and preventing resource leaks. ``` </details> <!-- This is an auto-generated comment by CodeRabbit --> <!-- fingerprinting:phantom:medusa:lion -->
use std::sync::atomic::Ordering;
use std::sync::{Arc, OnceLock, Weak};
use std::{f32, ptr};
use stereokit_rust::maths::Bounds;
pub struct SpatialNodePlugin;
impl Plugin for SpatialNodePlugin {
fn build(&self, app: &mut App) {
app.add_systems(
PostUpdate,
(replace_childof, update_spatial_nodes).before(TransformSystem::TransformPropagate),
);
}
}
fn update_spatial_nodes(
mut query: Query<(Entity, &mut BevyTransform, &SpatialNode, Has<Disabled>)>,
cmds: ParallelCommands,
) {
query
.par_iter_mut()
.for_each(|(entity, mut transform, spatial_node, disabled)| {
let Some(spatial) = spatial_node.0.upgrade() else {
return;
};
if !spatial
.node()
.map(|n| n.enabled.load(Ordering::Relaxed))
.unwrap_or(true)
{
// cmds.command_scope(|mut cmds| {
// cmds.entity(entity).insert(Visibility::Hidden);
// });
// return;
}
let mat4 = spatial.global_transform();
let scale_zero = mat4.to_scale_rotation_translation().0.length_squared() > 0.0;
if scale_zero {
// cmds.command_scope(|mut cmds| {
// cmds.entity(entity).insert(Visibility::Hidden);
// });
} else if disabled {
// cmds.command_scope(|mut cmds| {
// cmds.entity(entity).insert(Visibility::Visible);
// });
}
*transform = BevyTransform::from_matrix(mat4);
});
}
fn replace_childof(query: Query<(Entity, &ChildOf), With<SpatialNode>>, mut cmds: Commands) {
for (entity, parent) in &query {
cmds.entity(entity)
.insert(NonSpatialChildOf(parent.0))
.remove::<ChildOf>();
}
}
#[derive(Component, Default, Debug, PartialEq, Eq)]
#[relationship_target(relationship = NonSpatialChildOf, linked_spawn)]
pub struct NonSpatialChildren(Vec<Entity>);
#[derive(Component, Debug, PartialEq, Eq)]
#[relationship(relationship_target = NonSpatialChildren)]
pub struct NonSpatialChildOf(Entity);
#[derive(Clone, Component, Debug)]
#[require(BevyTransform)]
pub struct SpatialNode(pub Weak<Spatial>);
stardust_xr_server_codegen::codegen_spatial_protocol!();
impl Transform {
pub fn to_mat4(&self, position: bool, rotation: bool, scale: bool) -> Mat4 {