ImageViewer: set preferred rotation and auto rotation

[poire-z]

Add option to set the preferred rotation in ImageViewer, defaults to the most natural rotation for right handed users.
Remove the obsolete setting, that used to be used for more things, but ended up being used only by ImageViewer (no migration, its name and its effect were contradictory).
Add option to auto-rotate for best fit on launch, so landscape images are auto-rotated to the preferred rotation.

Discussed in (and supersedes) #10540.

As previously with the bottom menu, it's not obvious to express a rotation in icons and words :) (And we can't use the nice icons we made for the bottom menu in our TouchMenu).
It's a bit less critical here, as there's only 2 choices and once people have tested it or choosen the other, they'll be done.

Not sure what @stelzch meant at #10540 (comment) :

However, I am still having issues if the device screen rotation is changed, in this case the image rotation does not follow the cw/ccw setting.

I guess it is that when in landscape, the rotation was inverted - which is the expected behaviour. We actually chose the prefered orientation for when we turn the device in lanscape - the prefered orientation when in portrait is fixed/hardcoded: it's the natural one. (No idea what happens with Android tablets with kickstatnds that are naturally used in landscape... Don't want to think about that :))
And when reading in landscape mode, the rotation is inverted so we get to the natural portait orientation.
Confusing to explain :)
There's not much symbols in Unicode to express this. I found these 2 ones in our nerdfont that may express something - actually, lucky the arrow are in the clockwise rotation express with the words :) But they don't say if the black thingy is the image or the device.
It can make sense if you think of it as a landscape image displayed in portrait mode: small - and getting bigger when you rotate it, the side of the image following the way of the arrow is where it will be once the image is rotated (but you have to rotate your device the other way :).... confusing.)
Dunno if the symbols should be inverted when the menu is displayed in lanscape... and/or if I should get rid of "(when in portrait)", or include the symbol in the parens (harder/clumy for the translators).

Anyway, better ideas for the wording and symbols welcome.
("Counter clockwise" or "Anti clockwise" if we're going with "clockwise" ? With a hyphen or a space in between ?)

---

This change is 

[poire-z]

@Frenzie : I let you decide if it will go in v2023.12 or not - no urgency - it has some new translatable strings, but they are nested deep in the menu, so they would be hardly noticed if still in English.

[poire-z]

No really nice expressive menu items with Unicode rotation arrows:

http://xahlee.info/comp/unicode_arrows.html

[Frenzie]

Fine by me for 2023.12

[mergen3107]

For rotation symbols, for my Profiles I use U+21BB and U+21BA.

Also, I think this notation contradicts with notation in dispatcher for device rotation. Don't know if this matter, but might bring more confusion.

See, for my Profile named "Rotate CCW", I mean rotate content CCW, but I have to use CW rotation action in Dispatcher for that.

Here, your first screenshot shows the notation similar to my Profile, not Dispatcher.

Do you care/want to make them uniform?

I personally don't think I will use this feature (but I need to test Kindle Scribe's auto rotation in Image Viewer).

[mergen3107]

Counterclockwise and Clockwise are good, I think.

[poire-z]

Yours:

The ones in Dispatcher's rotation submenu:

I don't really mind with what we are going - and I don't really care about uniformity: they all, everywhere, don't really make visual sense, except for visually explaining the clockwiseness of the rotation :) (but these 2 sets still make it more confusing, as they express a rotation of 180° or 360°, unlike the ones I found in nerdfont that have it 90°).
But in all cases, we are still stuck wondering if it's the content or the device that goes that way of the rotation, and have to test and see what happens.

So, any more feedback about which set of symbols/wording we should go with ?

[NiLuJe]

I'm happy with CW/CCW, and am perfectly fine with the icons you went with, so, nothing much to add ;p.

[NiLuJe]

Hmm, if you rotate clockwise, it's what used to be the device's top edge (when in Portrait) that ends up in your right hand?

[NiLuJe]

