derf / feh

a fast and light image viewer
https://feh.finalrewind.org
Other
1.51k stars 159 forks source link

2.27 - 'feh --zoom fill -g 800x600' Image Now Over Blown #404

Open 0pLuS0 opened 6 years ago

0pLuS0 commented 6 years ago

Hi @derf

I run feh like below to give me a UI, which I set wallpaper from;

feh --zoom fill -g 800x600 --title Feh "/home/foo/Wallpaper/"

In 2.26.3 and other versions below it, this cmd worked, and the wallpaper would appear centered/scaled in the Feh UI.

Now in 2.27 the wallpaper image is overblown and doesn't appear correctly, so I'm not sure now if this is a bug, or Feh options have changed?

Please see the attached images.

2.26.3 showing the correct scale

feh-2 26 3

2.27 image overblown

feh-2 27

vredesbyyrd commented 6 years ago

I use feh as the image viewer for RTV, a terminal reddit app. I am seeing the same issue. Downgrading to 2.26, no problemo.

ulteq commented 6 years ago

Could you try adding the --scale-down option?

0pLuS0 commented 6 years ago

@ulteq ok I tried it and it works;

feh --scale-down --zoom fill -g 800x600 --title Feh "/home/foo/Wallpaper/"

So would this be the correct way to now use feh, or we still have a bug?

Also my entire cmd looks ok?

Thanks

ulteq commented 6 years ago

So would this be the correct way to now use feh, or we still have a bug?

That's something @derf has to decide. I do think that the current way is the "correct way" of doing it, but I also understand that a change in behavior between versions is not wanted.

Also my entire cmd looks ok?

Yes.

0pLuS0 commented 6 years ago

Ok, thanks...

0pLuS0 commented 6 years ago

@derf Ok, so new version v2.27.1, but is this fix --keep-zoom-vp issues introduced in 2.27 (patch by ulteq) mentioned on the website, this is the fix for this problem?

Because if I don't use --scale-down I am still seeing the same issue...

derf commented 6 years ago

That's a tough one. On the one hand, the current behaviour seems to be the most straightforward to implement (which is important, as feh's zoom code used to be a huge mess until @ulteq cleaned it up). On the other hand, zoom behaviour in limited-geometry mode should be like zoom-behaviour in fullscreen mode, which isn't the case here. Then again, there are also cases where --zoom options are only applied to images which are too small (whereas this bug only applies to images which are too big), so from that point of view the current behaviour is sensible.

I'll leave this report open until I have an idea of what should be the correct behaviour in this case.

0pLuS0 commented 6 years ago

@derf ok hopefully you won't be pulling hair... LOL

Thanks

ulteq commented 6 years ago

the current behaviour seems to be the most straightforward to implement

I think we would only have to adjust one line of code to go back to the old behaviour. The question is: Do we want to go back?

zoom behaviour in limited-geometry mode should be like zoom-behaviour in fullscreen mode

It never was though?

Then again, there are also cases where --zoom options are only applied to images which are too small

That's why I changed the behavior, but I guess it was not the best idea.

0pLuS0 commented 6 years ago

@ulteq hi, well I'm just an end-user, so of course I have no clue here... LOL

I only saw a problem and reported it...

ulteq commented 6 years ago

I only saw a problem and reported it...

Don't worry, I'm glad you did report it.

0pLuS0 commented 6 years ago

I wonder if others are like me, I love to tweak and hack on my box, like all the time, but just as much, I LOVE the Open Source Community, where I can report issues and see things done. :+1:

ulteq commented 6 years ago

A very simple solution would be to auto enable --scale-down together with --zoom fill. That would emulate the old behaviour.

ulteq commented 6 years ago

After thinking about it again, my final suggestion would be to improve the documentation and keep the code as it is.

derf commented 6 years ago

Yeah, that seems to be the best solution. I might even get around to that today :)