Fit to width and height

[programmer-ceds]

This code adds "fit to width" and "fit to height" entries to the View menu and icons to the toolbar.

Whilst there is a fit to width plugin I think that this code has a number of advantages:

1. The new code is included in the standard program - the user doesn't have to realise that they have to enable a plugin to provide what is a common function of viewing programs.

2. The new code provides a better fit to width result than the plugin - see the examples in the uploaded file. The examples have the background set to orange to highlight the differences.

3. The new code also offers fit to height.

4. The new code uses modifiers to provide a number of options - these are listed in the keyboard shortcuts section of the help file. The key combinations are as follows (note that Ctrl was not used since Ctrl + W terminates xviewer):

W (with no modifiers) - fit to width with the image scrolled so that the vertical mid-point of the image is halfway down the window. This is the way that the fit to width plugin operates.

Shift + W - fit to width with the image scrolled so that the top of the image is at the top of the window (this was the feature that sparked the changed code - for example when looking at 'contact sheets' that only had a few images at the top having the image scrolled halfway down was not helpful)

Alt + W - fit to width with the image scrolled so that the bottom of the image is at the bottom of the window

Shift + Alt + W - fit to width with the vertical scrolling based on the position of the mouse cursor

H (with no modifiers) - fit to height with the horizontal mid-point of the image halfway across the window

Shift + H - fit to height with the image scrolled so that the left-hand side of the image is at the left of the window

Alt + H - fit to height with the image scrolled so that the right-hand side of the image is at the right of the window

Shift + Alt + H - fit to width with the horizontal scrolling of the image based on the position of the mouse cursor

The menu entries and icons work as if the keys had been used without any modifiers being applied.

I have provided icons for all of the sizes that are included in xviewer - I'm not sure whether or not this is necessary.

[programmer-ceds]

Further to the above I had considered allowing the Shift and Alt modifiers to be used with the icons and menu entries but couldn't decide what to do if an invalid combination was used.

Using the keyboard only the user might expect an invalid combination of key and modifiers would cause no action - I wasn't sure that this would be the case with a mouse click plus modifiers. If others think differently I can modify the code accordingly. The alternative approaches that I considered were:

1. Ignore all modifiers apart from the Shift and Alt keys (e.g. Ctrl + Shift + key would be processed simply as Shift + key)
2. Pass all of the modifiers to the width/height fitting code - this would result in invalid combinations being ignored and width/height fitting not happening

[clefebvre]

This is very nice and it works well. I completely agree with core functionality getting in the core program.

• The plugin should be removed from xviewer-plugins.
• We should take the opportunity to grab existing translations and inject them in here too.
• I would prefer if the new features were only visible in the menu, and not the toolbar, it's getting a bit crowded.

Controlling the scroll position feels niche to me. Couldn't we just always set it to the center or according to mouse position? People are unlikely to look at the help and discover this. I'm wondering if it's that useful also and worried we'll lack translations for the newly introduced msgids.

Looking at the code, I find xviewer_window_simulate_keypress() overkill and unnecessary.

[programmer-ceds]

@clefebvre

1. Agree about removing the plug-in but not sure how to go about that, or is that something that you would do?
2. Similar comments regarding translations as to point 1
3. I can remove the icons from the menu bar .
4. So what if we just had W and H to fit to width and height centred in the other axis and Shift+W and Shift+H to fit width and height with the scrolling of the other axis based on the mouse position (although personally I would prefer W and H without Shift to do this as I normally use the feature to look at various sections of panoramas and a single keypress makes life easier. However, doing what I have suggested would be compatible with the existing fit width plug-in)
5. How do I get to execute the code in xviewer-scroll-view.c/display_key_press_event() if I don't simulate a keypress? Unless that section of code in the keyhandler is made into a function - but I think that that would be even more clunky.

[clefebvre]

@programmer-ceds

I'll take care of 1 and 2.

[clefebvre]

4. single keypress (W or H) which follows mouse position for scroll position sounds good to me.
5. you'd need to expose the feature via a cmd function like all the other ones.. check how ViewZoomIn works for instance, it calls xviewer_window_cmd_zoom_in().

[programmer-ceds]

4. single keypress (W or H) which follows mouse position for scroll position sounds good to me.

   5. you'd need to expose the feature via a cmd function like all the other ones.. check how ViewZoomIn works for instance, it calls xviewer_window_cmd_zoom_in().

@clefebvre - That was the answer I was hoping for in 4 - I'll change the code.

With regards to 5. The fit width/height functions are handled by two switch cases in xviewer-scroll-view.c/display_key_press_event(). I see from xviewer_window_cmd_zoom_in() how to get the priv structure for the display so I could call scroll-view functions to fit width and height but these functions would then have to duplicate some of the initial processing performed in the keyhandler and, more importantly, the zooming processing performed following the switch statement - resulting in having the same section of code to maintain in two places. The simulation of the keys seems like a simpler option to me.

@clefebvre Update: I have removed the icons from the toolbar and changed the key handling so that H and W take the scroll position for the opposite axis from the mouse. The shortcuts page of the help files has been edited to reflect this change.

Just waiting for your comments on point 5 then I will submit the changed files.

[clefebvre]

hi @programmer-ceds, for point 5, I don't have a strong opinion on it. If you prefer keeping it this way, then it's fine with me.

[programmer-ceds]

@clefebvre - For point 5I would prefer to keep the code as it is.

I have just tried to upload the edited files and am getting an error that I haven't seen before. I'm fairly confident that I am entering the stings correctly - any ideas? I think that the original submit was via https rather than SSH - could this be the issue?

`richard@Home-Linux:~/xviewer/xviewer$ git add src/xviewer-scroll-view.c

richard@Home-Linux:~/xviewer/xviewer$ git add src/xviewer-window.c

richard@Home-Linux:~/xviewer/xviewer$ git add help/C/shortcuts.page

richard@Home-Linux:~/xviewer/xviewer$ git commit -m "Fit to width and height"

[master ea9874a] Fit to width and height

3 files changed, 232 insertions(+), 14 deletions(-)

richard@Home-Linux:~/xviewer/xviewer$ git push -u origin FitWidthAndHeight

error: src refspec FitWidthAndHeight does not match any

error: failed to push some refs to 'https://github.com/linuxmint/xviewer.git'

richard@Home-Linux:~/xviewer/xviewer$ `

One other point - I wonder if it would be better to centre the scrolling when the menu items are used to fit to width/height - otherwise the scrolling is based on where the cursor is as it is activating the menu item.

[clefebvre]

Looks like you're pointing at the wrong remote.. your origin should be your own repo, not Linux Mint's.

[programmer-ceds]

Looks like you're pointing at the wrong remote.. your origin should be your own repo, not Linux Mint's.

@clefebvre - not sure why that is I have followed exactly the same procedure that I have before without seeing this problem.

I think it must be connected with the HTTPS/SSH change - I have changed some things and I now get:

error: failed to push some refs to 'git@github.com:programmer-ceds/xviewer.git'

so some progress forwards. I'll try again later on. Worst case I guess I delete the PR and start again,

[programmer-ceds]

I'm going round in circles with this - I can remove most of the error messages but still left with two. It's going to be quicker to close this PR, delete the fork and start with a new fork from the current master. Hope that's OK.