(Assuming we're talking about device rotations, and not buffer rotations, which seems to hold true given the code below ;))

[poire-z]

Mhhh, no :)
In my mind, in that context of ImageViewer staying in the current device orientation (so, always with the buttons at the bottom (not at the device bottom) in the current view), it is the image that is rotated inside ImageViewer (and the bottom middle button shows "Rotate" when it is not rotated, and "No rotation" when it is rotated).

So, the default [x] Clockwise when in portrait will rotate the image clockwise: what used to be the device bottom edge (when in Portrait) ends up in your right hand. (That is, if you hold it by the right bottom corner, you just let the device tilt on its left and gravity does the rest, and that corner you're still holding is at the top right).

The screenshot in #10540 (comment) shows what happens with [x] Counterclockwise when in portrait.

And don't ask me about why rotation_angle = rotate_clockwise and 270 or 90 - I haven't and don't want to look in ImageWidget to see why it works :) but it works !

[NiLuJe]

In that case, yeaaaah, I'd probably ask to invert the text (i.e., CW -> CCW), because everything else in the UI assumes we're talking about device rotation (similarly, the icons no longer make any sense now that you've clarified that, because the ones you've chosen very much look like device rotations to me).

[poire-z]

I don't care about the icons. (I understand the black thingie in mine could be thought of as the device or as the image, it's as confusing as I wrote in my first post :).
If you think the rounded arrows, being more abstract, makes it less confusing, fine with me. (Which pair then, the 180° or 360° ones?)

But even if it's in the "Rotation>" mixed of all rotation stuff submenu, it's still about rotation when in ImageViewer, which itself does not rotate: only the image in it is rotated.
So, CW / CCW meaning how one should rotate the device to see the original image - while having the ImageViewer's buttons' text vertical on the side and unreadable - feels... a lot more odd to me.

[poire-z]

Could work:

With or without the 2nd part about landscape ? And if with, which variations (comma or parens)?

[poire-z]

(Is "when in portrait" Englishly correct enough?)

[poire-z]

I guess that at this point, we could have 4 menu items :)

[x] Rotate image CW when in portrait
[ ] Rotate image CCW when in portrait
----
[ ] Rotate image CW when in landscape
[x] Rotate image CCW when in landscape

[NiLuJe]

(Is "when in portrait" Englishly correct enough?)

Possibly something along the lines of "in portrait mode" might be better understood?

[NiLuJe]

I guess that at this point, we could have 4 menu items :)

That's is indeed also an option (and perhaps easier on I18N ;)).

[poire-z]

So, latest changes give this (showing the options with their default value - my explanation of why as comment in the code):

@NiLuJe : I'm still puzzled about the rotation_angle = rotate_clockwise and 270 or 90.
It gives the expected behaviour - but I don't really understand why :/ Any insight ? (imagewidget.lua, blitbuffer.lua, the rotation mode being 1 when 90° should be the one rotating the image clockwise...)

[NiLuJe]

I'm still puzzled about the rotation_angle = rotate_clockwise and 270 or 90.

Because BB is talking in terms of device rotation (i.e., àla LinuxFB), which is the inverse of the effective buffer rotation.

Which means, now that I've tried it, I still have some issues:

• The default (for Portrait, at least, unsure about landscape) has changed. I... don't like it on principle. (I also happen to prefer the old default, but that's mostly irrelevant to my concern).
• The fact that this is right next to the Screen rotation stuff that is expressed in terms of device rotation, while this one doesn't, is extremely confusing. I have a really hard time making sense of what it says vs. what it does. (i.e., it does the exact opposite of what I would be expecting, going in expecting device rotations).

Even in this context, I find talking in terms of "if I rotate the device this way" much clearer to visualize than groking that "oh, wait, the image itself needs to rotate the other way around, whut?!". Possibly because that's the thing I'm actually holding and rotating, which makes it way more tangible? That may or may not be just me, so RFC?

[NiLuJe]

