koreader / kindlepdfviewer

(DEPRECATED, please use KOReader instead) A PDF (plus DJVU, ePub, TXT, CHM, FB2, HTML...) viewer made for e-ink framebuffer devices, using muPDF, djvulibre, crengine
GNU General Public License v3.0
499 stars 98 forks source link

making rotate screen mode a saved configuration specific to book #573

Open dracodoc opened 11 years ago

dracodoc commented 11 years ago

I found that when I rotate screen 90 degree for a book, pressing home will actually close book but the display is not updated correctly. Five way button will move a long landscape orientation selection line above the current book page, but not really usable. Another pressing Home will quit KPV.

So it seemed pressing home in rotated mode is not processed correctly. I think KPV should rotate screen back to normal and return to file browser when Home is pressed, since operating file browser in landscaping mode is not convenient and pointless. .

Besides, every time user open a book suitable for landscape reading, he need to rotate screen manually. So I suggest to make rotated screen as a a saved configuration specific to book

  1. When user rotated screen to read book A, directly closing book will return to regular portrait mode file browser.
  2. The rotated screen configuration(include screen orientation and zoom mode) is saved in book A history like other history information, so next time when user open book A, the screen will rotate to last reading mode automatically, user can start read immediately without any interference.
  3. The usual bookmark or last reading position may be based on page number only, but landscaping mode with fit width view may need to save more detailed last reading position. This involves more effort but it should be done, otherwise user need to flip through to find the exact last reading position. I believe this kind of detailed reading position is also saved already in fit content width mode.

Whether to rotate screen to read is always specific to each book, with this design, each book only need to be manually rotated for one time if needed, then user will never need to adjust screen orientation manually again.

tigran123 commented 11 years ago

I think we should fix the bug reported earlier: filechooser does not work in landscape mode. After this bug is fixed we could implement some of the useful suggestions you are making here.

And I don't think that landscape mode is useless for filechooser. Some may prefer it because it has a bigger chance of showing the full filename (some people give very long filenames to their books :)

dracodoc commented 11 years ago

Yes, there are two methods to solve these problems:

  1. make filechooser work in landscape mode. And it need additional efforts to save screen orientation.
  2. Just save screen orientation and always operation filechooser in landscape mode. So one step will solve two problems.

Will anybody really operate filechooser in landscape mode? How can he use the keyboard in landscape mode? Sony 505 have a landscape mode which will rotate all the operations, I found it difficult to operate, and 505 don't have too many keys. I think it'll be impossible to input something with keyboard in landscape mode.

As for long file name, pressing right can show all the details of that file, and the showed first part of long file name should be enough to recognize different files.

tigran123 commented 11 years ago

Yes, I personally prefer the info shown by Right key when I need to see the full filename.

Also, I personally prefer forcing filechooser into portrait mode as well. But to be consistent this should also be done with Last Documents dialog. But Last Documents is sensitive to orientation and works in landscape mode.

dracodoc commented 11 years ago

This Last Document is really special! This is my test with kindle DX:

  1. The gravity sensor used to interfere with KPV when quitting, this has been solved. KPV now just ignore the gravity sensor, this is all right to me, I personally always turn it off.
  2. As I said before, with screen rotated, pressing home will quit current book but the display is messed up, another pressing home will quit KPV.
  3. If the book is open from Last Document, with screen rotated, pressing home will quit current book and return to Last Document in Landscape mode, further it can return to filechooser in Landscape mode! Although the display didn't update correctly, move the cursor and press OK will select a book in that location as if the filechooser is there but not shown correctly. The book will be opened in Landscape mode. However, just like 2. quit book will have messed up filechooser again.

I think the Landscape mode of Last Document should be removed too, as it is not working fully, and it breaks other operations. Just force all file operations in portrait mode is better, simpler and more logical.

tigran123 commented 11 years ago

I agree with all these suggestions. But obviously I can't speak for other KPV developers.

kkzxak47 commented 11 years ago

Just force all file operations in portrait mode is better, simpler and more logical.

That's the right thing to do. At least andscape mode in filechooser is not an exciting feature to me.

kai771 commented 11 years ago

If it's any help, I also agree with dracodoc's suggestions.

tigran123 commented 11 years ago

The only tricky part about implementing all these suggestions is that they cannot be tested on the emulator, i.e. the entire development/testing cycle has to be done on the Kindle hardware...

kai771 commented 11 years ago

Or we could change UniReader:screenRotate() to check if the emulator is running, and simulate the behaviour for the emulator if it does (I guess disposing and making a new fb would be needed?). And no, don't look at me... I don't know how to do that.

