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
498 stars 98 forks source link

Suggestion: kpdfdjview = lightweight version of kindlepdfviewer with only PDF/DjVu support. #274

Closed tigran123 closed 11 years ago

tigran123 commented 11 years ago

I have various ideas which are not only unlikely to be accepted into the main kindlepdfviewer source tree, but I would not even desire for [some of] them to be accepted, because they imply removing some features which are, no doubt, useful to some people.

Therefore, I suggest to add a lightweight version of kindlepdfviewer which is stripped of crengine and any other features not strictly necessary for optimal PDF and DjVu viewing. I have already started this work and it lives in my "experimental" branch now, so the proof of concept is here.

Before we consider "advantages and disadvantages", first of all, what exactly do I suggest? I suggest having a new branch called "kpdfdjview" which will have a version of kindlepdfviewer which is optimized for PDF and DjVu documents only and designed for an advanced user. But the main kindlepdfviewer development will be tracked (I am happy to do this) and all bugfixes and useful (for PDF/DjVu) features will be merged into it.

The advantages are (in random order, except item 0):

  1. Most importantly, a smaller program can be analyzed and polished to perfection, whereas a large system supporting all conceivable file formats can not.
  2. Smaller memory footprint.
  3. Fewer writes to flash memory in the log (crengine can generate 1000 messages/second if, e.g., cache area is not there and it wants to swap).
  4. Some limitations of the program dependent on crengine can be removed, e.g. filemanager can display all files because none of them is going to be fed to crengine which can crash our program. I was planning on execing external mcview (via myts as an fb-aware terminal) as a generic viewer --- this properly belongs to filemanager.

What are the disadvantages?

  1. kpdfdjview will not support file formats other than PDF and DjVu. But neither would it claim to do so, so it is not a disadvantage.
  2. For the person(s) maintaining kpdfdjview it would be quite tempting to fix bugs in just the kpdfdjview branch and not bother with the master kindlepdfviewer, especially when the solution in the master tree would involve crengine-related extra work. However, yesterday when fixing the TOC crash I did carefully check the crengine side and modify it accordingly and only afterwards merged the appropriate (non-crengine) part of the fix into "experimental". So, a disciplined person would always do "the right thing", i.e. if the bug exists in the master tree --- fix it properly there and only then merge it (or part of it) into kpdfdjview.

The reasons I seek your (senior maintainers: @hwhw @houqp @dpavlin ) blessings are mainly ethical. If I just put kpdfdjview release on my Wiki Download page it would look as if I seek credit for something I have not created (as most of the good stuff in kpdfdjview obviously would have come originally from kindlepdfviewer). But if it comes from the generic kindlepdfviewer's Wiki Download (as a separate section perhaps) then all the credit properly belongs to the whole kindlepdfviewer team.

Alternatively, if you prefer I can put it on my Wiki Download page with the appropriate disclaimer that it is a lightweight PDF/DjVu-only version of kindlepdfviewer, maintained in parallel AND CO-EXISTENT with kindlepdfviewer on the same device.

What do you guys think? Yay or nay?

houqp commented 11 years ago

I am OK with this. Since it is a freesoftware, do what ever you think that can improve the user experience :)

My question is are you proposing adding the kpdfdjview branch to HW's repo?

dpavlin commented 11 years ago

I do have gripes with crengine (mobi support is inferior to epub), but to be honest, I spend half of my reader usage reading some of crengine formats. I guess that first step might be to update crengine to current upstream and try to persuade @tigran123 that it's useful :-)

tigran123 commented 11 years ago

@houqp Well, at the moment kpdfdjview exists as "experimental" branch in my local git repository. But this way you don't see what is going on there. I don't know how to create remote branches (like new_ui etc in HW's repository), but I found a tutorial on git-scm.com which may explain it. But if you prefer for this to stay in my local "experimental" branch, that is fine by me. I'll do what you think is better, i.e. friendly to other developers.

@dpavlin Oh, you don't need to convince me that it is useful, I totally agree that it is, and I too spend MORE than half of my reader usage time in CoolReader. However, let me try to persuade you instead :)

You are "a Unix addict" (as you say in your profile). Good, a man after my own heart indeed, for so am I! Now, as a Unix addict you remember the Unix philosophy very well, namely that all commands are small utilities that only do one job, and do it well (like cp doesn't try to rename files and mv doesn't assist in creating tar archives). This is why each utility has been polished into perfection, by doing only one job and doing it well. And there already exists a standalone CoolReader which, imho, does it job very well, so why adding a builtin crengine into KPV when there is a standalone CoolReader which is adequate? This is like trying to teach mv to create tar archives or to rename files inside tar archives. Now, to persuade me in a different direction you have to give me a good example of where crengine can do things which standalone CoolReader cannot (btw, continuous scrolling is not a good example because it is useful only when the floating picture is split across pages, but CoolReader doesn't allow this to happen anyway)

When I tested the screensaver hack I noticed something very peculiar --- with the hack installed, the first open of VERY LARGE files (small ones seemed ok, maybe because they don't go through mmap(2)?) in CoolReader became 5-6 times slower. I narrowed it down to the fact that the screensaver hack does mount --bind to fool the framework into using user's images for screensavers. This is very strange and unexplained (though I am a Linux kernel hacker, I don't understand why binding a mountpoint would delay accessing mmap'ed files by a factor of 5). Nevertheless, not doing mount --bind solved the performance of CoolReader. Now, in kpdf.sh we also do "mount --bind" on the fonts directory. I removed this in kpdfdjview.sh as "potentially dangerous". This is just a tiny example of where kpdfdjview is "safer, and potentially more stable".

tigran123 commented 11 years ago

I put the first build of kpdfdjview in my Downloads section.

dpavlin commented 11 years ago

You can create remote branch with simple git push upstream name_of_new_branch. If you don't specify branch name, only existing branches in upstream will be updated.

tigran123 commented 11 years ago

@dpavlin Thank you, it works! Now in my repository on github there is a branch experimental, so everyone can see what is going on there.

houqp commented 11 years ago

I think it is better to have an official branch for kpdfdjview in @hwhw 's repo, so we can keep track of everything easier. So if you don't mind, I will push your branch to the official repo. But we should better rename that branch to something like kpdfdjview.

There is no harm as long as patches are sent back to upstream. And now user have one more choice, to run coolreader and kpdfdjview for different formats. Though I myself still prefer kindlepdfview because I am just too lazy to exit the program ;P

tigran123 commented 11 years ago

@houqp Yes, I completely agree. I named it "experimental" simply because I didn't know how to rename a remote branch :)

Should I somehow rename the local branch "experimental" -> "kpdfdjview" and then push it to hwhw/kpdfdjview? I will try to find out how to rename a local branch.

houqp commented 11 years ago

Just create an other branch when you are in the "experimental" branch, the new branch will be based on the "experimental" branch and you can start to work on that.

Did @hwhw add you as a collaborator for his repo?

tigran123 commented 11 years ago

@houqp Yes, I do have Read+Write access now. (so whenever I do pull request I see a new green button "this request can be merged automatically" but, of course, I defer it to you and @dpavlin as senior architects of this program to decide what should be "merged automatically" and what shouldn't :)

tigran123 commented 11 years ago

Strange, I did this:

$ git branch
  experimental
* kpdfdjview
  master
$ git push upstream kpdfdjview
fatal: 'upstream' does not appear to be a git repository
fatal: The remote end hung up unexpectedly

and I did execute in the "distant past" this:

git remote add upstream https://github.com/hwhw/kindlepdfviewer.git

so I do have "upstream" defined (and that is where I do "git fetch upstream" and "git merge upstream/master").

Do you have an idea what the problem might be?

tigran123 commented 11 years ago

The way I pushed experimental to my origin was:

git push origin experimental

so I don't see why a similar git push upstream kpdfdjview refuses to work...

houqp commented 11 years ago

Oh, that's because you use the https repo URL. Try adding the upstream repo like this:

git remote add upstream git@github.com:hwhw/kindlepdfviewer.git (fetch)

tigran123 commented 11 years ago

Btw, a bit offtopic, but I would like to mention that I have just upgraded by Kindle 3 from firmware 3.3 to 3.4 and everything seems to work still, i.e. it is still jailbroken and launchpad works and executes all applications fine (though I haven't tested every possible program in every possible way of course :)

houqp commented 11 years ago

great news. Are there any new features coming with 3.4?

tigran123 commented 11 years ago

Hmmm, now it tries to do the ssh keys thingy and dies again:

Permission denied (publickey).
fatal: The remote end hung up unexpectedly
tigran123 commented 11 years ago

They claim that they improved the default font that is used in the main (mobi/azw) viewer to make it "higher contrast and more crisp", that is all :) (the other so-called "improvements" I totally ignore, such as "parental controls" and "comic books" and other such commercial or/and "politically correct" nonsense).

tigran123 commented 11 years ago

Btw, I see no difference with the font, to be honest...

houqp commented 11 years ago

LOL

Try git push upstream kpdfdjview:kpdfdjview ?

tigran123 commented 11 years ago

dies again:

$ git push upstream2 kpdfdjview:kpdfdjview
Permission denied (publickey).
fatal: The remote end hung up unexpectedly

I named it upstream2 to keep my old upstream as I know fetch/merge work fine from it (I am very careful :) i.e. I executed your advice as:

git remote add upstream2 git@github.com:hwhw/kindlepdfviewer.git "(fetch)"
houqp commented 11 years ago

That's strange... I just try creating a test branch in @hwhw 's repo without any problem.

So now you can push to your own repo, but cannot push to HW's repo? Can you troubleshoot according to the instructions from this page?

houqp commented 11 years ago

OH, there should not be a (fetch), try add the remote repo again with:

git remote add upstream2 git@github.com:hwhw/kindlepdfviewer.git
tigran123 commented 11 years ago

Ok, thank you, succeeded now! In fact, I succeeded with my original command:

git push upstream kpdfdjview

The reason it was failing originally is because I had to execute "remote add" again:

git remote add upstream https://github.com/hwhw/kindlepdfviewer.git

because when I test building from clean state I completely remove my local repository and clone a new one from github and do the whole process (make fetchthirdparty, make thirdparty etc) from scratch. So, the list of remote repositories is only kept in the local repository, not in the remote and hence "git remote add" must be repeated if you clone a fresh one. So, now I removed the upstream2 as unnecessary.

houqp commented 11 years ago

I think this can be closed now. @tigran123 , would you mind adding kpdfdjview to the official wiki? (a link or some introduction or something else)

tigran123 commented 11 years ago

@houqp Ok, will do.