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

Bevy 0.10 #18

Merged
merged 23 commits into from
Apr 30, 2023
Merged

Bevy 0.10 #18

merged 23 commits into from
Apr 30, 2023

Conversation

naomijub
Copy link
Contributor

@naomijub naomijub commented Mar 29, 2023

Remaining stuff

  • fix system ordering
  • credit update migration authors

@naomijub naomijub changed the title Bevy 0.10 DRAFT: Bevy 0.10 Mar 29, 2023
@naomijub
Copy link
Contributor Author

bevyengine/bevy#8257

@naomijub naomijub closed this Mar 29, 2023
@naomijub
Copy link
Contributor Author

naomijub commented Mar 31, 2023

@Game4all I need your help here. There is something I missed or did wrong when configuring the stages. It seems that when it reaches the render stage, something is missing from the app, a resource of something. I presume it might have to do with where ChunkLoadingStage is located

@Game4all
Copy link
Owner

Game4all commented Apr 1, 2023

I'll check out your branch later, but the errors you're running into are kind of weird for sure

@naomijub
Copy link
Contributor Author

naomijub commented Apr 1, 2023

Thanks

@Game4all
Copy link
Owner

Game4all commented Apr 1, 2023

started investigating late night, pipelined rendering doesn 't seem to play nice with extract<> resources. Managed to get stuff running without crashing (no chunks were rendered though) with the following patch:

diff --git a/src/voxel/render/terrain_uniforms.rs b/src/voxel/render/terrain_uniforms.rs
index dd0cb1f..2131718 100644
--- a/src/voxel/render/terrain_uniforms.rs
+++ b/src/voxel/render/terrain_uniforms.rs
@@ -217,11 +217,13 @@ impl Plugin for VoxelTerrainUniformsPlugin {
     fn build(&self, app: &mut bevy::prelude::App) {
         app.sub_app_mut(RenderApp)
             .init_resource::<TerrainUniforms>()
-            .add_system(extract_voxel_materials.in_set(RenderSet::ExtractCommands))
+            .init_resource::<GpuTerrainMaterials>()
+            .init_resource::<GpuTerrainRenderSettings>()
+            //.add_system(extract_voxel_materials.in_set(RenderSet::ExtractCommands))
             .add_system(prepare_terrain_uniforms.in_set(RenderSet::Queue))
             .add_system(upload_voxel_materials.in_set(RenderSet::Prepare))
-            .add_system(upload_render_distance_uniform.in_set(RenderSet::Prepare))
-            .add_system(extract_terrain_render_settings_uniform.in_set(RenderSet::ExtractCommands));
+            .add_system(upload_render_distance_uniform.in_set(RenderSet::Prepare));
+            // .add_system(extract_terrain_render_settings_uniform.in_set(RenderSet::ExtractCommands));
 
         info!("type size: {}", GpuVoxelMaterial::min_size().get() * 256);
     }

Need to test if this still crashes with pipelining disabled.
UPDATE: still crashes with the pipelined rendering plugin disabled; your initial guess was probably right abt something missing in the main app it seems.

@naomijub
Copy link
Contributor Author

naomijub commented Apr 2, 2023

Thanks, will take a look as well

@naomijub
Copy link
Contributor Author

naomijub commented Apr 3, 2023

@Game4all I will try to "librarize/cratify" the code so we can do this migration easier ok? Probably addressing this issue #12 in the way.

It is a really good project and would be awesome to have it as a bevy plugin, but I will first do it in bevy version 0.9 as it is stable and working

@Game4all
Copy link
Owner

Game4all commented Apr 3, 2023

@naomijub wow, that's very cool to hear! But since this is quite a colossal task, I would like to assist you in doing so. Before you start migrating anything, could you please add me on discord (Game4all#0727) or reach me through the mail displayed on my github profile so we can discuss the way to go with this before potentially wasting any effort? Thanks.

@naomijub
Copy link
Contributor Author

naomijub commented Apr 14, 2023

@Game4all I fixed the crash but Now I'm getting another error, shader error. But I'm not that good with wgsl. Maybe you can help

2023-04-14T17:13:30.211689Z ERROR bevy_render::render_resource::pipeline_cache: failed to create shader module: Validation Error

Caused by:
    In Device::create_shader_module

Shader '' parsing error: expected expression, found '#'
   ┌─ wgsl:70:41
   │
70 │     cascades: array<DirectionalCascade, #{MAX_CASCADES_PER_LIGHT}>,
   │                                         ^ expected expression


    expected expression, found '#'

Commenting this line (// .init_resource::<GpuTerrainRenderSettings>()) seems to avoid the crash, but then nothing renders:

impl Plugin for VoxelTerrainUniformsPlugin {
    fn build(&self, app: &mut bevy::prelude::App) {
        app.sub_app_mut(RenderApp)
            .init_resource::<TerrainUniforms>()
            .init_resource::<GpuTerrainMaterials>()
            // .init_resource::<GpuTerrainRenderSettings>()
            .add_system(extract_voxel_materials.in_set(RenderSet::ExtractCommands).in_schedule(ExtractSchedule))
            .add_system(prepare_terrain_uniforms.in_set(RenderSet::Queue))
            .add_system(upload_voxel_materials.in_set(RenderSet::Prepare))
            .add_system(extract_terrain_render_settings_uniform.in_set(RenderSet::ExtractCommands).in_schedule(ExtractSchedule))
            .add_system(upload_render_distance_uniform.in_set(RenderSet::Prepare));

        info!("type size: {}", GpuVoxelMaterial::min_size().get() * 256);
    }
}

@Game4all
Copy link
Owner

looking into this

@hans-pistor
Copy link
Contributor

You need to add some shader_defs and some additional imports

// shader file
#import bevy_pbr::fog
#import bevy_pbr::pbr_ambient
// specialized pipeline file
        let shader_defs = vec![ShaderDefVal::Int(
            "MAX_CASCADES_PER_LIGHT".into(),
            bevy::pbr::MAX_CASCADES_PER_LIGHT as i32,
        ),
        ShaderDefVal::Int(
            "MAX_DIRECTIONAL_LIGHTS".into(),
            bevy::pbr::MAX_DIRECTIONAL_LIGHTS as i32
        )];

        let vertex = VertexState {
            shader: self.shader.clone(),
            shader_defs: shader_defs.clone(),
            entry_point: "vertex".into(),
            buffers: vec![vertex_buffer],
        };
        let fragment = FragmentState {
            shader: self.shader.clone(),
            shader_defs,
            entry_point: "fragment".into(),
            targets: vec![Some(ColorTargetState {
                format: TextureFormat::bevy_default(),
                blend: Some(BlendState::REPLACE),
                write_mask: ColorWrites::ALL,
            })],
        };

@hans-pistor
Copy link
Contributor

Did some more digging, you probably want to specialize the mesh pipeline which will add all these shader defs for you as well as populating known vertex data

    fn specialize(
        &self,
        key: Self::Key,
        layout: &bevy::render::mesh::MeshVertexBufferLayout,
    ) -> Result<
        bevy::render::render_resource::RenderPipelineDescriptor,
        bevy::render::render_resource::SpecializedMeshPipelineError,
    > {
        let mut descriptor = self.mesh_pipeline.specialize(key, layout)?;
        descriptor.vertex.shader = self.shader.clone();
        descriptor.fragment.as_mut().expect("Failed to get fragment shader from mesh pipeline").shader = self.shader.clone();
        Ok(descriptor)

This issue is relevant: bevyengine/bevy#7285

@naomijub
Copy link
Contributor Author

Thanks a lot @hans-pistor I will try that

@naomijub
Copy link
Contributor Author

let mut descriptor = self.mesh_pipeline.specialize(key, layout)?;
descriptor.vertex.shader = self.shader.clone();
descriptor.fragment.as_mut().expect("Failed to get fragment shader from mesh pipeline").shader = self.shader.clone();
Ok(descriptor)

@hans-pistor by doing this change I get:

2023-04-20T01:50:36.092280Z ERROR bevy_render::render_resource::pipeline_cache: failed to create shader module: Validation Error

Caused by:
    In Device::create_shader_module

Shader '' parsing error: expected global item ('struct', 'const', 'var', 'type', ';', 'fn') or the end of the file, found 'let'
    ┌─ wgsl:313:1
    │
313 │ let VOXEL_MAT_FLAG_LIQUID: u32 = 2u; // 1 << 1
    │ ^^^ expected global item ('struct', 'const', 'var', 'type', ';', 'fn') or the end of the file


    expected global item ('struct', 'const', 'var', 'type', ';', 'fn') or the end of the file, found 'let'

@hans-pistor
Copy link
Contributor

Did you read the error message? It tells you what it's expecting vs what it got

@naomijub
Copy link
Contributor Author

I'm far from specialist in shaders, but it doesnt seem to make much sense:
The shader:

let VOXEL_MAT_FLAG_LIQUID: u32 = 2u; // 1 << 1
let TERRAIN_CHUNK_LENGTH: u32 = 32u;

struct VoxelMat {
    base_color: vec4<f32>,
    flags: u32,
    emissive: vec4<f32>,
    perceptual_roughness: f32,
    metallic: f32,
    reflectance: f32,
};

// A GPU-suited representation of voxel materials.
struct VoxelMaterials {
    materials: array<VoxelMat>
};

struct TerrainRenderSettings {
    render_distance: u32,
};

@group(2) @binding(0)
var<storage> VOXEL_MATERIALS: VoxelMaterials;

@group(2)  @binding(1)
var<storage> terrain_settings: TerrainRenderSettings;

// Returns computed fragment color from the current ambient light + diffuse per face lighting
fn calc_voxel_lighting(col: vec3<f32>, n: vec3<f32>) -> vec3<f32> {
    let per_face_light = vec3<f32>(0.8, 1.0, 0.6);
    let normal = abs(dot(n, vec3<f32>(1., 0., 0.)) * per_face_light.x) 
               + abs(dot(n, vec3<f32>(0., 1., 0.)) * per_face_light.y) 
               + abs(dot(n, vec3<f32>(0., 0., 1.)) * per_face_light.z);

    return normal * col + lights.ambient_color.xyz * 0.21;
}

@hans-pistor
Copy link
Contributor

Replace the global lets with const

@naomijub
Copy link
Contributor Author

Weird it is not working well on my windows 🤔

@Game4all
Copy link
Owner

Seems there's definitely a problem somewhere as 0 meshes get extracted to the render world hence why nothing gets rendered, as exposed by this patch:

diff --git a/src/voxel/render/pipeline.rs b/src/voxel/render/pipeline.rs
index 7d73908..0ac8746 100644
--- a/src/voxel/render/pipeline.rs
+++ b/src/voxel/render/pipeline.rs
@@ -95,6 +95,7 @@ fn queue_voxel_meshes(
 ) {
     let draw_custom = oq_draw_funcs.read().get_id::<DrawVoxel>().unwrap();
     let key = MeshPipelineKey::from_msaa_samples(msaa.samples());
+    println!("{} queueable voxel meshes", material_meshes.iter().len());
     for (view, mut transparent_phase) in views.iter_mut() {
         let view_matrix = view.transform.compute_matrix();
         let view_row_2 = view_matrix.row(2);

This looks like some system ordering problem??? Will dig deeper and hopefully find the root cause of this.

@hans-pistor
Copy link
Contributor

hans-pistor commented Apr 23, 2023

Oh you know what. This is likely related to Frustrum culling. Let me pull your latest commit & confirm that

@hans-pistor
Copy link
Contributor

hans-pistor commented Apr 23, 2023

@Game4all Can you try removing the Aabb on the VoxelTerrainMeshBundle? The Mesh will calculate it's own (from it's vertices) if there isn't an Aabb component present on the entity.

The camera uses the Aabb to determine if points are within the frustrum

@hans-pistor
Copy link
Contributor

Diff

diff --git a/src/voxel/render/pipeline.rs b/src/voxel/render/pipeline.rs
index 7d73908..91f13b4 100644
--- a/src/voxel/render/pipeline.rs
+++ b/src/voxel/render/pipeline.rs
@@ -95,6 +95,7 @@ fn queue_voxel_meshes(
 ) {
     let draw_custom = oq_draw_funcs.read().get_id::<DrawVoxel>().unwrap();
     let key = MeshPipelineKey::from_msaa_samples(msaa.samples());
+    println!("Meshes queued: {}", material_meshes.iter().len());
     for (view, mut transparent_phase) in views.iter_mut() {
         let view_matrix = view.transform.compute_matrix();
         let view_row_2 = view_matrix.row(2);
@@ -134,7 +135,7 @@ pub struct VoxelTerrainMeshBundle {
     pub global_transform: GlobalTransform,
     pub visibility: Visibility,
     pub computed_visibility: ComputedVisibility,
-    pub aabb: Aabb,
+    // pub aabb: Aabb,
 }
 pub struct VoxelMeshRenderPipelinePlugin;
 
diff --git a/src/voxel/world/meshing.rs b/src/voxel/world/meshing.rs
index c47f3e5..4386a25 100644
--- a/src/voxel/world/meshing.rs
+++ b/src/voxel/world/meshing.rs
@@ -28,7 +28,7 @@ pub fn prepare_chunks(
             mesh: meshes.add(Mesh::new(PrimitiveTopology::TriangleList)),
             transform: Transform::from_translation(chunk_key.0.as_vec3()),
             visibility: Visibility::Hidden,
-            aabb: Aabb::from_min_max(Vec3::ZERO, Vec3::splat(CHUNK_LENGTH as f32)),
+            // aabb: Aabb::from_min_max(Vec3::ZERO, Vec3::splat(CHUNK_LENGTH as f32)),
             ..Default::default()
         });
     }

Output:

Meshes queued: 15
Meshes queued: 25
Meshes queued: 200
Meshes queued: 287
Meshes queued: 323
Meshes queued: 486
Meshes queued: 658
Meshes queued: 847
Meshes queued: 1027
Meshes queued: 1172

@hans-pistor
Copy link
Contributor

Weird, sometimes I get the world rendered & sometimes there's nothing. Sometimes when the world does render it's super buggy

image

@hans-pistor
Copy link
Contributor

hans-pistor commented Apr 23, 2023

Consistent rendering naomijub/vx_bevy@bevy-0.10...hans-pistor:vx_bevy:fix-schedule-ambiguties

(frustrum culling wasn't the root issue)

I left in all the debug code, @Game4all @naomijub can one of you productionalize this & I think that should be the final "big" blocker for 0.10? For one, I am manually scheduling via systems but we should probably have better scheduling via the Sets

@naomijub naomijub changed the title DRAFT: Bevy 0.10 Bevy 0.10 Apr 25, 2023
@naomijub
Copy link
Contributor Author

@Game4all This branch is working perfectly from my side with @hans-pistor changes

@naomijub naomijub mentioned this pull request Apr 25, 2023
@@ -12,8 +12,8 @@ mod debug;
mod voxel;

fn main() {
App::default()
.add_plugins(DefaultPlugins)
let mut app = App::default();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hans-pistor : "I left this in to demonstrate how to find scheduling ambiguties."

// app.edit_schedule(CoreSchedule::Main, |schedule| {
  //     schedule.set_build_settings(ScheduleBuildSettings {
  //         ambiguity_detection: bevy::ecs::schedule::LogLevel::Error,
  //         ..Default::default()
  //     });
  // });

@@ -39,7 +39,7 @@ static SHARED_MESH_BUFFERS: Lazy<ThreadLocal<RefCell<MeshBuffers<Voxel, ChunkSha
Lazy::new(ThreadLocal::default);

/// Queues meshing tasks for the chunks in need of a remesh.
fn queue_mesh_tasks(
pub fn queue_mesh_tasks(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hans-pistor: "Probably not right to keep these as pub. Instead we should have more strict ordering within the different Sets"

@Game4all
Copy link
Owner

Will look at this later in the week

@naomijub
Copy link
Contributor Author

naomijub commented Apr 26, 2023

I also noticed a significant fps drop, maybe LODs could improve this

@meyerzinn
Copy link
Contributor

meyerzinn commented Apr 29, 2023

Can't believe I didn't see this PR before embarking upon my own epic quest... ugh. Well, feel free to take anything you want from the other one.

Only major difference is that I simplified the pipeline by using a custom material for voxels, so we can get actual shadows using the default prepasses.

@naomijub
Copy link
Contributor Author

@meyerzinn do you want to merge your changes in this one? Maybe create a pr in my fork?

@meyerzinn
Copy link
Contributor

I can try rebasing on this PR.

@Game4all
Copy link
Owner

Linked PR got some improvements I would love to get merged in after finishing migrating this. Also system scheduling looks a bit better over there so I may cherry pick on top of the existing branch.

@Game4all
Copy link
Owner

I've reverted the changes to worldgen for now. Let's get things rolling

@Game4all Game4all merged commit 3e59c76 into Game4all:master Apr 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants