From c4357e663a8bc2e5960be4c2c024bd0faa2bf638 Mon Sep 17 00:00:00 2001 From: Filip Hejsek Date: Fri, 20 Feb 2026 21:07:08 +0100 Subject: [PATCH 1/4] Add a function for updating the viewport size - Never modify viewportSize outside of the added UpdateViewportSize() - Set a sane initial size in the construtor instead of haphazardly doing it in Render() - Ensure that the viewport and client sizes do not get out of sync in free size mode - Also update the size before positioning video when the tool changes (but see the added TODO) --- src/video_display.cpp | 33 +++++++++++++++++++++++---------- src/video_display.h | 8 ++++++++ 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/video_display.cpp b/src/video_display.cpp index 28c2d5f866..22c407904b 100644 --- a/src/video_display.cpp +++ b/src/video_display.cpp @@ -152,6 +152,8 @@ VideoDisplay::VideoDisplay(wxToolBar *toolbar, bool freeSize, wxComboBox *zoomBo con->videoController->JumpToFrame(con->videoController->GetFrameN()); SetLayoutDirection(wxLayout_LeftToRight); + + UpdateViewportSize(wxSize(1, 1)); } VideoDisplay::~VideoDisplay () { @@ -210,9 +212,6 @@ void VideoDisplay::Render() try { return; } - if (viewportSize.GetWidth() == 0) viewportSize.SetWidth(1); - if (viewportSize.GetHeight() == 0) viewportSize.SetHeight(1); - if (!content_height || !content_width) PositionVideo(); @@ -293,6 +292,17 @@ void VideoDisplay::DrawOverscanMask(float horizontal_percent, float vertical_per gl.DrawMultiPolygon(points, vstart, vcount, pos, v, true); } +void VideoDisplay::UpdateViewportSize(wxSize newSize) { + // In free size mode, we ignore the argument and use the client size + // to avoid the viewport and client sizes getting out of sync + if (freeSize) + newSize = GetClientSize() * scale_factor; + assert(newSize != wxDefaultSize); + if (newSize.GetWidth() < 1) newSize.SetWidth(1); + if (newSize.GetHeight() < 1) newSize.SetHeight(1); + viewportSize = newSize; +} + void VideoDisplay::PositionVideo() { auto provider = con->project->VideoProvider(); if (!provider || !IsShownOnScreen()) return; @@ -356,10 +366,10 @@ void VideoDisplay::FitClientSizeToVideo() { if (!provider || !IsShownOnScreen()) return; - viewportSize.Set(provider->GetWidth(), provider->GetHeight()); - viewportSize *= windowZoomValue; + wxSize newViewportSize(provider->GetWidth(), provider->GetHeight()); + newViewportSize *= windowZoomValue; if (con->videoController->GetAspectRatioType() != AspectRatio::Default) - viewportSize.SetWidth(viewportSize.GetHeight() * con->videoController->GetAspectRatioValue()); + newViewportSize.SetWidth(newViewportSize.GetHeight() * con->videoController->GetAspectRatioValue()); wxEventBlocker blocker(this); if (freeSize) { @@ -368,22 +378,23 @@ void VideoDisplay::FitClientSizeToVideo() { wxSize cs = GetClientSize(); wxSize oldSize = top->GetSize(); - top->SetSize(top->GetSize() + viewportSize / scale_factor - cs); + top->SetSize(top->GetSize() + newViewportSize / scale_factor - cs); SetClientSize(cs + top->GetSize() - oldSize); } else { - SetMinClientSize(viewportSize / scale_factor); - SetMaxClientSize(viewportSize / scale_factor); + SetMinClientSize(newViewportSize / scale_factor); + SetMaxClientSize(newViewportSize / scale_factor); GetGrandParent()->Layout(); } + UpdateViewportSize(newViewportSize); // must be called after setting the client size PositionVideo(); } void VideoDisplay::OnSizeEvent(wxSizeEvent &) { if (freeSize) { - viewportSize = GetClientSize() * scale_factor; + UpdateViewportSize(); PositionVideo(); windowZoomValue = double(content_height) / con->project->VideoProvider()->GetHeight(); zoomBox->ChangeValue(fmt_wx("%g%%", windowZoomValue * 100.)); @@ -597,6 +608,8 @@ void VideoDisplay::SetTool(std::unique_ptr new_tool) { else { // FitClientSizeToVideo fits the window to the video, which we don't want to do GetGrandParent()->Layout(); + // TODO Is the following really necessary? Wouldn't it be taken care of by OnSizeEvent? + UpdateViewportSize(); PositionVideo(); } } diff --git a/src/video_display.h b/src/video_display.h index d6f68aee56..067239d7b6 100644 --- a/src/video_display.h +++ b/src/video_display.h @@ -147,6 +147,14 @@ class VideoDisplay final : public wxGLCanvas { /// @brief Recompute the size of the viewport based on the current window zoom and video resolution, /// then resize the client area to match the viewport void FitClientSizeToVideo(); + /// @brief Set the viewport size to @p newSize + /// + /// You should call @ref PositionVideo() after this. + /// + /// In free size mode, the @p newSize argument is ignored and client size is used instead. + /// + /// @param newSize The new size, ignored in free size mode + void UpdateViewportSize(wxSize newSize = wxDefaultSize); /// @brief Update content size and position based on the current viewport size, content zoom and pan /// /// Updates @ref content_left, @ref content_width, @ref content_bottom, @ref content_top and @ref content_height From fd258b6c25a25d95aedc859f49be040c95ceb2db Mon Sep 17 00:00:00 2001 From: Filip Hejsek Date: Fri, 20 Feb 2026 19:34:18 +0100 Subject: [PATCH 2/4] Store video pan in absolute units and rescale when the viewport changes --- src/video_display.cpp | 32 ++++++++++++++++++-------------- src/video_display.h | 2 +- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/video_display.cpp b/src/video_display.cpp index 22c407904b..1d0f56a3cb 100644 --- a/src/video_display.cpp +++ b/src/video_display.cpp @@ -300,6 +300,11 @@ void VideoDisplay::UpdateViewportSize(wxSize newSize) { assert(newSize != wxDefaultSize); if (newSize.GetWidth() < 1) newSize.SetWidth(1); if (newSize.GetHeight() < 1) newSize.SetHeight(1); + if (viewportSize.GetHeight() >= 1) { + double ratio = double(newSize.GetHeight()) / viewportSize.GetHeight(); + pan_x *= ratio; + pan_y *= ratio; + } viewportSize = newSize; } @@ -338,14 +343,14 @@ void VideoDisplay::PositionVideo() { double content_top_exact = double(viewportSize.GetHeight() - content_height) / 2; // Don't allow panning too far out of bounds - double max_pan_x = (0.5 * content_width + 0.4 * viewportSize.GetWidth()) / viewportSize.GetHeight(); - double max_pan_y = (0.5 * content_height + 0.4 * viewportSize.GetHeight()) / viewportSize.GetHeight(); + double max_pan_x = 0.5 * content_width + 0.4 * viewportSize.GetWidth(); + double max_pan_y = 0.5 * content_height + 0.4 * viewportSize.GetHeight(); pan_x = mid(-max_pan_x, pan_x, max_pan_x); pan_y = mid(-max_pan_y, pan_y, max_pan_y); // Apply panning - content_left_exact += pan_x * viewportSize.GetHeight(); - content_top_exact += pan_y * viewportSize.GetHeight(); + content_left_exact += pan_x; + content_top_exact += pan_y; content_left = std::round(content_left_exact); content_top = std::round(content_top_exact); @@ -502,8 +507,8 @@ void VideoDisplay::OnGestureZoom(wxZoomGestureEvent& event) { } void VideoDisplay::Pan(Vector2D delta) { - pan_x += delta.X() * scale_factor / viewportSize.GetHeight(); - pan_y += delta.Y() * scale_factor / viewportSize.GetHeight(); + pan_x += delta.X() * scale_factor; + pan_y += delta.Y() * scale_factor; PositionVideo(); } @@ -539,18 +544,17 @@ Vector2D VideoDisplay::GetZoomAnchorPoint(wxPoint position) { // // position = viewportSize / 2 + anchorPoint // - // Panning shifts the video center by `pan * viewportHeight`, so we have to add that to the viewport center: + // Panning shifts the video center by `pan`, so we have to add that to the viewport center: // - // position = viewportSize / 2 + pan * viewportHeight + anchorPoint + // position = viewportSize / 2 + pan + anchorPoint // // Finally, to apply scaling, we need to multiply the offset from the video center by the zoom value, so the final formula is // - // position = viewportSize / 2 + pan * viewportHeight + anchorPoint * contentZoomValue + // position = viewportSize / 2 + pan + anchorPoint * contentZoomValue // // Now, to obtain the anchor point from the position, we have to invert the formula. Vector2D viewportCenter = Vector2D(viewportSize.GetWidth(), viewportSize.GetHeight()) / 2; - Vector2D scaledPan = Vector2D(pan_x, pan_y) * viewportSize.GetHeight(); - return (Vector2D(position) - viewportCenter - scaledPan) / contentZoomValue; + return (Vector2D(position) - viewportCenter - Vector2D(pan_x, pan_y)) / contentZoomValue; } void VideoDisplay::ZoomAndPan(double newZoomValue, Vector2D anchorPoint, wxPoint newPosition) { @@ -558,10 +562,10 @@ void VideoDisplay::ZoomAndPan(double newZoomValue, Vector2D anchorPoint, wxPoint // Compute a pan value to maintain the formula derived above Vector2D viewportCenter = Vector2D(viewportSize.GetWidth(), viewportSize.GetHeight()) / 2; - Vector2D newScaledPan = Vector2D(newPosition) - viewportCenter - anchorPoint * newZoomValue; + Vector2D newPan = Vector2D(newPosition) - viewportCenter - anchorPoint * newZoomValue; - pan_x = newScaledPan.X() / viewportSize.GetHeight(); - pan_y = newScaledPan.Y() / viewportSize.GetHeight(); + pan_x = newPan.X(); + pan_y = newPan.Y(); contentZoomValue = newZoomValue; PositionVideo(); diff --git a/src/video_display.h b/src/video_display.h index 067239d7b6..422abb3685 100644 --- a/src/video_display.h +++ b/src/video_display.h @@ -147,7 +147,7 @@ class VideoDisplay final : public wxGLCanvas { /// @brief Recompute the size of the viewport based on the current window zoom and video resolution, /// then resize the client area to match the viewport void FitClientSizeToVideo(); - /// @brief Set the viewport size to @p newSize + /// @brief Set the viewport size to @p newSize and rescale the pan values /// /// You should call @ref PositionVideo() after this. /// From 40c30c102592841038923fc0852598c11e3c8539 Mon Sep 17 00:00:00 2001 From: Filip Hejsek Date: Tue, 10 Feb 2026 22:47:25 +0100 Subject: [PATCH 3/4] Fix incorrect window zoom value bug The window zoom value was calculated incorrectly in free size mode when content zoom was active. --- src/video_display.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/video_display.cpp b/src/video_display.cpp index 1d0f56a3cb..486ef3e512 100644 --- a/src/video_display.cpp +++ b/src/video_display.cpp @@ -315,8 +315,8 @@ void VideoDisplay::PositionVideo() { content_width = viewportSize.GetWidth(); content_height = viewportSize.GetHeight(); - // Adjust aspect ratio if necessary if (freeSize) { + // Adjust aspect ratio if necessary int vidW = provider->GetWidth(); int vidH = provider->GetHeight(); @@ -332,6 +332,12 @@ void VideoDisplay::PositionVideo() { else if (videoAr - displayAr > 0.01) { content_height = content_width / videoAr; } + + // Update windowZoomValue + // We must use content_height before content zoom has been applied to it + windowZoomValue = double(content_height) / vidH; + zoomBox->ChangeValue(fmt_wx("%g%%", windowZoomValue * 100.)); + con->ass->Properties.video_zoom = windowZoomValue; } // Apply content zoom @@ -398,16 +404,8 @@ void VideoDisplay::FitClientSizeToVideo() { } void VideoDisplay::OnSizeEvent(wxSizeEvent &) { - if (freeSize) { - UpdateViewportSize(); - PositionVideo(); - windowZoomValue = double(content_height) / con->project->VideoProvider()->GetHeight(); - zoomBox->ChangeValue(fmt_wx("%g%%", windowZoomValue * 100.)); - con->ass->Properties.video_zoom = windowZoomValue; - } - else { - PositionVideo(); - } + if (freeSize) UpdateViewportSize(); + PositionVideo(); } void VideoDisplay::OnMouseEvent(wxMouseEvent& event) { From 43fb81f588c7c75be6db868bbd9c92e598be1ac5 Mon Sep 17 00:00:00 2001 From: Filip Hejsek Date: Wed, 11 Feb 2026 00:37:59 +0100 Subject: [PATCH 4/4] Preserve content size when resizing detached video and panning is active --- src/video_display.cpp | 22 ++++++++++++++-------- src/video_display.h | 8 ++++++-- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/video_display.cpp b/src/video_display.cpp index 486ef3e512..d710464f9e 100644 --- a/src/video_display.cpp +++ b/src/video_display.cpp @@ -153,7 +153,7 @@ VideoDisplay::VideoDisplay(wxToolBar *toolbar, bool freeSize, wxComboBox *zoomBo SetLayoutDirection(wxLayout_LeftToRight); - UpdateViewportSize(wxSize(1, 1)); + UpdateViewportSize(false, wxSize(1, 1)); } VideoDisplay::~VideoDisplay () { @@ -292,7 +292,7 @@ void VideoDisplay::DrawOverscanMask(float horizontal_percent, float vertical_per gl.DrawMultiPolygon(points, vstart, vcount, pos, v, true); } -void VideoDisplay::UpdateViewportSize(wxSize newSize) { +void VideoDisplay::UpdateViewportSize(bool rescalePan, wxSize newSize) { // In free size mode, we ignore the argument and use the client size // to avoid the viewport and client sizes getting out of sync if (freeSize) @@ -300,7 +300,7 @@ void VideoDisplay::UpdateViewportSize(wxSize newSize) { assert(newSize != wxDefaultSize); if (newSize.GetWidth() < 1) newSize.SetWidth(1); if (newSize.GetHeight() < 1) newSize.SetHeight(1); - if (viewportSize.GetHeight() >= 1) { + if (rescalePan && viewportSize.GetHeight() >= 1) { double ratio = double(newSize.GetHeight()) / viewportSize.GetHeight(); pan_x *= ratio; pan_y *= ratio; @@ -308,10 +308,12 @@ void VideoDisplay::UpdateViewportSize(wxSize newSize) { viewportSize = newSize; } -void VideoDisplay::PositionVideo() { +void VideoDisplay::PositionVideo(bool preserveContentSize) { auto provider = con->project->VideoProvider(); if (!provider || !IsShownOnScreen()) return; + int old_content_height = content_height; + content_width = viewportSize.GetWidth(); content_height = viewportSize.GetHeight(); @@ -340,6 +342,9 @@ void VideoDisplay::PositionVideo() { con->ass->Properties.video_zoom = windowZoomValue; } + if (preserveContentSize) + contentZoomValue = double(old_content_height) / content_height; + // Apply content zoom content_width *= contentZoomValue; content_height *= contentZoomValue; @@ -399,13 +404,14 @@ void VideoDisplay::FitClientSizeToVideo() { GetGrandParent()->Layout(); } - UpdateViewportSize(newViewportSize); // must be called after setting the client size + UpdateViewportSize(true, newViewportSize); // must be called after setting the client size PositionVideo(); } void VideoDisplay::OnSizeEvent(wxSizeEvent &) { - if (freeSize) UpdateViewportSize(); - PositionVideo(); + bool preserveContentSize = freeSize && IsContentZoomActive(); + if (freeSize) UpdateViewportSize(false); + PositionVideo(preserveContentSize); } void VideoDisplay::OnMouseEvent(wxMouseEvent& event) { @@ -611,7 +617,7 @@ void VideoDisplay::SetTool(std::unique_ptr new_tool) { // FitClientSizeToVideo fits the window to the video, which we don't want to do GetGrandParent()->Layout(); // TODO Is the following really necessary? Wouldn't it be taken care of by OnSizeEvent? - UpdateViewportSize(); + UpdateViewportSize(true); PositionVideo(); } } diff --git a/src/video_display.h b/src/video_display.h index 422abb3685..a16f7b73c2 100644 --- a/src/video_display.h +++ b/src/video_display.h @@ -153,12 +153,15 @@ class VideoDisplay final : public wxGLCanvas { /// /// In free size mode, the @p newSize argument is ignored and client size is used instead. /// + /// @param rescalePan Should the current pan be rescaled for the new viewport? /// @param newSize The new size, ignored in free size mode - void UpdateViewportSize(wxSize newSize = wxDefaultSize); + void UpdateViewportSize(bool rescalePan, wxSize newSize = wxDefaultSize); /// @brief Update content size and position based on the current viewport size, content zoom and pan /// /// Updates @ref content_left, @ref content_width, @ref content_bottom, @ref content_top and @ref content_height - void PositionVideo(); + /// + /// @param preserveContentSize Should content zoom be adjusted to maintain current content size? + void PositionVideo(bool preserveContentSize = false); /// Set the window zoom level to that indicated by the dropdown void SetWindowZoomFromBox(wxCommandEvent&); /// Set the window zoom level to that indicated by the text @@ -225,6 +228,7 @@ class VideoDisplay final : public wxGLCanvas { /// @brief Reset content zoom and pan void ResetContentZoom(); + bool IsContentZoomActive() { return contentZoomValue != 1 || pan_x != 0 || pan_y != 0; } /// Get the last seen position of the mouse in script coordinates Vector2D GetMousePosition() const;