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

Query::par_for_each_mut() executing its closure multiple times for the same entity each run? #2247

Closed
forbjok opened this issue May 24, 2021 · 2 comments
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior

Comments

@forbjok
Copy link
Contributor

forbjok commented May 24, 2021

Bevy version

v0.5.0

Operating system & version

Windows 10 20H2

What you did

use bevy::{prelude::*, render::draw::OutsideFrustum, tasks::AsyncComputeTaskPool};

fn main() {
    App::build()
        .add_plugins(DefaultPlugins)
        .add_system(update_chunk_system.system())
        .add_system_to_stage(
            CoreStage::PostUpdate,
            remesh_chunk_system.system(),
        )
        .add_startup_system(setup.system())
        .run();
}

fn setup(mut commands: Commands) {
    // Spawn chunks
    for _ in 0..5 {
        commands
            .spawn()
            .insert(Chunk { needs_remesh: true });
    }
}

struct Chunk {
    pub needs_remesh: bool,
}

fn update_chunk_system(
    mut chunk_query: Query<&mut Chunk>,
) {
    for mut chunk in chunk_query.iter_mut() {
        chunk.needs_remesh = true;
    }
}

fn remesh_chunk_system(
    mut chunk_query: Query<&mut Chunk, Without<OutsideFrustum>>,
    task_pool: Res<AsyncComputeTaskPool>,
) {
    chunk_query.par_for_each_mut(&task_pool, 4, |mut chunk| {
        if !chunk.needs_remesh {
            // This code should NEVER run, since update_chunk_system is
            // setting needs_remesh back to true EVERY frame... BUT IT DOES. WHY?
            // Seems like this iteration may be somehow being run multiple times each frame, which it shouldn't.
            // This ONLY happens when:
            // * The number of chunks is higher than the batch size
            // * The Without<OutsideFrustum> query filter is present in the Query
            //   ... even more strangely, it ONLY seems to happen if the component being
            //       filtered on is OutsideFrustum. I tried replacing it with a custom
            //       component that would never be present on the entity, and the issue
            //       went away.
            dbg!("NOREM");
            return;
        }

        // REMESH HAPPENS HERE

        // Set needs_remesh back to false
        chunk.needs_remesh = false;
    });
}

What you expected to happen

chunk_query.par_for_each_mut() should execute its closure once, and only once per frame for each chunk entity, meaning that NOREM should never be printed, since needs_remesh is always being set back to true by the update_chunk_system.

What actually happened

NOREM gets printed constantly, presumably meaning that chunk_query.par_for_each_mut() is executing its closure multiple times on the same entity each frame because otherwise needs_remesh should always be true on every entity every time this system runs.

Additional information

This issue seems to happen only under very specific circumstances. The criteria I've been able to nail down so far are:

  • The number of entities being iterated MUST be higher than the batch size used
  • The Without<OutsideFrustum> query filter MUST be present. I tried replacing it with a custom component, and the issue was no longer happening. I have no idea why or how OutsideFrustum is special in this context, it just happened to be what I was checking for in the project I encountered the issue in, where I was using it to avoid remeshing chunks that are not being rendered.
@forbjok forbjok added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels May 24, 2021
@mockersf
Copy link
Member

It looks like you're hitting the same bug as #1943, fixed by #1945. Could you check with latest main from git if you can reproduce?

@TheRawMeatball TheRawMeatball added A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels May 24, 2021
@forbjok
Copy link
Contributor Author

forbjok commented May 24, 2021

Tested with the Bevy main branch now, and it does indeed appear to be fixed there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

No branches or pull requests

3 participants