#79 inverted smart scroll isn't very smart

[asl97]

problem: the inverted smart scroll "jumps" over the center part of the page

when: it happen when the image is too long (width) or when the screen resolution isn't high enough

to show a example of the problem, i use this long(width) but short(height) image (attachment),
on my screen(1280x768), normal inverted scroll mode,
it show 1, 2 and 3(part*), when i press the space bar, it "jumps" to 7(part*), 8 and 9
skipping 4, 5 and 6, for manga mode, it "jumps" from 7,8,9 to 1,2,3

if attachment disappear:
http://i.imgur.com/mmA4m9t.png
for a real world example, this page should do: http://gallery.minitokyo.net/download/575195

* part of the number

[asl97]

now that i read what i posted, i think i wasn't very clear about what i mean by "when the screen resolution isn't high enough"

what i mean by that is that when the when the screen resolution isn't big, it make the program to skip over the center of the image for smaller image too,
if you still don't understand what i mean by that, just ignore the screen resolution part.

ps, it seem the normal smart scroll works fine, only the inverted smart scroll is effected by it.

[Oddegamra]

I can see what you mean. Inverted smart scroll started out as a little hack with the least amount of effort required as possible. What is does is simply reverse the distance MComix scrolls into X and Y direction. So, instead of scrolling left/right first, it scrolls down first. This doesn't change that the pivotal points still remain the same, i.e. after having scrolled down, it will jump right to the second page, which usually is the left/rightmost position, depending on normal mode/manga mode. I can imagine that on small screens, parts of the image would be skipped in the center. I'll look into splitting the inverse scrolling mode from the normal mode and then try to make it at least a bit smarter.

[Ark]

Talking about making it smarter, how about "merging" this feature request (which actually looks like a bug to me) with [#75]?

I just added axis remapping capabilities to scrolling.py. It would be cool to see this piece of code in action. I attached a piece of code for testing purposes.

However, there are still some bugs in my attempts to integrate scrolling.py and I don't wont to break anything. I am only used to branching in Git (to some extent) but not SVN. (I know that there is an SVN interface for Git but I am not used to it, either.) Because of that, I don't commit my experiments to the repository here for now.

[Oddegamra]

Concerning merging of tickets, I'd love to, but apparently, there is no such feature in this current version of the SF ticketing platform. In any case, I'd agree that this is more of a bug than something that should be considered a feature.

Great work on the scrolling code so far. I've been thinking about ways to best integrate the module into the main code for a while. Even if some things break, this would greatly help to get a better "feel" for changes and how they interface with the remaining code. Unfortunately, I have come up short until now, but I only recently started at a new job and am thus a bit short on time.

Well, and concerning branching, I'm not completely certain that it would even work for this repository. As far as I know, the repository has to be set-up in a certain way to allow for branches (i.e. it has to have trunk/branches/tags folders at the root level). Incidentally, I also only have a local Git repository that hooks into this SVN repo. If you happen to have a Github account, we can exchange experimental branches there. I usually keep both this repository and http://github.com/oddegamra/mcomix up-to-date. The only problem is that syncing code from Git and then pushing it onto the SVN repository changes Git commits, so other Git repositories often have to be kept in sync with the --force switch to overwrite local history.

[Ark]

Thank you for the reply.

I just fixed a few bugs ([r954] and [r955]). Maybe this already helps understanding the flaws in the current design. For example, it would be nice if EventHandler._flip_page looks like this:

def _flip_page(self, number_of_pages):
    self._extra_scroll_events = 0
    self._window.flip_page(number_of_pages)

That is, it would be nice to get rid of methods like MainWindow.next_page_fast_forward, MainWindow.previous_page etc. and only have a single method MainWindow.flip_page(self, number_of_pages) instead (which itself calls MainWindow.set_page for example). The thing is that these old methods merely differ in (non-trvial) constants! And we should eliminate constants as soon as possible. (For example, see [r955] event.py:71 and event.py:74, I just introduced 10 and -10, respectively. I should go and replace them with something like SEVERAL_PAGES and -SEVERAL_PAGES, respectively.)

However, there are some pieces of code still depending on some of these methods (e.g. ui.py:39), so we need to get rid of these calls first. However, if you want to keep these methods, we should change them to do nothing but call another method. (Presumably, the new implementation will pass a constant to the other method, so, again, its mere purpose is to get rid of the constant.)

About branching: I don't know whether or when I will sign up to GitHub, so I hope it's okay if I attach patches and/or files in this thread instead. They might need some discussion anyway. (I wonder if it's possible to turn this SVN repo to a Git repo.)

PS: It happend again. I got mail for r955 but not for r954. Strange. EDIT: I just got back to my computer and I finally received the mail.

[Ark]

OT: I'm not a Windows user so it doesn't really bother me anyway. How far does the following actually affect MComix? http://www.gluster.org/2013/08/how-far-the-once-mighty-sourceforge-has-fallen/

[Oddegamra]

From what I understand, the installer is provided by SF on a purely opt-in basis, as configured by the project administrator. Filezilla agreed to this knowing that their software would be bundled up with other, less savory components for a small monetary compensation. Well, if they wanted to ruin the Filezilla reputation, they did so splendidly. I considered moving away from SF when I read about it, but it's probably more work than it's worth at this point. At least until SF decides to roll out their installer software for everyone without asking for permission first.

[asl97]

i already move from github to sourceforge only to move to google code
(cause github need https and so does sourceforge to see the codes)
that can easily browse the newest code which i use for my inbuilt update system without ssl

moving is very easy,
first, make a repo in the place where you want to move to,
second, clone the empty repo on to your computer,
third, copy the files from the local repo to the new empty repo,
fourth, "git add ." to add all the files in the new repo,
fifth , "git commit -a -m 'move from * to here'" to commit,
and last, "git push" and a whole lot of waiting

and it's done :D

[asl97]

i wonder how long would the repo on fedora (and other linux) take to get updated to the newest xcomix once this bug get fix...

[Ark]

Thanks for the explanations.

I identified the following major steps and pitfalls to consider when implementing the smarter scrolling:

1. Given a 1-dimensional arrangement of n-dimensional non-overlapping boxes S (=a sequence of images) and an intersecting n-dimensional box V (=a viewport), what is the intersection of S and V? (Since we have to deal with plain images, n must be 2 here.) Actually, I already coded something that calculates that. The result says which part of which image is located in which part of the viewport.
2. This is needed to calculate which image the user is currently looking at. I don't know how to calculate that right now. However, it can't be difficult since viewers such as Evince are capable of calculating this.
3. Now that we know what the reader is looking at, we remove all the other images from the next calculation. Thus, we might need to adjust the scrollbar values, move the origin to the upper left corner of the image etc. so we can perform the calculations as if there are no other images.
4. This step is easy: We simply let scrolling.py calculate the new position of the viewport.
5. Then we integrate the result to the actual viewport that contains the other images, i.e. switch back to the former origin, readjust the scrollbars accordingly etc.

However, there might be at least the following things to consider:

• We must ensure that the last two steps do not accidentally change the image we calculated in step 2. For instance, if the reader is looking at image A, we must ensure that the new viewport position means that the reader is still looking at A.
• However, if (and only if) the reader is about to turn the page (thanks to the scrolling mechanism and because the reader really is at the end of page A), we must turn to page B (and, of course, reflect this by moving the viewport accordingly).
• In order to handle several images in a row (not necessarily only 2), we might need to perform several page turns at once. For instance, if there are 10 images in a row and the first three of them are completely visible through the reader's viewport, hitting the space bar once should take the reader to the fourth, fifth and sixth image immediately. Another approach would be moving forward only one image per keystroke. Yet another idea: It should be up to the user. But I think this is merely a detail right now.

Maybe the actual calculations are not as complicated as I think and/or they are already implemented in some OSS. Any way, I attached some ugly Java code that performs some of the calculations we might need. They might also be interesting for [#63].

Comments are welcome.

[asl97]

java?
surely it possible to code it in python, so far, all the java apps i use always seem to crash.

also, how would you link java code into a python app and wouldn't linking different language cause slow down due to the headers to start both the java and python stuff?

edit: oh my, it the second page :D

[Oddegamra]

also, how would you link java code into a python app and wouldn't linking different language cause slow down due to the headers to start both the java and python stuff?

I'm pretty sure he only posted the code for reference, not for actual inclusion.

Maybe the actual calculations are not as complicated as I think and/or they are already implemented in some OSS. Any way, I attached some ugly Java code that performs some of the calculations we might need. They might also be interesting for [#63].

Great idea for a general algorithm. Ignoring all images other than the "current" one is especially good, since this essentially reduces the complexity to one page at a time. Concerning item 2, it might not be necessary to do much calculation to determine the image the user is looking at. How about the "active" image is defined simply as the image that takes up more space in the viewport? Since image/viewport position and sizes are known, we could just take the visible width of image 1 and 2 and take the larger of the two. This likely works best for images of about equal sizes. If the user loads a 100x100 next to a 1x1 image (in the worst case), this would of course invalidate possibility of scrolling image 2...

We could also keep track of pages that have already been scrolled down completely, and thus always scroll one page at a time and increment the counter when the next image is scrolled into view, or decrement the counter if the user steps backwards. The problem to this approach is that the user could just use his mouse to move the picture. When he uses the smart scroll key the next time, the image would revert to the last scrolled position, which would likely make little sense.

I could also imagine combining both approaches, i.e. use an internal counter for normal scrolling, reset the counter when the user uses his mouse or the arrow keys to move the image, and then determine the active image simply by the larger size when he goes back to using the smart scroll key.

[Ark]

I'm pretty sure he only posted the code for reference, not for actual inclusion.

Yep.

How about the "active" image is defined simply as the image that takes up more space in the viewport?

This is not sufficient. If you have two images of equal size, both of them are way greater than the viewport (each dimension), the viewport has an even number of pixels in each dimension and you scroll to the center of the "combined" image (that is, the area of the center-right part of the left image equals the area of the center-left part of the right), you simply cannot decide which one to take.

In these cases, we should use the reading direction (western or manga) in order to make a decision, I think. That is, we should tend to choose the image that appears earlier in the book. In the example above, we should take the left image if we are in western mode, and we should use the right image if we are in manga mode.

But even this way, choosing the image that consumes more space simply doesn't work. Consider case A in the attachment (manga mode). The first (purple) image consumes less space than the second (green) image. Thus, smart scrolling will continue on the second image and the user will never see the lower right areas of the first image if he/she uses only smart scrolling.

So I think it is a good idea to not rely on counters or similar "stateful" things. Instead, smart scrolling should work everywhere and behave the same way regardless of how we got into the current set-up of reading direction, viewport and (countless) images. And, of course, if the user uses only smart scrolling, he or she should see every part of every image, smart scrolling always advances towards the end of the book and never enters loops etc.

In order to do so, I think now that the current image should be the one that is closest to the center of the viewport. (I wonder if other viewers such as Evince behave the same way.) This, however, implies that you can scroll in a way that every image gets a chance to be the closest one.

A very simple approach works this way: The area that is "visitable" by scrolling is expanded so that every corner of every image can become the center of the viewport. This is illustrated in figure B. This approach has the advantage that, no matter how big the image is, you can always scroll to an "uncovered" (i.e. background-only) area. In figure B, if the user scrolls towards north, MComix could dynamically load the previous two pages. This way, it should be easy to combine this with a "continuous" mode (see [#63]).

However, if the user does not want to use or cannot use a continuous mode (that's how it currently is, obviously), it might be better to shrink the visitable area to what is really needed for the closest-to-center approach. This is shown in figure C. In order to calculate the visitable area for this case, we need to do the following:

1. For each image: Calculate the area that is visitable as if this image is the only image that is visible.
2. Combine these areas to obtain the axis-aligned minimum bounding box.

The drawback of this closest-to-center approach, however, is that double page mode will not work the same way it currently does.

Well, I just committed some code that we might need in order to implement the closest-to-center approach, see [r956]. Any questions, suggestions, comments etc.? What to do next?

[Ark]

I just added tons of Python code (including docs). (Almost) everything that has something to do with scrolling can now be calculated independently from toolkits such as GTK+. For [r963], I attached some code that demonstrates how to use a FiniteLayout object (an implementation of the arrangements as shown in figure C in my previous post). Have fun experimenting with it. Feel free to ask if you have any questions or suggestions for this API.

In order to make actual use of this API, it might be necessary to keep the state of a FiniteLayout object and the state of the GUI widgets (mainly viewport position and viewport size) in sync. Admittedly, PyGTK is still a closed book to me, but I think it shouldn't be too difficult to write some code that copies the state of a FiniteLayout to some kind of "unmanaged" widget and vice versa.

[Ark]

Hi Oddegamra! Happy new year! (I'm late, I know.) How's it going?

It looks like I get more and more used to Python and functional programming while coding for MComix. I'm currently squashing bugs by accident (sorry!) while incorporating zoom.py and scrolling.py. As you can see, in the last few weeks up to r981, I changed lots of things that have a quite visible influence on the user's experience. What do you think about these changes?

Another question: In previous revisions, "invert smart scroll" is silently ignored if two images are shown. Bug or feature? I changed "invert smart scroll" in r981 because the new way looks more consistent to me. What do you think?

(Notes to myself: There are some things left to do, especially MainWindow.scroll_to_fixed should be replaced by appropriate calls to FiniteLayout. Oh, and the changelog is kinda outdated.)

[Oddegamra]

Hey Ark,

happy new year to you, too. I've been keeping up with your changes and have been syncing regularly to test the things you've been committing. Overall, the quality seems to be quite good, I've yet to find a problem while casually using the program.

As for invert smart scrolling, I think it was intentional that it did nothing in double-page mode. Originally, the feature was born from the idea that some people release double-page spreads as single image, i.e. [ | ] instead of [_] [_]. The original smart scrolling code does the wrong thing here, since it scrolls down first, then left/right. Of course, it doesn't know that it is looking at two pages, and should be scrolling left/right first, then down. So, for cases like this, you can enable invert smart scrolling and scroll in the right direction. For double-page mode, MComix should do the right thing by itself, since it knows that it is looking at two pages.

There should be no problem with keeping the functionality as it is right now, though. After all, the user can still enable/disable the setting when necessary.

Oh, and feel free to update the changelog whenever you think it is appropriate. ;)

[asl97]

it move (left/right) too less if you use the scroll wheel, feature or bug?

[Ark]

This is supposed to be a feature. See "Preferences" -> "Behaviour" -> "Number of pixels to scroll per mouse wheel turn". (Note that this is different from "Fraction to page to scoll per space key press (in percent)" where do you enter a percentage rather than an absolute number of pixels.) Simply increase these numbers.

Note: As of [r981], these numbers are interpreted as maximum amounts. (That is, the actual scrolling distance might be smaller.) The current preferences dialog, however, still shows the old labels I used in the previous paragraph.

[Ark]

After the few amendments, refactorings, cleanups etc. I performed the other day, I would like to summarize some important features, properties and constraints of the new smart scrolling implementation. (This also serves as a note to myself so I don't lose track of them.) Here we go:

1. Every part of every image will eventually be shown to the viewer (if you use only smart scrolling and don't change anything else). This fixes the bug described in this ticket.
2. The points of the grid the viewport snaps to when using smart scrolling are equally distributed. This greatly improves how the viewer perceives the "speed" of the scrolling, see [#75] for details.
3. Smart scrolling is now "stateless", meaning that it determines the next reasonable viewport position by only looking at the actual current positions, sizes, orientation etc. (The old implementation made use of state variables like _last_scroll_was_direction_1 and was often confused if something unforeseen had happened.)
4. If you use double page mode and you scroll to the horizontal center (e.g. by using numpad keys), you might perfectly split the viewport into two halves, each of them showing a portion of the left and the right page, respectively. In this case, the calculations just mentioned will conclude that the viewer has just read the page that comes earlier in the book. This also holds true if there is no perfect split because the sum of the widths of these two halves (excluding the gap between them) is odd. To handle this, the images are arranged in a way that, if you scroll to the horizontal center using the numpad keys, the page that appears earlier in the book will be closer to the center than the other one and thus said to be the page the viewer just has read.
5. More generally, the page that is closest to the center of the viewport is said to be the current one. There is a method called FiniteLayout.get_current_index(self) that determines the corresponding index (and, as implied earlier, respects the current orientation, e.g. western mode or manga mode). However, this property is currently used only by the smart scrolling implementation. This leads to inconsistencies. The "open with" feature, for example, always thinks that the current image is the one highlighted in the thumbnailer, no matter where you scroll to when in double page mode.
6. When you are in single page mode and the image is wide enough so you can scroll horizontally and there is an odd number of horizontal positions smart scrolling snaps to, there is a center position it will snap to eventually. (This is because these positions are equally distributed, as described above.) The current implementation ensures that this horizontally centered position equals the horizontally centered position you scroll to when using the numpad keys. (This also works accordingly for the vertical axis when using "invert smart scroll", see below.)
7. The source code for all of these geometry issues has been generalized to cope with any (natural) number of dimensions. While this might look overengineered at the first glance, it leads to less repetitive code and makes it way harder for bugs to hide. (Note: This generalization process is not yet finished, actually.)

Now for the downside: At least lens.py doesn't know about the new backend, though. As a result, the magnifying glass is totally broken right now. In order to fix this, I would like to write a new abstraction that directly uses PIL to perform all the necessary operations. (The current implementation uses a mix of some of GTK's built-in features, e.g. rotate_simple(gtk.gdk.PIXBUF_ROTATE_CLOCKWISE) and some of PIL's features.) This would implicitly and thus easily fix some other old bugs as well. (For example, the lens currently totally ignores what you have entered in the "Enhance image" dialog.)

@Oddegamra: Changing the backend to use PIL only would replace the "best" interpolation method from a hyperbolic interpolation to a bicubic interpolation because this is the best one PIL offers. (See https://en.wikipedia.org/wiki/Bicubic_interpolation for an example of what it looks like.) Is this okay for you?

By the way, instead of using PIL, I'm currently using Pillow right now, and it perfectly works as a drop-in replacement for PIL with the advantage that it is actively maintained. See https://pypi.python.org/pypi/Pillow/2.4.0 for details. Maybe you want to replace the PIL library used in the all-in-one bundles (like the current mcomix-1.00.win32.all-in-one.zip) with Pillow for the next release. (If you do so, don't forget to mention Pillow as an alternative to PIL in the readme.)

[Oddegamra]

I noticed some of these things, yeah. Getting rid of the state (scroll direction) has been an especially good improvement and might as well have fixed a few other annoying bugs (such as switching from an overly large, scrolled image to a new page: the next scroll step would often step to a completely unexpected position). Great work.

The lens' implementation has been a bother for a long time, so basically any change you make will probably be a 100% improvement to the current situation. If that means sacrificing some of GTK's post-processing: Well, whatever. I have considered moving away from Pixbuf in the past, but it turned out not to be feasible, since in the end, most of GTK's widgets still need a pixbuf at the end of the day (such as the thumbnail sidebar and the library's book widget). As long as only the lens is affected, it probably won't matter that much.

Concerning Pillow, I already switched myself, too. Already edited the website to mention Pillow. If it isn't in the readme, I might have forgotten about that point. Will be fixed in a few moments.

[Ark]

There used to be a post here which has got its own ticket now: [feature-requests:#92]