Skip to content

Conversation

@SpecificProtagonist
Copy link
Contributor

@SpecificProtagonist SpecificProtagonist commented May 6, 2025

Objective

The new viewport example allocates a texture in main memory, even though it's only needed on the GPU. Also fix an unnecessary warning when a viewport's texture doesn't exist CPU-side.

Testing

Run the viewport_node example.

@SpecificProtagonist SpecificProtagonist added C-Examples An addition or correction to our examples D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 6, 2025
Copy link
Member

@chompaa chompaa left a comment

Choose a reason for hiding this comment

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

Unfortunately, after this PR the viewport_node example looks like this for me:

Screenshot_2025-05-06_12-55-12

Note the distorted image.

@JMS55
Copy link
Contributor

JMS55 commented May 7, 2025

I didn't test the example admittedly.

@grind086
Copy link
Contributor

grind086 commented May 7, 2025

The main world access is used by update_viewport_render_target_size to update the texture when the viewport node's size changes.

pub fn update_viewport_render_target_size(
viewport_query: Query<
(&ViewportNode, &ComputedNode),
Or<(Changed<ComputedNode>, Changed<ViewportNode>)>,
>,
camera_query: Query<&Camera>,
mut images: ResMut<Assets<Image>>,
) {
for (viewport, computed_node) in &viewport_query {
let camera = camera_query.get(viewport.camera).unwrap();
let size = computed_node.size();
let Some(image_handle) = camera.target.as_image() else {
continue;
};
let size = Extent3d {
width: u32::max(1, size.x as u32),
height: u32::max(1, size.y as u32),
..default()
};
images.get_mut(image_handle).unwrap().resize(size);
}
}

@SpecificProtagonist
Copy link
Contributor Author

Thanks, fixed! <3 Also removed the query for the window size, as the result gets overwritten anyways.

@SpecificProtagonist SpecificProtagonist changed the title viewport_node example: Remove main world render asset usage viewport_node example: Remove main world image initialization May 7, 2025
Copy link
Member

@chompaa chompaa left a comment

Choose a reason for hiding this comment

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

Works as expected now, thanks!

Screenshot_2025-05-07_16-46-41

@SpecificProtagonist SpecificProtagonist added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 8, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 26, 2025
Merged via the queue into bevyengine:main with commit 1c8d2ee May 26, 2025
34 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Jun 9, 2025
# Objective

Follow up for #19116 and #19098

## Testing

viewport_node example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Examples An addition or correction to our examples D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants