Skip to content

Commit fcf22bb

Browse files
authored
Error on unloaded ImageRenderTarget (#20503)
# Objective When using `RenderTarget::Image` with an image set to use in the render world only, the `camera_system` is unable to get its render target info and fails silently, breaking things down the line. From what I saw the view uniforms stopped updating, but there might be other things it broke as well. ## Solution At least make it fail loudly :) Not sure if it should panic here, or return a better error enum that the caller can decide what to do with. Should all failures to update the render target info be hard errors? ## Testing Error gets printed when running the project I found the bug with
1 parent a22d288 commit fcf22bb

File tree

2 files changed

+56
-30
lines changed

2 files changed

+56
-30
lines changed

crates/bevy_render/src/camera.rs

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,15 @@ use bevy_camera::{
2020
primitives::Frustum,
2121
visibility::{self, RenderLayers, VisibleEntities},
2222
Camera, Camera2d, Camera3d, CameraMainTextureUsages, CameraOutputMode, CameraUpdateSystems,
23-
ClearColor, ClearColorConfig, Exposure, NormalizedRenderTarget, Projection, RenderTargetInfo,
24-
Viewport,
23+
ClearColor, ClearColorConfig, Exposure, ManualTextureViewHandle, NormalizedRenderTarget,
24+
Projection, RenderTargetInfo, Viewport,
2525
};
2626
use bevy_derive::{Deref, DerefMut};
2727
use bevy_ecs::{
2828
change_detection::DetectChanges,
2929
component::Component,
3030
entity::{ContainsEntity, Entity},
31+
error::BevyError,
3132
event::EventReader,
3233
lifecycle::HookContext,
3334
prelude::With,
@@ -167,7 +168,7 @@ pub trait NormalizedRenderTargetExt {
167168
resolutions: impl IntoIterator<Item = (Entity, &'a Window)>,
168169
images: &Assets<Image>,
169170
manual_texture_views: &ManualTextureViews,
170-
) -> Option<RenderTargetInfo>;
171+
) -> Result<RenderTargetInfo, MissingRenderTargetInfoError>;
171172

172173
// Check if this render target is contained in the given changed windows or images.
173174
fn is_changed(
@@ -222,28 +223,34 @@ impl NormalizedRenderTargetExt for NormalizedRenderTarget {
222223
resolutions: impl IntoIterator<Item = (Entity, &'a Window)>,
223224
images: &Assets<Image>,
224225
manual_texture_views: &ManualTextureViews,
225-
) -> Option<RenderTargetInfo> {
226+
) -> Result<RenderTargetInfo, MissingRenderTargetInfoError> {
226227
match self {
227228
NormalizedRenderTarget::Window(window_ref) => resolutions
228229
.into_iter()
229230
.find(|(entity, _)| *entity == window_ref.entity())
230231
.map(|(_, window)| RenderTargetInfo {
231232
physical_size: window.physical_size(),
232233
scale_factor: window.resolution.scale_factor(),
234+
})
235+
.ok_or(MissingRenderTargetInfoError::Window {
236+
window: window_ref.entity(),
233237
}),
234-
NormalizedRenderTarget::Image(image_target) => {
235-
let image = images.get(&image_target.handle)?;
236-
Some(RenderTargetInfo {
238+
NormalizedRenderTarget::Image(image_target) => images
239+
.get(&image_target.handle)
240+
.map(|image| RenderTargetInfo {
237241
physical_size: image.size(),
238242
scale_factor: image_target.scale_factor.0,
239243
})
240-
}
241-
NormalizedRenderTarget::TextureView(id) => {
242-
manual_texture_views.get(id).map(|tex| RenderTargetInfo {
244+
.ok_or(MissingRenderTargetInfoError::Image {
245+
image: image_target.handle.id(),
246+
}),
247+
NormalizedRenderTarget::TextureView(id) => manual_texture_views
248+
.get(id)
249+
.map(|tex| RenderTargetInfo {
243250
physical_size: tex.size,
244251
scale_factor: 1.0,
245252
})
246-
}
253+
.ok_or(MissingRenderTargetInfoError::TextureView { texture_view: *id }),
247254
}
248255
}
249256

@@ -265,6 +272,18 @@ impl NormalizedRenderTargetExt for NormalizedRenderTarget {
265272
}
266273
}
267274

275+
#[derive(Debug, thiserror::Error)]
276+
pub enum MissingRenderTargetInfoError {
277+
#[error("RenderTarget::Window missing ({window:?}): Make sure the provided entity has a Window component.")]
278+
Window { window: Entity },
279+
#[error("RenderTarget::Image missing ({image:?}): Make sure the Image's usages include RenderAssetUsages::MAIN_WORLD.")]
280+
Image { image: AssetId<Image> },
281+
#[error("RenderTarget::TextureView missing ({texture_view:?}): make sure the texture view handle was not removed.")]
282+
TextureView {
283+
texture_view: ManualTextureViewHandle,
284+
},
285+
}
286+
268287
/// System in charge of updating a [`Camera`] when its window or projection changes.
269288
///
270289
/// The system detects window creation, resize, and scale factor change events to update the camera
@@ -287,7 +306,7 @@ pub fn camera_system(
287306
images: Res<Assets<Image>>,
288307
manual_texture_views: Res<ManualTextureViews>,
289308
mut cameras: Query<(&mut Camera, &mut Projection)>,
290-
) {
309+
) -> Result<(), BevyError> {
291310
let primary_window = primary_window.iter().next();
292311

293312
let mut changed_window_ids = <HashSet<_>>::default();
@@ -320,25 +339,23 @@ pub fn camera_system(
320339
|| camera.computed.old_viewport_size != viewport_size
321340
|| camera.computed.old_sub_camera_view != camera.sub_camera_view)
322341
{
323-
let new_computed_target_info =
324-
normalized_target.get_render_target_info(windows, &images, &manual_texture_views);
342+
let new_computed_target_info = normalized_target.get_render_target_info(
343+
windows,
344+
&images,
345+
&manual_texture_views,
346+
)?;
325347
// Check for the scale factor changing, and resize the viewport if needed.
326348
// This can happen when the window is moved between monitors with different DPIs.
327349
// Without this, the viewport will take a smaller portion of the window moved to
328350
// a higher DPI monitor.
329351
if normalized_target.is_changed(&scale_factor_changed_window_ids, &HashSet::default())
330-
&& let (Some(new_scale_factor), Some(old_scale_factor)) = (
331-
new_computed_target_info
332-
.as_ref()
333-
.map(|info| info.scale_factor),
334-
camera
335-
.computed
336-
.target_info
337-
.as_ref()
338-
.map(|info| info.scale_factor),
339-
)
352+
&& let Some(old_scale_factor) = camera
353+
.computed
354+
.target_info
355+
.as_ref()
356+
.map(|info| info.scale_factor)
340357
{
341-
let resize_factor = new_scale_factor / old_scale_factor;
358+
let resize_factor = new_computed_target_info.scale_factor / old_scale_factor;
342359
if let Some(ref mut viewport) = camera.viewport {
343360
let resize = |vec: UVec2| (vec.as_vec2() * resize_factor).as_uvec2();
344361
viewport.physical_position = resize(viewport.physical_position);
@@ -350,12 +367,9 @@ pub fn camera_system(
350367
// arguments due to a sudden change on the window size to a lower value.
351368
// If the size of the window is lower, the viewport will match that lower value.
352369
if let Some(viewport) = &mut camera.viewport {
353-
let target_info = &new_computed_target_info;
354-
if let Some(target) = target_info {
355-
viewport.clamp_to_size(target.physical_size);
356-
}
370+
viewport.clamp_to_size(new_computed_target_info.physical_size);
357371
}
358-
camera.computed.target_info = new_computed_target_info;
372+
camera.computed.target_info = Some(new_computed_target_info);
359373
if let Some(size) = camera.logical_viewport_size()
360374
&& size.x != 0.0
361375
&& size.y != 0.0
@@ -376,6 +390,7 @@ pub fn camera_system(
376390
camera.computed.old_sub_camera_view = camera.sub_camera_view;
377391
}
378392
}
393+
Ok(())
379394
}
380395

381396
#[derive(Component, Debug)]
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
title: "RenderTarget error handling"
3+
pull_requests: [20503]
4+
---
5+
6+
`NormalizedRenderTargetExt::get_render_target_info` now returns a `Result`,
7+
with the `Err` variant indicating which render target (image, window, etc)
8+
failed to load its metadata.
9+
10+
This should mostly be treated as a hard error, since it indicates the rendering
11+
state of the app is broken.

0 commit comments

Comments
 (0)