Add default rendering processor #85

nchashch · 2016-08-23T16:46:07Z

Changes:

Add processors::rendering module containing RenderingProcessor type, component types such as Renderable, Light, Camera, and a RendererConfig type
Add examples/renderable.rs and examples/pong.rs examples
Add ApplicationBuilder::register method to allow registering components with the application using the following syntax:

Application::build(StateType::new(), context)
    .with::<ProcessorType>(processor, "processor_name", 0)
    .register::<ComponentType>()
    .done();

Add amethyst_renderer::Camera::orthographic method, which returns an orthographic projection matrix with the given parameters
Rename amethyst_context::Config to amethyst_context::ContextConfig
Add amethyst::engine::Config, which contains ContextConfig and RendererConfig

* Now Application::build() takes a Context as a paramater instead of Config * Now RenderingProcessor::new() takes scene_name and context as parameters and calls context.renderer.add_scene() * Add num_lights method to context::renderer::Renderer * Add num_fragments method to context::renderer::Renderer * Note: if you delete a Component associated Fragment or Light won't be deleted from the frame.

* Now Fragments and Ligts corresponding to deleted components are deleted from the frame

* Add RendererConfig config struct to the processors::rendering mod, it contains the pipeline (Forward or Deferred), the shading (Flat or Shaded), and the clear color. * Now RenderingProcessor::new takes RendererConfig as a parameter and creates and sets a pipeline according to this config * Add ACTIVE_SCENE_NAME and ACTIVE_CAMERA_NAME constants to processors::rendering mod * Now RenderingProcessor::new doesn't take scene_name as a parameter instead it adds the default scene with name = ACTIVE_SCENE_NAME to the frame

* Add translation, rotation_axis, rotation_angle, and scale fields to Renderable component

* Add amethyst::engine::Config containing ContextConfig and RendererConfig * Rename amethyst_context::Config to amethyst_context::ContextConfig

* Remove State::update impl from window.rs example * Remove State::update impl from sphere.rs example

* Don't register rendering processor Components in ApplicationBuilder::new.

* Now amethyst::processor::Camera::new takes Projection as an argument instead of fov, aspect, near, and far arguments. * Add amethyst::renderer::Camera::orthographic function, which returns an orthographic projection matrix.

* Add more doc comments to processors::rendering mod

kvark · 2016-08-23T17:17:47Z

examples/pong.rs

+        use std::ops::Deref;
+
+        // Get all needed component storages and resources
+        let context = context.lock().unwrap();


I wonder... technically, why can't this context just be an ECS resource?

I think it could work, but I haven't tried it yet. Probably it would make Application code less ugly, since it would allow removing all context.lock.unwrap() calls in Application impl and only &mut World would need to be passed to State methods instead of &mut Context and &mut World. Also it would simplify Processor implementation code, since you won't have to deal with Arc<Mutex<Context>> argument in Processor::run anymore.

But I think it would be better to put this change in a separate PR.

Sure thing. What about println! calls though - would you be able to fix them in this PR?

There is no propper logging yet, so it is either println! or panic!. And I think logging also should be added in a separate PR.

kvark · 2016-08-23T17:28:28Z

src/processors/rendering.rs

