Closed insel-maz closed 1 month ago
Not exactly sure what this change would actually fix or improve.
projectM already has an API function to set the output size, so the application can already choose to render at any specified resolution.
In v4.2, we'll be introducing proper custom FBO rendering support. See the master branch, which already has the additional API functions for this purpose. projectM itself should not perform any up- or downscaling or any other additional postprocessing an application may require. Rendering to a custom FBO/texture instead of the native window surface at the required resolution, then drawing the resulting texture into the application's output surface at the required size and location would be the proper way of doing that. The resulting texture can even be used as a source material for 3D objects, e.g. a TV screen inside a larger scene.
RenderContext is a structure that is passed to the preset and contains all the data like vewport size, audio data and timing information. After the preset has finished rendering, this structure is essentially no longer required except for transitions.
At the current state the viewport is set by for example by MilkdropPreset to the viewportSize of renderContext (e.g. 540 px * 960 px).
At the last step, when the texture is copied to the framebuffer, the viewport still applies. The problem is that the framebuffer (e.g. 1080 px * 1920 px) may differ in size from what I told projectm to render in.
My pull request restores the viewport (e.g. 1080 px * 1920 px) so that the texture copier upscales to the right framebuffer size.
Isn't that exactly the use case explained above? Which would be implemented like this:
While this seems like a lot of steps, most of them are required anyway or just once, e.g. setting the texture size. Plus, you can apply whatever scaling or other transformation to the projectM output afterwards.
I should probably explain why I don't think adding such scaling and possibly other things to the library:
While projectM could surely perform the scaling, this will then lead to others wanting more effects, scaling algorithms etc. inside the library, which isn't really a core responsibility of projectM. Adding features which users can easily apply outside in their applications adds more code, thus making projectM more prone to bugs and increasing the maintenance overhead for the development team.
Resetting the viewport parameters after rendering could be done to restore the original state, but this applies to all other OpenGL states as well, so the application should always make sure to set up the proper rendering states before performing the next draw call.
I agree that good software should focus on one task. But in the last step, the image data should be transferred correctly from the texture to the frame buffer. I was happy when I saw that the final texture is already drawn there as a whole to the framebuffer, because it's exactly what I would otherwise have to do one more time by hand (as you described). I would like to avoid additional copy steps (this is an experiment on mobile). And with the current code base I think it is not necessary. The good stuff is already there. A picture is worth a thousand words. Current main branch:
With my fix applied:
I see the point. The next thing I would ask about would then be the specification of texture filtering, etc.
And so that I don't have to ask, I have updated the code according to my ideas. ;)
Well, I'd actually say this proves my point. Such a change will lead to more and more additions, which are all based on individual needs of application developers, while not being a core task of projectM. The visuals should be rendered at the specific resolution to a texture (or viewport) with a matching size.
Any additional scaling, filtering or transformation required by an application should be done outside the library. The only reason I can see here is "it's just some testing code and I don't want to implement it myself" - which is fine. In this case, I'd suggest adding the required changes to libprojectM in your personal fork to try out how things work. If they look right, just pull out the required code/draw calls necessary to achieve the desired effect and revert to the official libprojectM build.
If your app is also licensed under the GPL, you can even put a modified copy of libprojectM into your app's source code. This comes with some maintenance overhead to keep it up to date, but might be a viable solution.
Besides that, any rendering-specific API calls should go into the OpenGL header, as we may add additional APIs later which may have different features and parameters.
An intermediate solution would be creating a separate small library as middleware which would perform any postprocessing like scaling and filtering etc., basically proxying the render call. This library could exist separately, not affecting the core API and having its own release cycle.
If you're interested in creating (and possibly maintaining) such a side project, I'd like to invite you to chat with me and/or the other devs on our Discord server. This library could also be put under a less restrictive license like MIT to enable static linking to applications, while libprojectM can still beinked as a shared library to a closed-source app as required by the LGPL.
After consulting with the other maintainers, we've decided not to add this into core library because of the aforementioned reasons of keeping the library/API slim and maintainable.
The general idea is appealing though, and some people might find it very useful to have scaling/filtering available as a ready-to-use feature. I've summarized the idea, added a few more suggestions and created issue #845 for implementing it as a separate middleware library.
We'd be happy if you want to start implementing it, e.g. with the up-/downscaling filters, so others can later join and add more effects. We'll create a repository in the "projectM-visualizer" org once an initial version of the library is ready.
Restore the original viewport (e.g. of the default frame buffer) so up or down scaling will work. Based on the snippet: https://stackoverflow.com/a/63279269/628696
This change is a quick fix. Maybe it is better to introduce two extra fields in RenderContext saving the original viewport size.