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

Add screenshot api #7163

Merged
merged 24 commits into from Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
7311b14
Add screenshot api
TheRawMeatball Jan 11, 2023
539370c
fix clippy
TheRawMeatball Jan 11, 2023
ed06f12
Comment updates
TheRawMeatball Jan 12, 2023
a3208de
Switch to using custom texture for screenshots
TheRawMeatball Jan 12, 2023
6707d19
appease clippy
TheRawMeatball Jan 12, 2023
6487690
Adress more feedback
TheRawMeatball Jan 13, 2023
bfcd473
Adress WASM panic
TheRawMeatball Jan 13, 2023
6ba0228
clippy
TheRawMeatball Jan 13, 2023
90f5fed
fix hdr issue
TheRawMeatball Jan 13, 2023
baabde3
clippy, again
TheRawMeatball Jan 13, 2023
1941274
Add standalone example
TheRawMeatball Jan 13, 2023
de6eb41
Merge branch 'main' into screenshot-api
TheRawMeatball Jan 23, 2023
86798ad
appease clippy
TheRawMeatball Jan 23, 2023
ed31728
Merge branch 'main' into screenshot-api
TheRawMeatball Jan 31, 2023
edb1e57
fix bug
TheRawMeatball Jan 31, 2023
cfdb2c8
Merge branch 'main' into screenshot-api
TheRawMeatball Feb 8, 2023
1267d52
Merge remote-tracking branch 'upstream/main' into screenshot-api
TheRawMeatball Feb 15, 2023
6e0c39b
fix stuff
TheRawMeatball Feb 15, 2023
ef26a66
formatting
TheRawMeatball Feb 15, 2023
b49ff4b
Merge branch 'bevyengine:main' into screenshot-api
TheRawMeatball Feb 17, 2023
d2d72cb
Merge remote-tracking branch 'upstream/main' into screenshot-api
TheRawMeatball Feb 24, 2023
cf5fd04
fix things up
TheRawMeatball Feb 24, 2023
1da0a8a
Merge commit 'cf5fd04' into HEAD
TheRawMeatball Apr 18, 2023
23bad7a
print to log when a screenshot is taken
TheRawMeatball Apr 19, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions Cargo.toml
Expand Up @@ -1909,6 +1909,16 @@ description = "Illustrates how to customize the default window settings"
category = "Window"
wasm = true

[[example]]
name = "screenshot"
path = "examples/window/screenshot.rs"

[package.metadata.example.screenshot]
name = "Screenshot"
description = "Shows how to save screenshots to disk"
category = "Window"
wasm = false