+                        if let Some(transform) = context.renderer.mut_fragment_transform(ACTIVE_SCENE_NAME, idx) {
+                            *transform = renderable.transform;
+                        } else {
+                            println!("Error: entity with id = {0} is deleted, \


same here, error!

kvark · 2016-08-23T17:32:47Z

Got a few notes on the PR, plus the cgmath dependency seems to be screwed up (according to Travis).
Other than that, here is some impressive work, thanks @nchashch !
Would be great to have @csherratt and @ebkalderon eyes here as well.

* Panic if renderer_config.pipeline or renderer_config.shading field is invalid.

Emilgardis · 2016-09-05T09:28:41Z

~~hello_world.md needs to get updated to match the changes to Config. Otherwise everything looks good!~~ Edit: Oops, no changes needed 👍

ghost · 2016-09-05T17:40:34Z

src/processors/rendering.rs

+impl RenderingProcessor {
+    pub fn new(renderer_config: RendererConfig, context: &mut Context) -> RenderingProcessor {
+        let clear_color = renderer_config.clear_color;
+        let pipeline = match renderer_config.pipeline.as_str() {


Ni: this might be tidier if you use a single match.

match (renderer_config.pipeline.as_str(), renderer_config.shading.as_str()) { ("Forward", "Flat") => forward_flat(clear_color), ("Forward", "Shaded") => forward_shaded(clear_color), ("Deferred", "Flat") => deferred_flat(clear_color), ("Deferred", "Shaded") => deferred_shaded(clear_color), _ => panic!("Error: Can't provide rendering pipeline requested in renderer_config, \ renderer_config.shading field is invalid.") }

ghost · 2016-09-05T17:44:51Z

Some minor nits, but otherwise it LGTM

kvark · 2016-09-08T13:06:46Z

@nchashch ok, considering the logging is left out for another PR, could you please rebase the code (and may be fix a few @csherratt -noted nits) for the merge?

ebkalderon · 2016-09-08T13:09:25Z

@nchashch This looks pretty good to me! Hopefully we can get this in a mergeable state soon.

* Derive Copy and Clone for Light, Projection and Camera types * Derive Clone for Renderable type

kvark · 2016-09-10T21:20:00Z

Thanks @nchashch, merging now!

nchashch added 15 commits August 4, 2016 02:59

Add a simple rendering processor

f5c9519

Add Light component

f0a2710

Fix deletion problem in rendering processor

4596072

* Now Fragments and Ligts corresponding to deleted components are deleted from the frame

Update hello_world, window, and sphere examples

3b9c6b9

Add Renderable::update_transform_matrix method

196d26f

* Add translation, rotation_axis, rotation_angle, and scale fields to Renderable component

Add application Config

ba803eb

* Add amethyst::engine::Config containing ContextConfig and RendererConfig * Rename amethyst_context::Config to amethyst_context::ContextConfig

Remove unnecessary State::update impls from examples

60e4ee7

* Remove State::update impl from window.rs example * Remove State::update impl from sphere.rs example

Refactor examples' event handling code

ffbb420

Add ApplicationBuilder::register method

794d020

* Don't register rendering processor Components in ApplicationBuilder::new.

Add Projection enum to amethyst::processor mod

c9da8b3

* Now amethyst::processor::Camera::new takes Projection as an argument instead of fov, aspect, near, and far arguments. * Add amethyst::renderer::Camera::orthographic function, which returns an orthographic projection matrix.

Add examples/pong.rs example

831433a

Don't reexport everything in processors::rendering

b4e80b9

* Add more doc comments to processors::rendering mod

Sync with the latest develop branch

cac16ba

kvark reviewed Aug 23, 2016
View reviewed changes

Use cgmath version 0.10 in amethyst_renderer

1d90b1b

kvark reviewed Aug 23, 2016
View reviewed changes

nchashch added 3 commits August 23, 2016 21:32

Use cgmath version 0.10 in amethyst_context

2361e42

Fix code style in examples and rendering.rs

f783e2b

Panic if RenderingProcessor::new fails

bd897b8

* Panic if renderer_config.pipeline or renderer_config.shading field is invalid.

ghost reviewed Sep 5, 2016
View reviewed changes

nchashch added 5 commits September 10, 2016 16:30

Fix match statement in RenderingProcessor::new

65f56a7

Derive Copy trait for amethyst_renderer::Light

a4c0500

Derive Copy+Clone for types in rendering proc mod

73dd7eb

* Derive Copy and Clone for Light, Projection and Camera types * Derive Clone for Renderable type

Sync with the latest develop branch

b17b86e

Use cgmath 0.11 in context and renderer crates

1e12aac

kvark merged commit e1c862b into amethyst:develop Sep 10, 2016

LucioFranco mentioned this pull request Sep 10, 2016

Default Amethyst Processors and Components for ECS #60

Closed

7 tasks

nchashch deleted the renderingprocessor branch October 8, 2016 17:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add default rendering processor #85

Add default rendering processor #85

nchashch commented Aug 23, 2016

kvark Aug 23, 2016

nchashch Sep 4, 2016

kvark Sep 4, 2016

nchashch Sep 8, 2016

kvark Aug 23, 2016

kvark commented Aug 23, 2016

Emilgardis commented Sep 5, 2016 •

edited

ghost Sep 5, 2016

ghost commented Sep 5, 2016

kvark commented Sep 8, 2016

ebkalderon commented Sep 8, 2016

kvark commented Sep 10, 2016

Add default rendering processor #85

Add default rendering processor #85

Conversation

nchashch commented Aug 23, 2016

kvark Aug 23, 2016

Choose a reason for hiding this comment

nchashch Sep 4, 2016

Choose a reason for hiding this comment

kvark Sep 4, 2016

Choose a reason for hiding this comment

nchashch Sep 8, 2016

Choose a reason for hiding this comment

kvark Aug 23, 2016

Choose a reason for hiding this comment

kvark commented Aug 23, 2016

Emilgardis commented Sep 5, 2016 • edited

ghost Sep 5, 2016

Choose a reason for hiding this comment

ghost commented Sep 5, 2016

kvark commented Sep 8, 2016

ebkalderon commented Sep 8, 2016

kvark commented Sep 10, 2016

Emilgardis commented Sep 5, 2016 •

edited