Update: Actually, maybe the check should be in Screen:screenRotate()... or maybe the check isn't needed, just some code that's missing in einkSetOrientation() in einkfb.c (it just returns if it's running in emulator). Anyway, I do think the emulator should be updated so it would be able to simulate orientation change too.

dracodoc commented 11 years ago

Sorry I don't understand, which part have to be tested on hardware?

  1. KPV just ignore kindle's gravity sensor, which is already done.
  2. all the operations in rotated screen in reading mode are done too, no need to change.
  3. The changes will be:

A. Remove Last Document's landscape mode, just make it as regular portrait. B. When closing books in landscape mode, always rotate screen back to portrait first. C. Save book rotation information to history, rotate book automatically next time.

I think all the stuff here can be based on code that already proven, right? The changes in B, C can even be simulated by keyboard events.

tigran123 commented 11 years ago

Unfortunately, not. It is a lot more complex than that, because of the ties to Amazon's "native screen mode" which is controlled from outside KPV. So, yes, we will need to throw away lots of code from KPV but we need to understand what is going on carefully before we start throwing code away (so as not to throw away that which is needed :)

And none of this stuff works on the emulator, so we'll have to test everything directly on Kindle hardware.

dracodoc commented 11 years ago

OK, I got it.

There is a simpler method but with big flaws: when rotating screens, don't use the native screen mode, just rotate the displayed image itself, and keep other things in portrait mode.

All the keyboard shortcut still work naturally except the five way rocker, but any pop up dialog will be displayed in portrait mode.

Of course the zoom mode need to be remapped in this mode too. A possible method is just redefine the screen resolution to the swapped landscape resolution, and all the other calculation about display should work if they are abstracted correctly.

This is definitely not perfect, but it is acceptable for me.

tigran123 commented 11 years ago

Hmmm, I think we should do things properly. The worst mistake during programming is changing code one doesn't fully understand. So, if/when I learn enough about orientation support in KPV to implement these changes I'll do it. But this does not preclude others (@houqp @dpavlin @hwhw ) who probably understand it already (as they wrote this stuff) from doing it if they agree with the basic idea of forcing filemanager (inc history, searches, etc) into portrait mode.

houqp commented 11 years ago

I have no objection to disable landscape mode in filechooser. But it is not a easy task (at least for me) in the old code base ;p

Because after we physically rotated the screen (native screen mode), the framebuffer will interpret the bitmap differently, thus we need to tweak the bitmap when drawing filechooser before sending it to framebuffer. Well, I think with proper abstraction on KPV's eink control code, it might be not that difficult to achieve. But still, you need to rewrite stuffs because of the abstraction you introduced...

houqp commented 11 years ago

And yes, I definitely agree with @dracodoc that rotate screen mode configuration should be saved for each book.

dracodoc commented 11 years ago

Then maybe just rotate the screen image instead of using native screen mode will be easier? Because the rotation will basically just introduce a new zoom mode, and nothing else will be changed.

Of course, the pop up dialog will still be in portrait mode, but I can live with that, because user may have to rotate the kindle to portrait to use the keyboard anyway, then it's actually very natural to have portrait pop up dialogs.

houqp commented 11 years ago

@dracodoc the rotate screen image approach is better in the sense that it can avoid interacting with the native amazon system when rotating the screen, thus eliminating all these race and glitch screen bugs (which is really annoying and looks scary for users).

But it still have the problem I mentioned above, i.e. now the native screen mode will always be portrait mode, but when we want to display landscape image, we need to adjust the bitmap because framebuffer will always think the bitmap is generated for portrait mode.

Seems that @hwhw also want to approach in the same way in the new code base, I saw related code but I am not sure. Still I introduced the low-level rotate approach to the new code base later because I cannot not come up with a good abstraction for bitmap operation.

Maybe it is not that hard to implement it, I might missed something here...

dracodoc commented 11 years ago

Yes it could involve more subtle changes but they are in a good direction. Because this method is future proof, KPV will not need to handle possible new native screen problems from newer models.

And the reasoning that user could actually prefer operating keyboard under portrait mode is another plus.

houqp commented 11 years ago

Yes, totally agree with you.

So if someone make it I will be very happy :) I am just not a graphic guy. I didn't even know what bitmap is before hacking on KPV ;p

tigran123 commented 11 years ago

As a conservative voice I suggest that we should not rush with implementing it, but explore all the options first.

In any case, we should make a stable release first, I think, before doing something as potentially destablizing as the changes described above. At the moment KPV is so nice and stable that I fear to destabilize it...

houqp commented 11 years ago

Yeah, so let's do it in the new code base before it goes stable in the future ;p

kai771 commented 11 years ago

@tigran123 and @houqp Too late, I already did it ;). Now go and test :P.

kai771 commented 11 years ago

If anyone is interested, rotation mode is saved per book in Librerator. Opening a book will open it exactly as you left it when you closed it. Portrait is forced in FileChooser by default, but that can be changed editing defaults.lua too.