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

stem waveformrenderer using rendergraph #14192

Merged
merged 2 commits into from
Feb 3, 2025
Merged

Conversation

m0dB
Copy link
Contributor

@m0dB m0dB commented Jan 18, 2025

Ports the stem waveformrenderer to a rendergraph node. This can be reviewed and merged independently from the PRs for the other waveformrenderers.

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.

LGTM! Just a small nit
Just need to do testing

Edit: tested and just one nit!

src/waveform/renderers/allshader/waveformrendererstem.cpp Outdated Show resolved Hide resolved
@acolombier
Copy link
Member

Some conflict have developed - Happy to merge once resolved!

@m0dB
Copy link
Contributor Author

m0dB commented Feb 2, 2025

I have resolved this conflict, but I have one doubt: in main we use ControlProxy, and here we use PollingControlProxy. One of the conflicts was around this code. I think the change to PollingControlProxy comes from you, right?

@acolombier
Copy link
Member

Yes, that's right - a ControlProxy is not needed here as we don't need any signal/connection so a PollingControlProxy a sufficient

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.

LGTM, thanks!

@acolombier acolombier merged commit 91100d7 into mixxxdj:main Feb 3, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants