Skip to content

[WIP] Renderer backend selection #877

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

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from
Draft

[WIP] Renderer backend selection #877

wants to merge 27 commits into from

Conversation

revmischa
Copy link
Collaborator

@revmischa revmischa commented Apr 17, 2025

This pull request refactors the libprojectM rendering pipeline to improve modularity and OpenGL backend integration. The changes primarily involve replacing the use of a generic RenderItem base class with a more specific OpenGLRenderItem class, updating initialization methods, and introducing a unique pointer for texture flipping. These updates aim to streamline the rendering process and enhance maintainability.

Transition to OpenGL-specific Rendering:

  • Replaced the inheritance of Renderer::RenderItem with libprojectM::Renderer::Backend::OpenGL::OpenGLRenderItem in multiple classes, including Border, CustomShape, CustomWaveform, DarkenCenter, Filters, and FinalComposite. This change ensures the rendering logic is explicitly tied to the OpenGL backend. [1] [2] [3] [4] [5] [6]

  • Updated initialization calls from RenderItem::Init() to Init() in constructors across several classes to align with the new base class implementation. [1] [2] [3] [4] [5] [6]

Texture Flipping Refactor:

  • Replaced the Renderer::CopyTexture instance with a std::unique_ptr<Renderer::CopyTexture> to manage the texture flipping filter. This change improves memory management and ensures proper cleanup. [1] [2] [3]

  • Updated calls to the Draw and Texture methods of m_flipTexture to use the new unique pointer syntax (m_flipTexture->Draw and m_flipTexture->Texture). [1] [2] [3] [4]

Header File Updates:

  • Replaced #include <Renderer/RenderItem.hpp> with #include <Renderer/Backend/OpenGL/OpenGLRenderItem.hpp> in header files to reflect the new OpenGL-specific base class. [1] [2] [3] [4] [5] [6]

Code Cleanup:

  • Removed redundant or outdated comments and documentation blocks in CustomWaveform and FinalComposite to simplify the code and improve readability. [1] [2]

…, `VideoEcho`, and `Waveform`) to use `OpenGLRenderItem` instead of `RenderItem`, and include the correct header file. This fixes the build errors by:

1. Changing the base class to `OpenGLRenderItem` which contains the required `m_vaoID` and `m_vboID` members
2. Updating the includes to point to the new header location
3. Removing redundant `RenderItem::` prefixes since we inherit directly from `OpenGLRenderItem`
4. Cleaning up some formatting inconsistencies

The changes maintain all existing functionality while fixing the compilation errors related to missing OpenGL buffer IDs. The classes will now properly inherit the OpenGL-specific rendering functionality they require.
@revmischa revmischa marked this pull request as draft April 17, 2025 20:55
@kblaschke
Copy link
Member

kblaschke commented Apr 18, 2025

Just as a comment here: Please don't remove the Doxygen comments from the classes (e.g. this one). They may look hard to read in the code, but enable us to generate a proper code documentation from it once all classes are properly commented/documented. If a comment is outdated after editing/refactoring something, it should rather be updated to reflect the current state.

Only documentation of:

  • override methods which already have a doxygen comment in the base class and don't have a notable extra functionality and
  • private functions/members which are self-explanatory

can/may be removed.

Most modern IDEs also provide a nice rendering of these Doxygen comments directly in the source tree. These IDEs (e.g. CLion) also show the parameter docs when hovering over a call or typing the function arguments, which is more information than just the declaration provides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants