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

Use same syntax for updating uniform inputs and uniform buffer inputs #1683

Closed
3 tasks
heinezen opened this issue Aug 25, 2024 · 0 comments · Fixed by #1690
Closed
3 tasks

Use same syntax for updating uniform inputs and uniform buffer inputs #1683

heinezen opened this issue Aug 25, 2024 · 0 comments · Fixed by #1690
Labels
area: renderer Concerns our graphics renderer code quality Does not alter behavior, but beauty of our code 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 opengl

Comments

@heinezen
Copy link
Member

heinezen commented Aug 25, 2024

Required Skills: C++, maybe OpenGL

Difficulty: Easy

In #1664 we switched the structure of OpenGL uniform inputs (GlUniformInput) to more performance friendly code. Uniform buffer inputs (GlUniformBufferInput) were not changed in the PR as the performance gain would have been only marginal. However, it would make sense for code quality purposes to keep the update syntax of both input types roughly the same, so that they can be maintained in the same way.

The tasks mostly consist of porting the changes from #1664 to the flow used for updating the uniform buffers. A simple example for uniform buffers that can be used for testing purposes is available in renderer demo 5 which you can access by running this command:

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

Tasks:

  • Port the changes to GlUniformInput made in Vectorize uniform inputs #1664 to the GlUniformBufferInput class
  • Change the update workflow of uniform buffers, so that it aligns with the update workflow of regular uniform inputs
  • (optional) Test the performance of the new solution using callgrind:
valgrind --tool=callgrind ./run test -d renderer.tests.renderer_demo 5

Further Reading

@heinezen heinezen added improvement Enhancement of an existing component area: renderer Concerns our graphics renderer lang: c++ Done in C++ code code quality Does not alter behavior, but beauty of our code good first issue Suitable for newcomers just do it You can start working on this, there should be nothing left to discuss opengl 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 moved this from 📋 Backlog to 👀 In review in openage renderer Sep 18, 2024
@heinezen heinezen moved this from To do to Review in progress in openage code quality Sep 18, 2024
@heinezen heinezen added the hacktoberfest For newcomers from Hacktoberfest event label Sep 20, 2024
heinezen added a commit that referenced this issue Sep 22, 2024
Update syntax of GlUniformBuffer to match that of GlUniformInput #1683
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in openage renderer Sep 22, 2024
@github-project-automation github-project-automation bot moved this from Review in progress to Done in openage code quality Sep 22, 2024
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 code quality Does not alter behavior, but beauty of our code 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 opengl
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant