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] Gfx Renderer #45

Closed
wants to merge 22 commits into from

Conversation

@ghost
Copy link

commented May 4, 2016

Basically from a high level command are abstracted away from the data in the rendering pipeline. Commands are matched to a Method that can work with it's data. Right now that is just based on the render target and the Command itself. I think this can expand in the future.

Users can add their own targets and their own methods to the Renderer, and in addition replace methods. This replacing could be neat for doing something like a shader toy.

Right now I have a basic flat shading pipeline, and a terribly unoptimized deferred lighting. But it should be easy to to keep adding new passes and build a library of useful ones.

Demo of it working: https://www.youtube.com/watch?v=m3Q_3behpPA

Problem areas:

  • No texture support yet
  • The mesh format is static, we will need something like gfx_mesh to allow for multiple formats without static types
  • Resizing of buffers is incomplete
  • Single threaded
let combuf = factory.create_command_buffer();

let sphere = build_sphere();
let buffer = factory.create_vertex_buffer(&sphere);

This comment has been minimized.

Copy link
@kvark

kvark May 4, 2016

Member

you can use let (buffer, slice) = factory.create_vertex_buffer_with_slice(&sphere, ());

frame.passes[0].operations = vec![
Box::new(amethyst_renderer::Clear{color: [0., 0., 0., 1.]}),
Box::new(amethyst_renderer::Draw{
camera: format!("main"),

This comment has been minimized.

Copy link
@kvark

kvark May 4, 2016

Member

There is too much format! noise. I understand that we need to receive String there, but maybe we can help it by adding some constructors like amethyst_render::Draw::new("main", "main")?


frame.scenes.insert(format!("main"), scene);
frame.passes.push(amethyst_renderer::RenderPasses::new(format!("gbuffer")));
frame.passes[0].operations = vec![

This comment has been minimized.

Copy link
@kvark

kvark May 4, 2016

Member

I'd much prefer the passes to be constructed in advance, and then you'd have passes: vec![pass1, pass2] in the frame construction code. Modifying passes[i] directly can be error-prone during refactoring.

output_depth: main_depth
}
));
frame.targets.insert(

This comment has been minimized.

Copy link
@kvark

kvark May 4, 2016

Member

we could have something like this as a helper:

frame.add_target("gbuffer", amethyst_renderer::GBuffer::new(..));
));
frame.targets.insert(
format!("gbuffer"),
Box::new(amethyst_renderer::GBuffer::new(&mut factory, (800, 600)))

This comment has been minimized.

Copy link
@kvark

kvark May 4, 2016

Member

should probably extract those dimensions from the main_color

scene: format!("main")
})
];
frame.passes.push(amethyst_renderer::RenderPasses::new(format!("main")));

This comment has been minimized.

Copy link
@kvark

kvark May 4, 2016

Member

the default passes could be something simpler for this example since you already have this code bound on Key1

This comment has been minimized.

Copy link
@ghost

ghost May 4, 2016

Author

Yeah, need to pull out the description of the passes into a function.


pub struct Renderer<R: gfx::Resources, C: gfx::CommandBuffer<R>> {
command_buffer: gfx::Encoder<R, C>,
methods: HashMap<(TypeId, TypeId), Box<Fn(&Box<Operation>, &Target, &Frame<R>, &mut gfx::Encoder<R, C>)>>

This comment has been minimized.

Copy link
@kvark

kvark May 4, 2016

Member

uh oh, ok
this is apparently what forces you to do all these R::* : Any constraints when implementing Method

This comment has been minimized.

Copy link
@ghost

ghost May 4, 2016

Author

I think I can add a magic trait that will make this prettier, one of the todos.

use gfx;

/// A `Method` is an implemnatnion of a Pass
pub trait Method<A, T, R, C>

This comment has been minimized.

Copy link
@kvark

kvark May 5, 2016

Member

Shouldn't Pass and Framebuffer be associated types? This would reduce the mental overhead of reading the trait and improve clarity. Unless there are cases where the same struct implements methods for different passes and framebuffers.
Nitpic: framebuffer is a very GL-specific name. I'd prefer Target, which happens to match T.

renderer.submit(&frame, &mut device);
window.swap_buffers().unwrap();
}
}

This comment has been minimized.

Copy link
@kvark

kvark May 5, 2016

Member

you don't seem to have newlines at the end of the files


pub struct Renderer<R: gfx::Resources, C: gfx::CommandBuffer<R>> {
command_buffer: gfx::Encoder<R, C>,
methods: HashMap<(TypeId, TypeId), Box<Fn(&Box<Pass>, &Framebuffer, &Frame<R>, &mut gfx::Encoder<R, C>)>>

This comment has been minimized.

Copy link
@kvark

kvark May 5, 2016

Member

I wonder if making a trait instead of this function would allow you to remove the Pass boxing, thus stripping the need to derive from Any for passes.

@kvark

This comment has been minimized.

Copy link
Member

commented May 5, 2016

Reminder: this PR is not to be merged, since it is targeting the wrong branch.

@ghost ghost closed this May 5, 2016

This issue was closed.

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.