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

[WIP] Implement texture staging #1202

Merged
merged 2 commits into from Mar 10, 2017

Conversation

@Bastacyclop
Copy link
Member

commented Mar 4, 2017

Implements texture staging by providing copy_buffer_to_texture and copy_texture_to_buffer methods.
Would close #1181

@kvark: @Androcas and me will move on to the D3D11 implementation if you like the OpenGL one, I'm not sure if we want to add another example for staging or modify an existing one?

fn on_resize(&mut self, window_targets: gfx_app::WindowTargets<R>) {
fn on_resize_ext<F: gfx::Factory<R>>(&mut self,
factory: &mut F,
window_targets: gfx_app::WindowTargets<R>) {

This comment has been minimized.

Copy link
@Bastacyclop

Bastacyclop Mar 4, 2017

Author Member

That's hacky but it's currently hard to get a hold on the factory with gfx_app

@@ -189,7 +188,7 @@ pub fn create_main_targets_raw(dim: texture::Dimensions, color_format: format::S
levels: 1,
kind: texture::Kind::D2(dim.0, dim.1, dim.3),
format: color_format,
bind: RENDER_TARGET,
bind: memory::RENDER_TARGET | memory::TRANSFER_SRC,

This comment has been minimized.

Copy link
@Bastacyclop

Bastacyclop Mar 4, 2017

Author Member

Not sure if we want more flexibility here

@kvark

kvark approved these changes Mar 7, 2017

Copy link
Member

left a comment

Looks great, minus a few nits.

@@ -59,8 +62,20 @@ impl Vertex {
}

//----------------------------------------

type DataType = <<ColorFormat as Formatted>::Surface as SurfaceTyped>::DataType;

This comment has been minimized.

Copy link
@kvark

kvark Mar 7, 2017

Member

while it is technically correct, it's still an overkill for the examples. I think using [u8; 4] is fine

self.download.raw(),
0
).unwrap();
println!("screenshot taken");

This comment has been minimized.

Copy link
@kvark

kvark Mar 7, 2017

Member

nit: indentation

let reader = factory.read_mapping(&self.download).unwrap();
let mut data = Vec::with_capacity(w as usize * h as usize * 4);
for l in reader.chunks(w as usize).rev() {
for &v in l {

This comment has been minimized.

Copy link
@kvark

kvark Mar 7, 2017

Member

nit: use data.extend

data.push(v[2]);
}
}
let path = "screen.png";

This comment has been minimized.

Copy link
@kvark

kvark Mar 7, 2017

Member

indentation again?


// In this example the transform is static except for window resizes.
let proj = cgmath::perspective(cgmath::deg(45.0f32), window_targets.aspect_ratio, 1.0, 10.0);
self.bundle.data.transform = (proj * default_view().mat).into();
}

fn on(&mut self, event: winit::Event) {
use winit::Event::*;

This comment has been minimized.

Copy link
@kvark

kvark Mar 7, 2017

Member

nit: lets not do * and instead just do use winit::{Event, ElementState, VirtualKeyCode as Key};

@@ -100,6 +100,16 @@ pub enum Command {
CopyBuffer(Buffer, Buffer,
gl::types::GLintptr, gl::types::GLintptr,
gl::types::GLsizeiptr),
CopyBufferToTexture(Buffer, gl::types::GLintptr,

This comment has been minimized.

Copy link
@kvark

kvark Mar 7, 2017

Member

no support for picking a layer yet?

This comment has been minimized.

Copy link
@Bastacyclop

Bastacyclop Mar 7, 2017

Author Member

Isn't that supported through RawImageInfo ?

This comment has been minimized.

Copy link
@kvark

kvark Mar 8, 2017

Member

RIght, I see now

@Bastacyclop

This comment has been minimized.

Copy link
Member Author

commented Mar 8, 2017

@kvark So you want to keep the cube example screenshot thing ? I did it for testing but I didn't think we would keep it (with the screenshot being saved upon resize :s).

@kvark

This comment has been minimized.

Copy link
Member

commented Mar 8, 2017

@Bastacyclop to be honest, I think another example (like the flowmap one) would be a better place for screenshot function

@Bastacyclop Bastacyclop force-pushed the Bastacyclop:textage branch from 75db505 to e77a4e5 Mar 8, 2017

@Bastacyclop

This comment has been minimized.

Copy link
Member Author

commented Mar 10, 2017

@kvark I think it's ready to be merged, we only tested framebuffer download.

@kvark

This comment has been minimized.

Copy link
Member

commented Mar 10, 2017

Thanks @Bastacyclop !

@kvark kvark merged commit 6a1e6af into gfx-rs:master Mar 10, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details

@kvark kvark removed the status: working label Mar 10, 2017

@Bastacyclop Bastacyclop deleted the Bastacyclop:textage branch Mar 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.