Skip to content

Commit

Permalink
Fix Minor Bug with Zoom Timeline to Fit
Browse files Browse the repository at this point in the history
As reported here:
https://forum.shotcut.org/t/minor-bug-with-zoom-timeline-to-fit/38270

Perform the zoom twice (separated by a timer delay) to get the zoom
correct.
  • Loading branch information
bmatherly committed Aug 11, 2023
1 parent bd62f33 commit 2df1292
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 8 deletions.
30 changes: 23 additions & 7 deletions src/qml/views/keyframes/keyframes.qml
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,6 @@ Rectangle {
adjustZoom(-0.0625);
}

function zoomToFit() {
setZoom(Math.pow((tracksFlickable.width - 50) * timeScale / tracksContainer.width - 0.01, 1 / 3));
scrollZoomTimer.stop();
tracksFlickable.contentX = 0;
}

function resetZoom() {
setZoom(1);
}
Expand All @@ -97,6 +91,26 @@ Rectangle {
}
}

Timer {
id: zoomToFitTimer

property var loopCount: 0
interval: 1
repeat: true
function startZoomFit() {
loopCount = 0;
start();
}
onTriggered: {
setZoom(Math.pow((tracksFlickable.width - 50) * timeScale / tracksContainer.width - 0.01, 1 / 3));
loopCount++;
// Due to rounding errors, sometimes the zoom needs to be calculated twice to get it right.
if (loopCount >= 2) {
stop();
}
}
}

MouseArea {
anchors.fill: parent
acceptedButtons: Qt.RightButton
Expand Down Expand Up @@ -608,7 +622,9 @@ Rectangle {
}

function onZoomToFit() {
zoomToFit();
scrollZoomTimer.stop();
tracksFlickable.contentX = 0;
zoomToFitTimer.startZoomFit();
}

function onResetZoom() {
Expand Down
22 changes: 21 additions & 1 deletion src/qml/views/timeline/timeline.qml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,26 @@ Rectangle {
}
}

Timer {
id: zoomToFitTimer

property var loopCount: 0
interval: 1
repeat: true
function startZoomFit() {
loopCount = 0;
start();
}
onTriggered: {
setZoom(Math.pow((tracksFlickable.width - 50) * multitrack.scaleFactor / tracksContainer.width - 0.01, 1 / 3));
loopCount++;
// Due to rounding errors, sometimes the zoom needs to be calculated twice to get it right.
if (loopCount >= 2) {
stop();
}
}
}

MouseArea {
anchors.fill: parent
acceptedButtons: Qt.RightButton
Expand Down Expand Up @@ -807,9 +827,9 @@ Rectangle {
}

function onZoomToFit() {
setZoom(Math.pow((tracksFlickable.width - 50) * multitrack.scaleFactor / tracksContainer.width - 0.01, 1 / 3));
scrollZoomTimer.stop();
tracksFlickable.contentX = 0;
zoomToFitTimer.startZoomFit();
}

function onSetZoom(value) {
Expand Down

2 comments on commit 2df1292

@ddennedy
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, these changes to Zoom to Fit are much worse for me than the last release. All I do is add a 2.5 minute video to the timeline using the append keyboard shortcut and click Zoom to Fit. Not only does it not fit even after repeated clicks on the button, but the timeline origin is somewhere left of where it should be and timeline scrolling is completely broken. I do not reproduce it on the 23.07 release. I am probably going to revert all changes to zoom. The minor bug is so minor compared to this.

@ddennedy
Copy link
Member

Choose a reason for hiding this comment

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

Oh wait, I found this is due to having the Scroll to Playhead on Zoom option turned on. I will add a check to skip this stuff if that is on.

Please sign in to comment.