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

Resizing camera target image causes Validation Error #5595

Closed
zaycev opened this issue Aug 6, 2022 · 8 comments
Closed

Resizing camera target image causes Validation Error #5595

zaycev opened this issue Aug 6, 2022 · 8 comments
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@zaycev
Copy link

zaycev commented Aug 6, 2022

Bevy version

0.8

[Optional] Relevant system information

2022-08-06T16:11:23.218862Z  INFO bevy_render::renderer: AdapterInfo { name: "Apple M1", vendor: 0, device: 0, device_type: IntegratedGpu, backend: Metal }

What you did

  1. Used post_processing as an example, made my main camera output to an Image target.
  2. Subscribed to window resize events.
  3. Updated camera target image descriptor size and size.

What went wrong

Panic due to validation error.

2022-08-06T16:14:40.407438Z ERROR wgpu::backend::direct: Handling wgpu errors as fatal by default    
thread 'main' panicked at 'wgpu error: Validation Error

Caused by:
    In a RenderPass
      note: encoder = `<CommandBuffer-(0, 8478, Metal)>`
    In a pass parameter
      note: command buffer = `<CommandBuffer-(0, 8478, Metal)>`
    attachments have differing sizes: ("color", Extent3d { width: 800, height: 800, depth_or_array_layers: 1 }) is followed by ("resolve", Extent3d { width: 798, height: 780, depth_or_array_layers: 1 })

Additional information

Resizing code that causes error:

fn update_target_on_resize(
    ....
    mut events:              EventReader<WindowResized>,
    mut query_camera: Query<(Entity, &Camera, &CameraVelocity, &Transform, &OrthographicProjection), With<CameraMain>>,
) {

    // Take the last resize event.
    let mut window_resized = None;
    for event in events.iter() {
        window_resized = Some(event);
    }

    if let Some(event) = window_resized {
        if let Ok(post_processing_material_handle) = query_handle.get_single() {
            if let Some(post_processing_material) = materials.get_mut(&post_processing_material_handle.handle) {
                if let Some(camera_target) = images.get_mut(&post_processing_material.source_image) {

                    // TODO: use physical dimensions.
                    let size = Extent3d{
                        width:  event.width  as u32,
                        height: event.height as u32,
                        ..default()
                    };
                    camera_target.texture_descriptor.size = size;
                    camera_target.resize(camera_target.texture_descriptor.size);

Workaround with despawning main camera:

fn update_target_on_resize(
    mut commands:     Commands,
    mut events:       EventReader<WindowResized>,
        query_handle: Query<&PostProcessingHandle>,
        config:       Res<MainCameraConfig>,
    mut materials:    ResMut<Assets<PostProcessingMaterial>>,
    mut images:       ResMut<Assets<Image>>,
    mut query_camera: Query<(Entity, &Camera, &CameraVelocity, &Transform, &OrthographicProjection), With<CameraMain>>,
) {

    // Take the last resize event.
    let mut window_resized = None;
    for event in events.iter() {
        window_resized = Some(event);
    }

    // Resize image for post-processing material and re-spawn main camera.
    if let Some(event) = window_resized {
        if let Ok(post_processing_material_handle) = query_handle.get_single() {
            if let Some(post_processing_material) = materials.get_mut(&post_processing_material_handle.handle) {
                if let Some(camera_target) = images.get_mut(&post_processing_material.source_image) {

                    // TODO: use physical dimensions.
                    let size = Extent3d{
                        width:  event.width  as u32,
                        height: event.height as u32,
                        ..default()
                    };
                    camera_target.texture_descriptor.size = size;
                    camera_target.resize(camera_target.texture_descriptor.size);

                    let (
                        old_main_camera_entity,
                        old_main_camera,
                        old_main_camera_velocity,
                        old_main_camera_transform,
                        old_main_camera_projection,
                    ) = query_camera.single_mut();
                    
                    commands.entity(old_main_camera_entity).despawn();
                    
                    let mut main_camera = Camera2dBundle {
                        camera_2d: Camera2d {
                            clear_color: CLEAR_COLOR,
                            ..default()
                        },
                        camera: Camera {
                            priority: old_main_camera.priority - 1,
                            target: RenderTarget::Image(post_processing_material.source_image.clone()),
                            ..default()
                        },
                        ..default()
                    };
                    
                    let resolution = event.width / event.height;
                    
                    main_camera.projection.top           =  1.0;
                    main_camera.projection.bottom        = -1.0;
                    main_camera.projection.right         =  1.0 * resolution;
                    main_camera.projection.left          = -1.0 * resolution;
                    main_camera.projection.scaling_mode  = ScalingMode::Auto {
                        min_width: 100.0,
                        min_height: 100.0,
                    };
                    main_camera.projection.scale         = old_main_camera_projection.scale;
                    main_camera.transform                = *old_main_camera_transform;
                    
                    commands
                        .spawn_bundle(main_camera)
                        .insert(CameraMain)
                        .insert(RenderLayers::from_layers(&[0, 1, 2, 3, 4, 5, 6]))
                        .insert(*old_main_camera_velocity)
                        .insert(config.movement.clone())
                        .insert(UiCameraConfig {
                            show_ui: false,
                            ..default()
                        });
                }

            }
        }
    }
}
@zaycev zaycev added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Aug 6, 2022
@DGriffin91
Copy link
Contributor

DGriffin91 commented Aug 6, 2022

@zaycev I've had this issue as well. I looked into it a bit, but didn't find exactly what the problem is. A workaround I have currently is to manually send a AssetEvent::Modified for the image handle when resizing, and that has worked at least in my case.

@Weibye Weibye added A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels Aug 7, 2022
@TethysSvensson
Copy link
Contributor

TethysSvensson commented Aug 15, 2022

Here is a minimal example that reproduces the problem on my machine:

use bevy::{
    prelude::*,
    render::{
        camera::RenderTarget,
        render_resource::{
            Extent3d, TextureDescriptor, TextureDimension, TextureFormat, TextureUsages,
        },
    },
};

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .insert_resource(FirstFrame(true))
        .add_startup_system(setup)
        .add_system(resize)
        .run()
}

struct FirstFrame(bool);
struct RenderTexture(Handle<Image>);

fn setup(mut commands: Commands, mut images: ResMut<Assets<Image>>) {
    let size = Extent3d {
        width: 512,
        height: 512,
        ..default()
    };

    let mut image = Image {
        texture_descriptor: TextureDescriptor {
            label: None,
            size,
            dimension: TextureDimension::D2,
            format: TextureFormat::Bgra8UnormSrgb,
            mip_level_count: 1,
            sample_count: 1,
            usage: TextureUsages::TEXTURE_BINDING
                | TextureUsages::COPY_DST
                | TextureUsages::RENDER_ATTACHMENT,
        },
        ..default()
    };

    image.resize(size);

    let image_handle = images.add(image);

    commands.spawn_bundle(Camera2dBundle {
        camera: Camera {
            target: RenderTarget::Image(image_handle.clone()),
            ..default()
        },
        ..default()
    });
    commands.insert_resource(RenderTexture(image_handle));
}

fn resize(
    mut first_frame: ResMut<FirstFrame>,
    texture_handle: Res<RenderTexture>,
    mut images: ResMut<Assets<Image>>,
) {
    if first_frame.0 {
        first_frame.0 = false;
    } else {
        if let Some(image) = images.get_mut(&texture_handle.0) {
            image.resize(Extent3d {
                width: 1024,
                height: 1024,
                ..default()
            });
        }
    }
}

@TethysSvensson
Copy link
Contributor

I think this was introduced by #4745.

Before this change the physical size of the camera target was calculated directly in the extract_cameras system, however after this change the size is calculated and cached in the camera_system system.

On the surface this looks okay, however the camera_system runs too early for this to work as expected.

The reason for this is that the image.resize() function will only add an AssetEvent<Image> to the events field that lives inside the Assets<Image> resource, instead of adding it to the Events resource itself, where the camera_system looks.

The asset_event_system will move events from the events field to the Events<Image> resource itself, however that only happens after the camera_system has run.

This also explains why the workaround posted by @DGriffin91 works. This work-around adds the event directly to the Events resource, which will then be available once the camera_system runs.

I think a better work-around would be to schedule asset_event_system to also run in the PostUpdate stage like this:

app.add_system_to_stage(
    CoreStage::PostUpdate,
    Assets::<Image>::asset_event_system.before(CameraUpdateSystem),
);

I am not sure what a correct fix for the problem would look like.

Vrixyz added a commit to Vrixyz/bevy that referenced this issue Sep 6, 2022
@darthdeus
Copy link
Contributor

darthdeus commented Nov 3, 2022

I can confirm having just tried this that this workaround works Vrixyz@3f1cc36, getting a crash without the AssetEvent::Modified event, and no crash and proper resizing with the event. (posting this in case someone finds it randomly like I did)

@alice-i-cecile
Copy link
Member

This appears to be fixed on main; can you try to reproduce there?

@zaycev zaycev closed this as completed Feb 18, 2023
@Shatur
Copy link
Contributor

Shatur commented Mar 1, 2023

Bevy no longer crashes, but without AssetEvent::Modified the image will be stretched horizontally.
With the event emitted manually:
изображение
Without the mentioned workaround:
изображение

@alice-i-cecile alice-i-cecile reopened this Mar 2, 2023
@alice-i-cecile alice-i-cecile added the S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! label Mar 2, 2023
@Shatur
Copy link
Contributor

Shatur commented Nov 20, 2023

Still present even with Bevy Asset v2.

@Shatur
Copy link
Contributor

Shatur commented Nov 20, 2023

Since Bevy no longer crashes, I created a new issue with the updated description and minimal project to reproduce: #10665.

@Shatur Shatur closed this as completed Nov 20, 2023
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 S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

No branches or pull requests

7 participants