Bug 431915 - Rectangular region: resizing with keyboard shortcuts doesn't work properly
Summary: Rectangular region: resizing with keyboard shortcuts doesn't work properly
Status: RESOLVED FIXED
Alias: None
Product: Spectacle
Classification: Applications
Component: General (show other bugs)
Version: 20.12.1
Platform: Manjaro Linux
: NOR normal
Target Milestone: ---
Assignee: Boudhayan Gupta
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2021-01-22 07:46 UTC by Filip Majetić
Modified: 2022-04-11 15:15 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Majetić 2021-01-22 07:46:29 UTC
SUMMARY
Resizing the selection via keyboard is inconsistent, and some "directions" don't work.

STEPS TO REPRODUCE
1. Select a rectangular region
2. Press any of the resize keyboard shortcuts to see the results


OBSERVED RESULT
Pressing Alt+Shift+Right/Down increases selection width/height by 2 pixels
Pressing Alt+Shift+Left/Up does nothing
Pressing Alt+Right/Down increases selection width/height by 16 pixels
Pressing Alt+Left/Up reduces selection width/height by 14 pixels

EXPECTED RESULT
Alt+Shift+Arrow used to increase/decrease selection size by 1 pixel
Alt+Arrow used to increase/decrease selection size by 10, maybe 15 pixels.


SOFTWARE/OS VERSIONS
Linux/KDE Plasma: 
KDE Plasma Version: 5.20.5
KDE Frameworks Version: 5.78.0
Qt Version: 5.15.2
Kernel Version: 5.4.89-1-MANJARO
OS Type: 64-bit
Comment 1 Nate Graham 2021-01-22 15:31:32 UTC
Can confirm.
Comment 2 2wxsy58236r3 2021-02-22 05:22:47 UTC
My screen is 1920x1080 (only one monitor is connected, no scaling), but when I take a screenshot in rectangular region mode, and I draw the selection from the very top left of the screen to the very bottom right of the screen, the selection size can sometimes exceed the 1080p resolution, e.g. 1921x1082.

Then pressing Alt+Shift+Left/Up (which should do nothing due to the bug) repeatedly will further increase the selection size, e.g. 1951x1095, which is not expected because the size should not exceed the monitor's resolution.

But even if the selection size exceeds the resolution, the saved screenshot will be 1920x1080, which is fine.
Comment 3 oioi555x 2022-03-18 21:21:04 UTC
I've tried fixing a bug, but I don't know how to report it.
Could someone adapt to the upstream?

 src/QuickEditor/QuickEditor.cpp | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/QuickEditor/QuickEditor.cpp b/src/QuickEditor/QuickEditor.cpp
index 277f831..4bb90c8 100644
--- a/src/QuickEditor/QuickEditor.cpp
+++ b/src/QuickEditor/QuickEditor.cpp
@@ -204,9 +204,10 @@ void QuickEditor::keyPressEvent(QKeyEvent *event)
             break;
         }
         const qreal step = (shiftPressed ? 1 : magnifierLargeStep);
-        const int newPos = boundsUp(qRound(mSelection.top() * devicePixelRatio - step), false);
+        const int yPos = modifiers & Qt::AltModifier ? mSelection.bottom() : mSelection.top();
+        const int newPos = boundsUp(qRound(yPos * devicePixelRatio - step), false);
         if (modifiers & Qt::AltModifier) {
-            mSelection.setBottom(devicePixelRatioI * newPos + mSelection.height());
+            mSelection.setBottom(devicePixelRatioI * newPos);
             mSelection = mSelection.normalized();
         } else {
             mSelection.moveTop(devicePixelRatioI * newPos);
@@ -222,7 +223,7 @@ void QuickEditor::keyPressEvent(QKeyEvent *event)
         const qreal step = (shiftPressed ? 1 : magnifierLargeStep);
         const int newPos = boundsRight(qRound(mSelection.left() * devicePixelRatio + step), false);
         if (modifiers & Qt::AltModifier) {
-            mSelection.setRight(devicePixelRatioI * newPos + mSelection.width());
+            mSelection.setRight(devicePixelRatioI * newPos + mSelection.width() -1);
         } else {
             mSelection.moveLeft(devicePixelRatioI * newPos);
         }
@@ -237,7 +238,7 @@ void QuickEditor::keyPressEvent(QKeyEvent *event)
         const qreal step = (shiftPressed ? 1 : magnifierLargeStep);
         const int newPos = boundsDown(qRound(mSelection.top() * devicePixelRatio + step), false);
         if (modifiers & Qt::AltModifier) {
-            mSelection.setBottom(devicePixelRatioI * newPos + mSelection.height());
+            mSelection.setBottom(devicePixelRatioI * newPos + mSelection.height() - 1);
         } else {
             mSelection.moveTop(devicePixelRatioI * newPos);
         }
@@ -250,9 +251,10 @@ void QuickEditor::keyPressEvent(QKeyEvent *event)
             break;
         }
         const qreal step = (shiftPressed ? 1 : magnifierLargeStep);
-        const int newPos = boundsLeft(qRound(mSelection.left() * devicePixelRatio - step), false);
+        const int xPos = modifiers & Qt::AltModifier ? mSelection.right() : mSelection.left();
+        const int newPos = boundsLeft(qRound(xPos * devicePixelRatio - step), false);
         if (modifiers & Qt::AltModifier) {
-            mSelection.setRight(devicePixelRatioI * newPos + mSelection.width());
+            mSelection.setRight(devicePixelRatioI * newPos);
             mSelection = mSelection.normalized();
         } else {
             mSelection.moveLeft(devicePixelRatioI * newPos);
Comment 5 oioi555x 2022-03-25 08:00:15 UTC
Thank you for the useful advice.
I was able to submit a patch.
Comment 6 Nate Graham 2022-04-11 15:15:16 UTC
Git commit 813d8a7183e7bd8b8463d0b0f77366a392d39429 by Nate Graham, on behalf of oioi 555.
Committed on 11/04/2022 at 15:09.
Pushed by ngraham into branch 'master'.

Fix resizing with keyboard shortcuts

The reason why "Alt + Shift + Left/Up" did not scale down was an error in position specification.

The following function has only one caller.
I also removed it because it contains unused code and increases pixel error when converting from floating point numbers to integers.
"boundsLeft", "boundsRight", "boundsUp", "boundsDown"

If the scaling is other than 100%, the amount of movement may inevitably increase.

M  +25   -78   src/QuickEditor/QuickEditor.cpp
M  +0    -4    src/QuickEditor/QuickEditor.h

https://invent.kde.org/graphics/spectacle/commit/813d8a7183e7bd8b8463d0b0f77366a392d39429