• I have a really hard time making sense of what it says vs. what it does. (i.e., it does the exact opposite of what I would be expecting, going in expecting device rotations).

e.g., I'd much prefer it displayed as "Match (or Align with) a CW rotated device" or... something.

[NiLuJe]

TL;DR: Despite my personal bias towards expressing this sort of stuff in terms of device rotation, always and forever; I'd still categorize this as cognitive dissonance, compounded by being right next to the screen rotation stuff ;p.

But, as I said above, this might be highly subjective, and I do have a strong bias towards a specific way of approaching this, so, yeah, curious as to how others see this ;).

[mergen3107]

I strongly agree with @NiLuJe,

I don't really care about absolute meaning, but they should be consistent with each other. I can figure it out which way it actually rotates, but UX wise it would be nice if the user needs to do that once :D

[NiLuJe]

As another example addressing both the "BB rotation is the inverse of device rotation" and "I prefer the old default" concerns, in my CBZ processing, I duplicate double spreads: I keep the split pages, and then append a landscape copy.

I prefer to read landscape stuff with my device rotated CW... that means the image needs to be rotated CCW, ergo, the first argument to ImageMagick is -rotate '-90>' ;).

[poire-z]

Because BB is talking in terms of device rotation (i.e., àla LinuxFB), which is the inverse of the effective buffer rotation.

When looking at https://github.com/koreader/koreader-base/blob/48225b17ac2c313a37ae88378614269f33482b44/ffi/blitbuffer.lua#L731-L736, this makes no sense.
But ok, accepting that I don't get it and stoping worrying and loving the bomb :)

• The default (for Portrait, at least, unsure about landscape) has changed. I... don't like it on principle. (I also happen to prefer the old default, but that's mostly irrelevant to my concern).

You started it all though :/ #10540 (comment) at my previous comment where the screenshot shows stuff the way you preferred it...

e.g., I'd much prefer it displayed as "Match (or Align with) a CW rotated device" or... something.

Uh... Even more twisted and more gymnastic for my brain...
At least, read what is written on the tin and act according to that - instead of assuming it works the way you are used to it.
(It's not something new, it's because of that and 50% of people not getting the old icons and not undersdtanding the way the other 50% of people think about it and see no problem at all :), that we went with the bottom menu icons :) which made it all simpler...)

I don't really care about absolute meaning, but they should be consistent with each other. I can figure it out which way it actually rotates, but UX wise it would be nice if the user needs to do that once :D

Consistent in what ? Just the circle arrows ? That you as a user would just check all the circles that go the same way because the first ones you set it that way worked for you, and you should be satistified ?
Unlike all the others, this one rotates an image: it doesn't rotate a widget or the screen: you'd still have the ImageViewer bottom button in the original orientation. So for me, expressing it the way you wish it feels twicely odd...

I prefer to read landscape stuff with my device rotated CW

That means with the thick bottom of the device in your left hand ?
Odd, but ok. So, my appreciation of what is best for right handed people (what are you btw?) preferring the thick bottom in the right hand may be wrong.

Well, given the complexity in perception of how this works/should work/should be written/what's the default :), I would either go with:

[x] Rotate some way
[ ] Rotate the other way

:), or a single

[ ] Inverse default rotation in portrait mode
[ ] Inverse default rotation in landscape mode

(what ever you all think what should be the default, but we need some other opinion, because it's 1 to 1 for now ;))

Or keep the existing (odd to me) default, have no menu there mixed with the other (unrelated to me) rotation menu - and just have long-press on ImageViewer's No rotation (which is shown once it's rotated) bottom menu button rotate it the other way and save it as a hidden setting. It would be a little hidden, but you'll get instant feedback, instead of having it way far in a unrelated menu item.
(ImageViewer needs some room, we can't waste it with a top left hamburger menu with a checkbox, like we're adding to TextViewer.)

[NiLuJe]

Gotta run, will go through your answer more in a few hours ;). This is just a comment about the old defaults and why they were the way they were ;).

