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

rendergraph: use opengl node and add shaders #14021

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

m0dB
Copy link
Contributor

@m0dB m0dB commented Dec 14, 2024

This is a follow up for #14007

With this PR, the allshader waveformwidget and allshader waveformrenderer classes use rendergraph at the highest level: each waveformrenderXX class is derived from a rendergraph node, and all nodes are added to as children to a top node in the waveformwidget. Rendering is that done using the rendergraph engine.

At this point all allshader waveformrenderer classes are derived from a rendergraph::OpenGLNode, and the actual rendering code is still OpenGL, and not yet using the rendergraph render functionality. So effectively the change is that the paintGL functions are called through the rendergraph engine, traversing the nodes, rather than by iterating over them directly in the waveformwidget.

The subsequent PRs will derive each of the allshader waveformrenderer classes from rendergraph::GeometryNode (or of a rendergraph::Node containing a tree of GeometryNodes).

This PR also adds the rendergraph shaders, which will be needed by these PRs.

Copy link
Member

@acolombier acolombier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, just need to make a quick manual testing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of using python instead of perl? The rationnal is, most of the tools script we have are either shell (sh, ps or bat for windows) or python, and I'm thinking it could be great to reduce the proliferation of languages.

Happy to merge it as is, and provide a follow up PR to translate into Python if you would want some help on the matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for me to do this in perl is that I can do this very quickly in perl, and in python it would take me much longer. If you feel like porting it to python, be my guest!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll send you a PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rgba.frag.gl
texture.frag.gl
unicolor.frag.gl
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you feel about automating the generation of the .gl script using something like this?

execute_process(
  COMMAND ./generate_shaders_gl.pl
  RESULT_VARIABLE result_generate_gl
)
if (${result_generate_gl})
  message(FATAL_ERROR "Unable to generate the GL files: ${result_generate_gl}")
endif()

Alternatively, what do you think of auto generating this in the pre-commit hooks? (Note: using Python for the script might be a requirement, but this needs to be checked)

      - id: generate-rendergraph-gl
        name: generate-rendergraph-gl
        description: "Generate GL files for rendergraph saher"
        entry: ./src/rendergraph/shaders/generate_shaders_gl.pl
        require_serial: true
        language: perl
        files: ^src/rendergraph/shaders/pattern.frag/.*\.(frag|vert)$

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it is worth the effort. I can simply commit the generated shaders. If the need arises to add more shaders, we can improve on this. Also note that this is only a temporary solution: With Qt >=6.6 we can use the qsb bundles from C++ directly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we've just updated to Ubuntu 24.04 (Qt 6.4), it may be 2 years+ before the minimum Qt version is 6.6+. I don't mind updating this in a follow up PR tho.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, yes, doing this in a follow up PR would be best.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre-commit is the better option, as it will only run on changes, not every time you build. Also, I'm not sure that python is already a build-time dependency.

I included the pre-commit hook in m0dB#18

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

Successfully merging this pull request may close these issues.

3 participants