[[example]]
name = "transparent_window"
path = "examples/window/transparent_window.rs"
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/camera/camera.rs
Expand Up @@ -423,7 +423,7 @@ impl NormalizedRenderTarget {
match self {
NormalizedRenderTarget::Window(window_ref) => windows
.get(&window_ref.entity())
.and_then(|window| window.swap_chain_texture.as_ref()),
.and_then(|window| window.swap_chain_texture_view.as_ref()),
NormalizedRenderTarget::Image(image_handle) => {
images.get(image_handle).map(|image| &image.texture_view)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/camera/camera_driver_node.rs
Expand Up @@ -52,7 +52,7 @@ impl Node for CameraDriverNode {
continue;
}

let Some(swap_chain_texture) = &window.swap_chain_texture else {
let Some(swap_chain_texture) = &window.swap_chain_texture_view else {
continue;
};

Expand Down
67 changes: 25 additions & 42 deletions crates/bevy_render/src/render_resource/texture.rs
Expand Up @@ -51,31 +51,21 @@ define_atomic_id!(TextureViewId);
render_resource_wrapper!(ErasedTextureView, wgpu::TextureView);
render_resource_wrapper!(ErasedSurfaceTexture, wgpu::SurfaceTexture);

/// This type combines wgpu's [`TextureView`](wgpu::TextureView) and
/// [`SurfaceTexture`](wgpu::SurfaceTexture) into the same interface.
#[derive(Clone, Debug)]
pub enum TextureViewValue {
/// The value is an actual wgpu [`TextureView`](wgpu::TextureView).
TextureView(ErasedTextureView),

/// The value is a wgpu [`SurfaceTexture`](wgpu::SurfaceTexture), but dereferences to
/// a [`TextureView`](wgpu::TextureView).
SurfaceTexture {
// NOTE: The order of these fields is important because the view must be dropped before the
// frame is dropped
view: ErasedTextureView,
texture: ErasedSurfaceTexture,
},
}

/// Describes a [`Texture`] with its associated metadata required by a pipeline or [`BindGroup`](super::BindGroup).
///
/// May be converted from a [`TextureView`](wgpu::TextureView) or [`SurfaceTexture`](wgpu::SurfaceTexture)
/// or dereferences to a wgpu [`TextureView`](wgpu::TextureView).
#[derive(Clone, Debug)]
pub struct TextureView {
id: TextureViewId,
value: TextureViewValue,
value: ErasedTextureView,
}

pub struct SurfaceTexture {
value: ErasedSurfaceTexture,
}

impl SurfaceTexture {
pub fn try_unwrap(self) -> Option<wgpu::SurfaceTexture> {
self.value.try_unwrap()
}
}

impl TextureView {
Expand All @@ -84,34 +74,21 @@ impl TextureView {
pub fn id(&self) -> TextureViewId {
self.id
}

/// Returns the [`SurfaceTexture`](wgpu::SurfaceTexture) of the texture view if it is of that type.
#[inline]
pub fn take_surface_texture(self) -> Option<wgpu::SurfaceTexture> {
match self.value {
TextureViewValue::TextureView(_) => None,
TextureViewValue::SurfaceTexture { texture, .. } => texture.try_unwrap(),
}
}
}

impl From<wgpu::TextureView> for TextureView {
fn from(value: wgpu::TextureView) -> Self {
TextureView {
id: TextureViewId::new(),
value: TextureViewValue::TextureView(ErasedTextureView::new(value)),
value: ErasedTextureView::new(value),
}
}
}

impl From<wgpu::SurfaceTexture> for TextureView {
impl From<wgpu::SurfaceTexture> for SurfaceTexture {
fn from(value: wgpu::SurfaceTexture) -> Self {
let view = ErasedTextureView::new(value.texture.create_view(&Default::default()));
let texture = ErasedSurfaceTexture::new(value);

TextureView {
id: TextureViewId::new(),
value: TextureViewValue::SurfaceTexture { texture, view },
SurfaceTexture {
value: ErasedSurfaceTexture::new(value),
}
}
}
Expand All @@ -121,10 +98,16 @@ impl Deref for TextureView {

#[inline]
fn deref(&self) -> &Self::Target {
match &self.value {
TextureViewValue::TextureView(value) => value,
TextureViewValue::SurfaceTexture { view, .. } => view,
}
&self.value
}
}

impl Deref for SurfaceTexture {
type Target = wgpu::SurfaceTexture;

#[inline]
fn deref(&self) -> &Self::Target {
&self.value
}
}

Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_render/src/renderer/graph_runner.rs
Expand Up @@ -57,9 +57,12 @@ impl RenderGraphRunner {
render_device: RenderDevice,
queue: &wgpu::Queue,
world: &World,
finalizer: impl FnOnce(&mut wgpu::CommandEncoder),
) -> Result<(), RenderGraphRunnerError> {
let mut render_context = RenderContext::new(render_device);
Self::run_graph(graph, None, &mut render_context, world, &[], None)?;
finalizer(render_context.command_encoder());

{
#[cfg(feature = "trace")]
let _span = info_span!("submit_graph_commands").entered();
Expand Down
9 changes: 7 additions & 2 deletions crates/bevy_render/src/renderer/mod.rs
Expand Up @@ -35,6 +35,9 @@ pub fn render_system(world: &mut World) {
render_device.clone(), // TODO: is this clone really necessary?
&render_queue.0,
world,
|encoder| {
crate::view::screenshot::submit_screenshot_commands(world, encoder);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why you didn't just import the screenshot module?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, other than not wanting to pollute the namespace i guess.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair, I don't mind either way. I'd prefer having the screenshot module in scope just to not have crate::view, but really it's up to you, really not a big deal.

},
) {
error!("Error running render graph:");
{
Expand Down Expand Up @@ -66,8 +69,8 @@ pub fn render_system(world: &mut World) {

let mut windows = world.resource_mut::<ExtractedWindows>();
for window in windows.values_mut() {
if let Some(texture_view) = window.swap_chain_texture.take() {
if let Some(surface_texture) = texture_view.take_surface_texture() {
if let Some(wrapped_texture) = window.swap_chain_texture.take() {
if let Some(surface_texture) = wrapped_texture.try_unwrap() {
surface_texture.present();
}
}
Expand All @@ -81,6 +84,8 @@ pub fn render_system(world: &mut World) {
);
}

crate::view::screenshot::collect_screenshots(world);

// update the time and send it to the app world
let time_sender = world.resource::<TimeSender>();
time_sender.0.try_send(Instant::now()).expect(
Expand Down
15 changes: 15 additions & 0 deletions crates/bevy_render/src/texture/image_texture_conversion.rs
Expand Up @@ -174,6 +174,7 @@ impl Image {
/// - `TextureFormat::R8Unorm`
/// - `TextureFormat::Rg8Unorm`
/// - `TextureFormat::Rgba8UnormSrgb`
/// - `TextureFormat::Bgra8UnormSrgb`
///
/// To convert [`Image`] to a different format see: [`Image::convert`].
pub fn try_into_dynamic(self) -> anyhow::Result<DynamicImage> {
Expand All @@ -196,6 +197,20 @@ impl Image {
self.data,
)
.map(DynamicImage::ImageRgba8),
// This format is commonly used as the format for the swapchain texture
// This conversion is added here to support screenshots
TextureFormat::Bgra8UnormSrgb => ImageBuffer::from_raw(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we introduce the TextureFormat::Bgra8UnormSrgb here? Is there a reason bgra might be preferred over rgba?

Copy link
Member

Choose a reason for hiding this comment

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

Seconding the confusion about why this is part of this PR. I don't mind it, just a bit confused.

Copy link
Member Author

Choose a reason for hiding this comment

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

wgpu apparently quite likes this format for swapchain textures - this was necessary to get the save to disk functionality working on my machine.

Copy link
Member

Choose a reason for hiding this comment

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

Interesting. Can you please leave a comment here to that effect then?

self.texture_descriptor.size.width,
self.texture_descriptor.size.height,
{
let mut data = self.data;
for bgra in data.chunks_exact_mut(4) {
bgra.swap(0, 2);
}
data
},
)
.map(DynamicImage::ImageRgba8),
// Throw and error if conversion isn't supported
texture_format => {
return Err(anyhow!(
Expand Down