mitchcurtis / slate

Pixel Art Editor
GNU General Public License v3.0
1.07k stars 103 forks source link

Cursor work #97

Closed not-surt closed 5 years ago

not-surt commented 5 years ago

Add option to always show crosshair cursor as I find I often loose track of the rectangle cursor at low zoom levels.

Snap rectangle cursor to pixel. Doesn't position properly on second split pane, can't figure out how to get the offset of the second pane.

mitchcurtis commented 5 years ago

Sorry for the late reply.

I had a quick look and test. Looks good, but a few issues.

Doesn't position properly on second split pane, can't figure out how to get the offset of the second pane.

From QML you can use secondPane.integerOffset.. or did you mean from C++? Can you link/point to the code where you need it?

mitchcurtis commented 5 years ago

Also, would you mind adding the following patch? I didn't notice it until now, but the ScrollView doesn't clip its contents, resulting in the settings coming out of the dialog:

diff --git a/app/qml/ui/OptionsDialog.qml b/app/qml/ui/OptionsDialog.qml
index eb97fee..5928874 100644
--- a/app/qml/ui/OptionsDialog.qml
+++ b/app/qml/ui/OptionsDialog.qml
@@ -80,6 +80,8 @@ Dialog {
             }

             ScrollView {
+                clip: true
+
                 ScrollBar.horizontal.policy: ScrollBar.AsNeeded

                 Layout.fillWidth: true
not-surt commented 5 years ago

Can you please add a setter for these two members? That way we can check if they actually change and potentially save a few unnecessary signal emissions.

Can do. I just figured since it should never be set anywhere other than updateCursorPos wouldn't be needed.

Is it necessary to remove these? Are they not used anywhere?

Ah, no, they aren't supposed to be removed. Still used for displaying cursor coordinates. Will restore them.

From QML you can use secondPane.integerOffset.. or did you mean from C++? Can you link/point to the code where you need it?

Sorry I meant the screen space offset of the pane itself. Position would have been the more sensible word for me to use. So I can offset the rectangle cursor based on the screen position of the current pane.

not-surt commented 5 years ago

Actually, thinking about, snapping the rectangle cursor to pixel but not showing the crosshair cursor doesn't give the user any way of knowing the exact cursor position which is pretty horrible usability.

So should rectangle ignore pixel snapping when the crosshair is hidden?

Is hiding the crosshair even desirable? Does it need to be an option?

Personally I always want to see the exact cursor position as well as the exact bush position so I'm in favour of always showing crosshair and always snapping rectangle.

mitchcurtis commented 5 years ago

I think it looks a bit weird/jarring having both visible, personally, so it's not something I would want to have on by default.

Always snapping the cursor rectangle sounds like it could be useful though, but again, I think this should be behind a setting. These are the kinds of changes that are small but affect usability in subtle ways.

not-surt commented 5 years ago

Reverted changes to rectangular cursor as couldn't figure out how to get correct position and won't work on its own with pixel snapping.

So just the option to always show the crosshair cursor now.