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

Mesh does not support DrawParams::color #215

Closed
LittleB0xes opened this issue Dec 3, 2020 · 10 comments
Closed

Mesh does not support DrawParams::color #215

LittleB0xes opened this issue Dec 3, 2020 · 10 comments
Labels
API: Breaking Indicates that resolving the issue would be a breaking change. Area: Documentation Issues related to the crate's documentation. Area: Graphics Issues related to graphics/rendering. Type: Bug Problems with the code/documentation that need to be fixed.

Comments

@LittleB0xes
Copy link

Here is my mesh creation

        let (pos_i, uv_i) = (Vec2::new(0.0, 0.0), Vec2::new(0.0, 0.0));
        let (pos_j, uv_j) = (Vec2::new(1.25, 1.25), Vec2::new(0.5, 0.5));
        let (pos_k, uv_k) = (Vec2::new(0.0, 2.5), Vec2::new(0.0, 1.0));
        let particle_vertices = &[
            Vertex::new(pos_i, uv_i, Color::WHITE),
            Vertex::new(pos_j, uv_j, Color::WHITE),
            Vertex::new(pos_k, uv_k, Color::WHITE),
        ];
        let particle_mesh = VertexBuffer::with_usage(ctx, particle_vertices, BufferUsage::Static)?.into_mesh();

When I draw it (and try to change the color with DrawParams)

            graphics::draw(
                ctx,
                &self.particle_mesh,
                DrawParams::new()
                    .position(p.position)
                    .origin(Vec2::new(0.0, 2.5))
                    .scale(Vec2::new(p.scale, p.scale))
                    .color(p.color)                //-> p.color is a Color
                    .rotation(p.theta),
            );   
        }

thinking my code had a problem, I performed the same test with the mesh.rs example (removing the texture). Same result, the color does not change

@LittleB0xes LittleB0xes added the Type: Bug Problems with the code/documentation that need to be fixed. label Dec 3, 2020
@17cupsofcoffee
Copy link
Owner

17cupsofcoffee commented Dec 3, 2020

Ah, this is a docs issue rather than a code issue - DrawParams::color can't really work for a mesh the way it is implemented now 😢 I'm not sure why I wrote that you could in the Mesh docs, that's a total brain fart on my end.

A bit of background as to why - when you draw a texture, that effectively generates a Vertex for each corner of the texture. The color for each of those vertexes is taken from DrawParams::color, and the shader multiplies that by the texture's color. This is how you're able to tint textures with a color.

With a mesh however, the Vertex is specified by you, and it's uploaded to the GPU in advance. There's no way for us to pass the tint color through to it after the fact, other than either reuploading the vertex data or by passing it in as a shader uniform. Either way, you're left with a useless DrawParams::color.

I think the way to fix this would be to make it so two colors are passed to the shader - a per-vertex color and a per-mesh color. Then I can make DrawParams::color work consistently across the entire renderer.

I will probably do this in the next version of Tetra, but I need to have a bit of a look at how other frameworks handle this first to make sure I'm not doing anything silly 😄 It'd also potentially be a breaking change for any existing custom shaders, so I'll have to make it a 0.6 release. But I kinda needed to do that anyway.

In the meantime, it's possible to tint a mesh using a custom shader (pass in the color as a uniform and then multiply the vertex color by it). I appreciate that's a bit of a pain to do though.

@17cupsofcoffee 17cupsofcoffee changed the title the color change in Draw Params doesn't seem to work when drawing a Mesh Mesh does not support DrawParams::color Dec 3, 2020
@17cupsofcoffee 17cupsofcoffee added API: Breaking Indicates that resolving the issue would be a breaking change. Area: Documentation Issues related to the crate's documentation. Area: Graphics Issues related to graphics/rendering. labels Dec 3, 2020
@17cupsofcoffee
Copy link
Owner

Aha, Love2D does exactly what I suggested (seperate shader inputs for vertex and mesh color). So that seems like a reasonable approach.

Thank you for the report, I'll make sure that gets done soon!

@LittleB0xes
Copy link
Author

So it's time for me to dive into shaders. 😄
Thank you for your clear answer

@17cupsofcoffee
Copy link
Owner

Once the fix is done, your original code should work fine without custom shaders, so the alternative is to just wait a few days 😛 But shaders are a fun thing to learn, so feel free to play around with them in the meantime!

The steps for the workaround would something like:

  • Copy the default vertex shader into your project, and make two changes:
    • Add a new uniform vec4 with a name like u_mesh_color.
    • Replace v_color = a_color with v_color = a_color * u_mesh_color.
  • In your code, load the shader using from_vertex_file (or new, but that requires you to pass in a fragment shader, whereas you just want to use the default).
  • To set the color, call .set_uniform(ctx, "u_mesh_color", Color::WHITE) (replace the color with whatever you want).
  • To enable the shader, call set_shader. All drawing will now use your custom shader and apply the tint color, including non-mesh drawing.
  • To disable the shader, call reset_shader.

@LittleB0xes
Copy link
Author

LittleB0xes commented Dec 3, 2020

For testing purpose, I used the default shader and in my particle display loop I did

        for p in self.particle_list.iter() {

            graphics::set_shader(ctx, &self.shader);
            let mut rng = rand::thread_rng();
            let red = rng.gen_range(0.0, 1.0);
            let green = rng.gen_range(0.0, 1.0);
            let blue = rng.gen_range(0.0, 1.0);

            self.shader.set_uniform(ctx, "u_red", red);
            self.shader.set_uniform(ctx, "u_green", green);
            self.shader.set_uniform(ctx, "u_blue", blue);
            graphics::draw(
                ctx,
                &self.particle_mesh,
                DrawParams::new()
                    .position(p.position)
                    .origin(Vec2::new(0.0, 2.5))
                    .scale(Vec2::new(p.scale, p.scale))
                    .rotation(p.theta),
            );

            graphics::reset_shader(ctx);
            
        }

My spaceship is now producing a nice flow of multicolored, sparkling particles.
image

This kind of technique should be sufficient while waiting for the future evolution of Tetra.
thank you for your work

@17cupsofcoffee
Copy link
Owner

🎉 Glad you were able to get it working! I usually wouldn't recommend calling set_shader/reset_shader in a loop like that, as changing shaders flushes any batched draw calls to the GPU, but since you're using Mesh (which doesn't batch draw calls) it doesn't really matter here.

I'll leave this issue open until the proper fix is done.

@17cupsofcoffee
Copy link
Owner

0.5.5 has been published with a fix for this issue.

I opted to keep vertex colours and mesh colours seperate, as trying to use a unified approach didn't really work very well with batched rendering. There's now an extra parameter (u_diffuse) in the fragment shader that holds the mesh color, and it's set to white when doing other kinds of rendering. This seemed like a simple/backwards compatible approach (it's the same thing Raylib does), and I'll hopefully be able to extend it if I ever add support for 3D meshes (don't hold your breath 😆)

@LittleB0xes
Copy link
Author

Oof! You told me to wait a few days ... you circle the sun much faster than me 😀

@17cupsofcoffee
Copy link
Owner

I couldn't help myself, leaving it broken till the weekend would have driven me crazy 😅

@LittleB0xes
Copy link
Author

It works perfectly 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: Breaking Indicates that resolving the issue would be a breaking change. Area: Documentation Issues related to the crate's documentation. Area: Graphics Issues related to graphics/rendering. Type: Bug Problems with the code/documentation that need to be fixed.
Projects
None yet
Development

No branches or pull requests

2 participants