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

Add vertical offset parameter to the No Sync filter #1625

Merged
merged 3 commits into from
Feb 6, 2025
Merged

Conversation

bmatherly
Copy link
Member

As requested here:
https://forum.shotcut.org/t/filter-no-sync-horizontal-offset/45942

Depends on this PR:
dyne/frei0r#203

Note: I swapped horizontal and vertical nomenclature in the UI. The vertical offset moves the picture up and down. The horizontal offset moves the picture left and right.

@ddennedy, code review not really required. But I would like to make sure we are both on the same page for horiztonal/vertical labels.

image

src/qml/filters/nosync/meta.qml Outdated Show resolved Hide resolved
src/qml/filters/nosync/meta.qml Outdated Show resolved Hide resolved
src/qml/filters/nosync/ui.qml Outdated Show resolved Hide resolved
src/qml/filters/nosync/ui.qml Outdated Show resolved Hide resolved
This will get the new parameter for No Sync
@bmatherly
Copy link
Member Author

This builds on Linux for me. I have not tried to build it in Windows or Mac.

@ddennedy
Copy link
Member

ddennedy commented Feb 6, 2025

If we have a build problem with Frei0r then alI of this PR will be reverted unless you expect me to l tease it apart because I typically do a squash merge.

@bmatherly
Copy link
Member Author

If we have a build problem with Frei0r then alI of this PR will be reverted unless you expect me to l tease it apart because I typically do a squash merge.

I think squash merge and full revert would be best. Or, we could try to test the build before we merge. I see that the PR only triggers the code style check. Is there a way to make it kick full builds from the branch?

Otherwise, I can checkout the branch on Windows and test the build. But I do not have a Mac to test on.

Or maybe it is easer to merge and then deal with a broken build. I would follow your preference.

@ddennedy
Copy link
Member

ddennedy commented Feb 6, 2025

Remove the unrelated Frei0r commit and wait for me to get around to it.

@bmatherly
Copy link
Member Author

The Frei0r commit is related because it will pull in this required commit that was recently merged into Frei0r master:
dyne/frei0r@2328ce3

I am happy to wait. This PR is not urgent to me.

@ddennedy
Copy link
Member

ddennedy commented Feb 6, 2025

Sorry I overlooked the Frei0r PR in the description because I was only following updates on my phone. Let’s try it. I think it was only the Windows SDK build that failed previously, and I can revisit that if needed.

@ddennedy ddennedy merged commit 37eb2c9 into master Feb 6, 2025
1 check passed
@ddennedy ddennedy added this to the v25.02 milestone Feb 6, 2025
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