- In rectangle mode, when to adjust selection, the right, bottom edge not follow mouse
- Fix wrong rectangle size
Details
- Reviewers
rkflx ngraham - Group Reviewers
Spectacle - Commits
- R166:a147b75ef337: Fix rectangle selection edge not follow mouse
- In rectangle mode, adjust the ( topright, right, bottom right, bottom, bottom left) selection and check mouse and edge position.
2 select full screen and check size
3 save image and check image size
Diff Detail
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Thanks, that's a nice bug you caught there! Basically the snapping of the cursor to the selection rectangle has an unwanted offset for both the bottom and the right edge.
However, are you sure your patch is working right? On a 1280x720px screen, without your patch the largest rectangle I'm able to draw is 1280x720px, but with your patch it only has a size of 1279x719px.
I'm also not a fan of those magic numbers in the code. Could you try to figure out where those are coming from and perhaps define them as a constant?
That said, I wonder why we need that snapping behaviour at all. Both GIMP and Inkscape do not snap the cursor to the object and instead simply allow to resize directly, resulting in a less jumpy and unexpected experience which is important for this kind of precision work.
If @ngraham agrees, maybe that would be a good direction for your patch (and also for Gwenview's Crop tool, I guess…).
I second everything @rkflx said. Let's disambiguate the magic numbers, and try to rid of this jumpy behavior in the first place.
- fix wrong rect width and height the pixels is start from 0, so the width should be start_offset - end_offset + 1
- fix right and bottom edge move unexpected bug
- select a rect, make sure the right/bottom edge is beside moniter edge
- drag the topleft, top, top right up quickly or drage top left, left, bottom left to left quickly, the bottom/ right edge move unexpected to fix this issue, just set x/y before width/height
Thanks for the updates, works great now and looks better with those constants. Some cleanup in the code left, then this can go in.
@ngraham Could you have a second look on the code with your QML experience? Thanks ;)
(I guess reducing the "jumpiness" is better done in a separate patch.)
src/QuickEditor/SelectionRectangle.qml | ||
---|---|---|
28–29 | Please change var to int, which should be faster. | |
84–85 | Is it really necessary to swap those lines? (Same comment applies to a bunch of other places…) |
Thanks for this patch, Tao! The code looks sensible to me, and it works as advertised. A good improvement to Rectangular Region mode
I noticed you didn't use arc to produce the patch, which means we need to ask you for your email address before we can land this (if and when Henrik signs off too). Can you provide your email address?
You might look into using arc in the future: https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist_to_post_patches
Thank you ngraham
Tao Guo <guotao945@gmail.com>
src/QuickEditor/SelectionRectangle.qml | ||
---|---|---|
84–85 | Yes, It's necessary, these changes fix another bug: select a rect, make sure the right/bottom edge is beside moniter edge I will submit these changes in separate patch |