The old default makes sense if you think about devices with buttons (especially asymmetric devices, e.g., Oasis/Forma), which are designed in a right-handed layout when in Portrait (e.g., the buttons on the right). Assuming a bottom grip, which makes sense if you don't want your hands to obscure the screen, you kinda need the buttons to be on the bottom on the device when in Landscape, and that means rotating the device CW ;).

[NiLuJe]

You started it all though :/ #10540 (comment) at my previous comment where the screenshot shows stuff the way you preferred it...

My bad, that's not quite what I meant ^^. I suspected there was a device<->bb rotation confusion somewhere, but it turns out it's not quite where I expected it at the time ;).

I prefer to read landscape stuff with my device rotated CW

That means with the thick bottom of the device in your left hand ? Odd, but ok. So, my appreciation of what is best for right handed people (what are you btw?) preferring the thick bottom in the right hand may be wrong.

See my previous message, but, essentially, yes, the fat lip with buttons at the bottom (in... both hands technically, since the point is having access to the buttons without going over the screen. Or either if gripping it one-handed, the hand doesn't really matter, the position of the buttons does).
(And, yeah, I'm right-handed).

[poire-z]

The old default makes sense if you think about devices with buttons (especially asymmetric devices, e.g., Oasis/Forma), which are designed in a right-handed layout when in Portrait (e.g., the buttons on the right). Assuming a bottom grip, which makes sense if you don't want your hands to osbcure the screen, you kinda need the buttons to be on the bottom on the device when in Landscape, and that means rotating the device CW ;).

Finally something reasonable that makes sense - and that explains why my preferred way (which I thought should be the default) should not be the default :)
I guess this (thinking about old kindle with a keyboard/keys at portrait bottom) also justify the fact that when reading in landscape, the default should get the user to rotate the device as the default portrait, so the user got back his grip at bottom).

Half of the issues solved then :)

[NiLuJe]

old kindle with a keyboard

The K3 was the very rare beast with symmetric page turns buttons, which is probably what drove someone to make this configurable in the first place. I think some later models switched to a slightly more asymmetric button layout, with Menu/Back on the left, and Forward/Back on the right.

But, yeah, for my own experience, the nail in the coffin was most likely the Forma, which is very asymmetric ;).

[poire-z]

I guess I have to keep the menu there because of the Auto-rotate 3rd item....

So, probably best to keep the existing default behaviours (rotate in portrait so that people with Libra&co thick-right-side-with-buttons have to turn it so that thick side is at the bottom - and the other way around when in landscape), and with this menu:

[ ] Inverse default rotation in portrait mode
[ ] Inverse default rotation in landscape mode
----
[ ] Auto-rotate for best fit

so we don't get 50% of people confused with the meaning of "clockwise" or a circle-arrow.

[NiLuJe]

I'm not going to go over the whole thing again because I understand it's probably tied to how people view objects in space of something, so instead I'm going to try to focus on what we can do about ;).

I don't necessarily have an issue with the current wording in a vacuum (even if my brain would grok it better the other way around), but I do have an issue with the cognitive dissonance it creates because of where it is, right next to the screen rota stuff, which is something I hadn't thought of yesterday.

So, what can we do about it?

• Given the proximity to the screen stuff, which uses the 180° arrows too, I now think those arrows actually reinforce the dissonance here. I think I'd be pretty happy with the icons we use for the bottom menu rotations instead (I can probably whip up a variant that uses the "mountain + sun in a landscape frame" usual pictogram for "this is an image" instead of the "A", if necessary).
• Depending on how well that works with the current wording or not, yeah, something based on "invert" like what you proposed could work, I guess ;).
• And, yeah, I don't really have a better idea of where to move this than where it already is anyway ;).

[poire-z]

• I think I'd be pretty happy with the icons we use for the bottom menu rotations instead (I can probably whip up a variant that uses the "mountain + sun in a landscape frame" usual pictogram for "this is an image" instead of the "A", if necessary).

