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

Optionally check if all uniform values have been set #1681

Closed
2 tasks
heinezen opened this issue Aug 25, 2024 · 9 comments · Fixed by #1718
Closed
2 tasks

Optionally check if all uniform values have been set #1681

heinezen opened this issue Aug 25, 2024 · 9 comments · Fixed by #1718
Labels
area: renderer Concerns our graphics renderer good first issue Suitable for newcomers hacktoberfest For newcomers from Hacktoberfest event improvement Enhancement of an existing component just do it You can start working on this, there should be nothing left to discuss lang: c++ Done in C++ code nice new thing ☺ A new feature that was not there before

Comments

@heinezen
Copy link
Member

heinezen commented Aug 25, 2024

Required Skills: C++ (understanding OpenGL terminology is helpful too)

Difficulty: Easy

Whenever we want to change something in the shader for an object, we use a UniformInput object to store the values. When the object is rendered, the data from the uniform input is passed to the GPU, e.g. via OpenGL. The definitions of the uniform data rypes are stored in the ShaderProgram class.

During rendering, the GPU does not strictly require that all uniform values have to be set for rendering to happen. However, when a uniform value is not set, e.g. when someone forgets to set pass it to the uniform input, this often leads to rendering errors, or worse: nothing being rendered at all. This is often infuriating to debug as it not always clear from the start whether the value was just forgotten or another error produces the rendering problems. To mitigate these issues, we should add a way to check if all uniform values for a uniform input have been set.

To try out the renderer and see how setting uniform values work, check out renderer demo 4 by running the following command:

./run test -d renderer.tests.renderer_demo 4

Tasks:

Further Reading

@heinezen heinezen added improvement Enhancement of an existing component area: renderer Concerns our graphics renderer nice new thing ☺ A new feature that was not there before lang: c++ Done in C++ code good first issue Suitable for newcomers just do it You can start working on this, there should be nothing left to discuss labels Aug 25, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in openage renderer Aug 25, 2024
@heinezen heinezen moved this to renderer in openage beginner tasks Aug 25, 2024
@heinezen heinezen pinned this issue Aug 25, 2024
@heinezen heinezen added the hacktoberfest For newcomers from Hacktoberfest event label Sep 20, 2024
@AdamYChe
Copy link

Hey, I think I'll try this issue out.

@heinezen
Copy link
Member Author

@AdamYChe Nice! If you have any problems, contact us in our chatroom

@yukirine
Copy link

Hey ! can i try this issue too ?

@heinezen
Copy link
Member Author

@yukirine Yes, you can!

@ZzzhHe
Copy link
Contributor

ZzzhHe commented Nov 26, 2024

Hi, I’d love to contribute to this! I was wondering if it would be appropriate to call the is_complete(..) function within GlShaderProgram::update_uniforms(...)? This way, the uniforms can be verified in each iteration of the loop before rendering. Would this approach make sense?

@heinezen
Copy link
Member Author

@ZzzhHe We would probably want to avoid this (even in debug mode) because the render loop should be as fast as possible and adding sanity checks could slow it down substantially. I think the best place to call the function is in the code of the several demos that we have, see https://github.com/SFTtech/openage/tree/master/libopenage/renderer/demo

@ZzzhHe
Copy link
Contributor

ZzzhHe commented Nov 26, 2024

@heinezen Thanks! I get it. Working on it.

@ZzzhHe
Copy link
Contributor

ZzzhHe commented Nov 26, 2024

I’ve implemented an is_complete() function in GlUniformInput, but using it requires dynamic_pointer_cast since new_uniform_input() returns a UniformInput instead of GlUniformInput. Would it make sense to add is_complete() as a virtual method in UniformInput? This would allow calling is_complete() directly on UniformInput. Or, is there another approach you’d recommend?

@heinezen
Copy link
Member Author

@ZzzhHe Yes, that would make sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: renderer Concerns our graphics renderer good first issue Suitable for newcomers hacktoberfest For newcomers from Hacktoberfest event improvement Enhancement of an existing component just do it You can start working on this, there should be nothing left to discuss lang: c++ Done in C++ code nice new thing ☺ A new feature that was not there before
Projects
Archived in project
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

4 participants