Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wireframe: panic when enabling wireframes after startup #1609

Closed
jakobhellermann opened this issue Mar 10, 2021 · 3 comments
Closed

Wireframe: panic when enabling wireframes after startup #1609

jakobhellermann opened this issue Mar 10, 2021 · 3 comments
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior P-Crash A sudden unexpected crash

Comments

@jakobhellermann
Copy link
Contributor

When the WireframeConfig::global setting is false at the beginning and is later changed to true, bevy panics with thread 'main' panicked at 'called Option::unwrap() on a None value', crates/bevy_render/src/pipeline/pipeline_compiler.rs:137:64

let source_descriptor = pipelines.get(source_pipeline).unwrap();

The same happens if a Wireframe component is added to an entity later on.

To reproduce it you can take the wireframe example and add a system which enables global wireframes after startup

app.add_system(delayed_wireframe.system()
fn delayed_wireframe(
    mut wireframe_config: ResMut<WireframeConfig>,
    time: Res<Time>,
    mut query: Query<&mut Timer>,
) {
    let mut timer = query.single_mut().unwrap();
    if timer.tick(time.delta()).just_finished() {
        commands.insert(cube, Wireframe);
    }
}
 fn setup(
    mut commands: Commands,
    mut wireframe_config: ResMut<WireframeConfig>,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<StandardMaterial>>,
) {
+    commands.spawn((Timer::from_seconds(1.0, false),));
     // ...
@bjorn3 bjorn3 added C-Bug An unexpected or incorrect behavior P-Crash A sudden unexpected crash A-Rendering Drawing game state to the screen labels Mar 10, 2021
@jakobhellermann
Copy link
Contributor Author

Relevant code:

draw_context
.set_pipeline(
&mut draw,
&render_pipeline.pipeline,
&render_pipeline.specialization,
)
.unwrap();

@simgt
Copy link

simgt commented Mar 11, 2021

It seems to be due to the initialisation of the pipeline in the Plugin::build method:

let world = app.world_mut().cell();
let mut shaders = world.get_resource_mut::<Assets<Shader>>().unwrap();
let mut pipelines = world
.get_resource_mut::<Assets<PipelineDescriptor>>()
.unwrap();
pipelines.set(
WIREFRAME_PIPELINE_HANDLE,
pipeline::build_wireframe_pipeline(&mut shaders),
);

The code bellow solves the issue for me:

diff --git a/crates/bevy_render/src/wireframe/mod.rs b/crates/bevy_render/src/wireframe/mod.rs
index 4251fb44..4ff46866 100644
--- a/crates/bevy_render/src/wireframe/mod.rs
+++ b/crates/bevy_render/src/wireframe/mod.rs
@@ -10,7 +10,7 @@ use bevy_asset::{Assets, Handle, HandleUntyped};
 use bevy_ecs::{
     query::With,
     reflect::ReflectComponent,
-    system::{IntoSystem, Query, QuerySet, Res},
+    system::{IntoSystem, Query, QuerySet, Res, ResMut},
     world::Mut,
 };
 use bevy_reflect::{Reflect, TypeUuid};
@@ -27,19 +27,21 @@ pub struct WireframePlugin;
 impl Plugin for WireframePlugin {
     fn build(&self, app: &mut AppBuilder) {
         app.init_resource::<WireframeConfig>()
+            .add_system_to_stage(crate::CoreStage::PreUpdate, setup_pipeline_system.system())
             .add_system_to_stage(crate::RenderStage::Draw, draw_wireframes_system.system());
-        let world = app.world_mut().cell();
-        let mut shaders = world.get_resource_mut::<Assets<Shader>>().unwrap();
-        let mut pipelines = world
-            .get_resource_mut::<Assets<PipelineDescriptor>>()
-            .unwrap();
-        pipelines.set(
-            WIREFRAME_PIPELINE_HANDLE,
-            pipeline::build_wireframe_pipeline(&mut shaders),
-        );
     }
 }
 
+pub fn setup_pipeline_system(
+    mut shaders: ResMut<Assets<Shader>>,
+    mut pipelines: ResMut<Assets<PipelineDescriptor>>
+) {
+    pipelines.set(
+        WIREFRAME_PIPELINE_HANDLE,
+        pipeline::build_wireframe_pipeline(&mut shaders),
+    );
+}
+

...but rebuilding/resetting the pipeline before each update is likely not what we want. So fixing it will likely require a different path, which I won't take since I have absolutely no idea what I'm doing. 🙄

Maybe @Neo-Zhixing has some idea?

@simgt
Copy link

simgt commented Mar 11, 2021

Still messing with things I don't understand, that solves the issue:

diff --git a/crates/bevy_render/src/wireframe/mod.rs b/crates/bevy_render/src/wireframe/mod.rs
index 4251fb44..b2ad852a 100644
--- a/crates/bevy_render/src/wireframe/mod.rs
+++ b/crates/bevy_render/src/wireframe/mod.rs
@@ -33,7 +33,7 @@ impl Plugin for WireframePlugin {
         let mut pipelines = world
             .get_resource_mut::<Assets<PipelineDescriptor>>()
             .unwrap();
-        pipelines.set(
+        pipelines.set_untracked(
             WIREFRAME_PIPELINE_HANDLE,
             pipeline::build_wireframe_pipeline(&mut shaders),
         );

I guess the problem is that because the Handle returned by Assets<PipelineDescriptor>::set() is "Strong", the asset gets freed, somehow? Or something.

@bors bors bot closed this as completed in 785aad9 Mar 12, 2021
bors bot pushed a commit that referenced this issue Mar 14, 2021
As mentioned in #1609.

I'm not sure if this is desirable, but on top of factoring the `set` and `set_untracked` methods I added a warning when the return value of `set` isn't used to mitigate similar issues.

I silenced it for the only occurence where it's currently done  https://github.com/bevyengine/bevy/blob/68606934e32ab45828c628e1cefd3873273f8708/crates/bevy_asset/src/asset_server.rs#L468
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior P-Crash A sudden unexpected crash
Projects
None yet
Development

No branches or pull requests

3 participants