We don't have code to display icons in TouchMenu items (and even less for embedding icons among text :)). Otherwise, I would have summoned you for such icons :)
That's why I had to look in the fonts for some glyphs.
No real need to go at making such code, we don't really need if we can go with "Invert default..." and we would hardly need that for anything else.

Depending on how well that works with the current wording or not, yeah, something based on "invert" like what you proposed could work, I guess ;).

I guess so :) it's also burried deep in the menu, so we don't need a foolproof wording. (I guess some people may read it as "invert the default I have set in the upper menu :) dunno if there's a notion of default among them - I don't see them all as you do high-tech device owners :)

[NiLuJe]

We don't have code to display icons in TouchMenu items (and even less for embedding icons among text :)). Otherwise, I would have summoned you for such icons :)

Oh, right, crap.

We don't have code for that anywhere else that would basically be mixed with a TextWidget either, right?

[mergen3107]

Why don't we put these settings (rotationg +90 or -90 deg) as a button right in the ImageViewer itself?

This way, users can just rotate the image on spot, seeing the result immediately. And if the settings stays after exiting from Image Viewer, they would need to do this only first time.

[mergen3107]

In this case even wording is not required, users will see the difference between "-90" and "+90" right there :D

[NiLuJe]

@poire-z mentioned something along those lines earlier:

Or keep the existing (odd to me) default, have no menu there mixed with the other (unrelated to me) rotation menu - and just have long-press on ImageViewer's No rotation (which is shown once it's rotated) bottom menu button rotate it the other way and save it as a hidden setting. It would be a little hidden, but you'll get instant feedback, instead of having it way far in a unrelated menu item.
(ImageViewer needs some room, we can't waste it with a top left hamburger menu with a checkbox, like we're adding to TextViewer.)

I... don't recall if there aren't some contexts where we have more buttons in ImageViewer, but, yeah, adding two small icons on each edge of the bottom bar in place of the center "rotate/no rotation" one certainly seems to make some sense on paper.

Going this route still leaves the question of what to do about the default behavior (and remembering it), though.

[poire-z]

Why don't we put these settings (rotationg +90 or -90 deg) as a button right in the ImageViewer itself?
In this case even wording is not required, users will see the difference between "-90" and "+90" right there :D

There will still be 50% of people who will think: blah, these buttons both rotate the other way that what's written on their tin :)

This way, users can just rotate the image on spot, seeing the result immediately. And if the settings stays after exiting from Image Viewer, they would need to do this only first time.

adding two small icons on each edge of the bottom bar in place of the center "rotate/no rotation" one certainly seems to make some sense on paper.

Not sure I'd like the busy'ness and seeing the other button I will never use, and having to remember the one I want is the one on the left (or on the right).
People will stick to one rotation, they don't really need to think: oh, it's better if I rotate it that way for this image, the other way for that other image - or, yep, it's evening, let's rotate it the other way for a change.

We don't have code for that anywhere else that would basically be mixed with a TextWidget either, right?

Probably, but it's with the help of SomewayContainer, OverlapSomething, HorizontalGroup, that we are happy not having to think about in TouchMenu items :) Let's really forget it.

We still need a menu for the "auto-rotate for best fit toggle", so if you really don't want it in the upper menu, it could go as a popup in the bottom menu - but franckly, as long I we don't need more features in ImageViewer, I'd rather not hack around the friendly big 3 bottom buttons.

[mergen3107]

OK, how about "rotate", "anti-rotate"? :D just to express the action and its counterpart.

Also, we could implement a hamburger menu and put all auto-rotation checks and rotation buttons there.

So, there would be "manual rotation", "manual anti-rotation", "auto-rotation check mark with rotation", "auto-rotation check mark with anti-rotation" (these two mutually exclusive, of course).
Rotation check marks' infomessage would explain that this would only apply if X>Y of the image.

[mergen3107]

Alternatively:

1. Move "Close button" to the top as an "X".
2. Keep the "original size" button in the middle.
3. Surround that button with "(manual) rotate" and "(manual) anti-rotate" buttons.
4. These are only manual ones. All auto-rotation options would be in the hamburger menu.

Back to NiLuJe's question about what should be as default - I keep my old stance that any new feature should stay "off" by default. However, you guys might argue that in this case very few people would notice and use it. So we can make it "on" by default to any rotation direction and wait for people to come here and complain :D

(On the other note, I was wondering whether there should be a summary "What's new" info windows when someone upgraded KOReader to a new milestone version? This way we might insure people are well informed about the new features. I found myself wondering so many times like "woah, this is a new feature!", especially at times when I had no extra time to follow GH :D)

[poire-z]

Alternatively:
1. Move "Close button" to the top as an "X".
2. Keep the "original size" button in the middle.
3. Surround that button with "(manual) rotate" and "(manual) anti-rotate" buttons.

What about additionally some title vignettes above and below the image (and on the left & right when in landscape), so we see even less of the image ? :)

Anyway, there won't be any change from the current behaviour (which felt odd to me, but that I can now accept as the default as it's what is best for devices with buttons and thick side on the rigth of the devices).
No auto rotate by default, and the current default rotations will be kept.
There will just be this new menu item, with 2 checkboxes to invert these default rotations, without having to go to Advanced settings and update DLANDSCAPE_CLOCKWISE_ROTATION = true to false (for users like me that don't have a device with that thickside on the right, but a thick side without buttons at the bottom that felt better hold in my right hand).
Plus a checkbox off by default to get auto-rotate for best fit.

[mergen3107]

so we see even less of the image ? :)

1. Image shows up in fullscreen, no buttons with ImageViewer startup anyway
2. Buttons can be drawn on top without resizing the image. So, see image full screen - tap, change settings - tap again to go back to fullscreen.

[NiLuJe]

Random 5AM thought: Should this warrant a one-time migration block? (To both migrate to the new setting, and drop the now unnecessary entry from G_defaults, if any).

[poire-z]

We drop it from defaults.lua - dunno how/if we should drop it from defaults.custom.lua.
(Given that I have manually updated my defaults.custom.lua to put in it comments I had in my defaults.persistent at the time, I would not be happy if it is auto updated/cleaned and my comments get lost. I of course got a backup, but I don't think I had yet had to restore it).

I'd say that most people have not updated it - and those who did were so bothered by the unnatural rotation :) that they will witness it - and by going into the menu, they will notice the "auto-rotate" item and test it, and may be like it (like I did) - instead of just no noticing anything if we do automigration.

[NiLuJe]

Reviewed 1 of 3 files at r1, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @poire-z)

[mangonerd]

I’ve been wanting this feature for so long! But now that it’s finally here, it doesn’t seem to be working.
I’m using a .cbz file and there are some landscape pages I assume would automatically rotate, but nada.
I’m on a Kobo Sage.
Am I doing something wrong?

[poire-z]

It's implemented in our "ImageViewer" widget (which has Original size | Rotate | Close buttons at its bottom), the one that shows image when 1) long-press on it in an EPUB - 2) long-press on a book cover in filebrowser and View cover - 3) long-press on an image in a Wikipedia lookup result - 4) when doing/viewing screenshots - 5) when Open with... on a document and selecting ImageViewer.

I don't think it's available when reading a PDF/CBZ - and what you see when reading it, whether it is text or image content, is just the regular pages rendered by our PDF engine. This feature is not about that.

[mergen3107]

@mangonerd
Why don't you "convert" CBZ -> CBZ using KCC with autorotation checked? I solely use that feature and it has never let me down yet.

P.S. Cool nickname by the way :D

[mangonerd]

@poire-z
Ah, got it. Being a new user, there’s still much I don’t understand. I did notice that when zooming in on a panel, it auto rotates, so that’s why. Too bad :)

@mergen3107
Yes, that is an option. It’s just that it’s an additional step and having it built in would be really convenient. Thanks for the suggestion though!