#75 Make smart scrolling even smarter

[Ark]

Assume a viewport of 100px (height), an image of 210px (height) and a "smart scrolling" viewport shift of 100%. Only vertical scrolling is possible.

Current behaviour: Starting from the top of said image, the image will be first scrolled by 100px and then by 10px when using smart scrolling.

Proposed behaviour: Since you must hit the space bar twice (to get to the bottom) anyway, MComix should scroll by (210px - 100px) / 2 = 55px twice. That is, scroll by 55px and then 55px again instead of 100px and then 10px. It should feel more natural this way.

(This example was about one dimension, but is should be the same way for the other dimension as well.)

I think the proposed behaviour should simply replace the old one. If someone complains, it could be made optional afterwards.

What do you think?

Actually, I already have an idea how to do this. (It is not as easy as I thought at the vey beginning, but now it looks like I found a way.)

[Oddegamra]

Well, the basic idea is that it follows the reading flow. The program scrolls down about half of the visible space. This way, the content the user has already read (since he wants to scroll down further) has scrolled past now, but a bit of the lower portion (which is the upper portion now) still remains on screen for easier orientation (I stole the 50% idea from vi). I find that this works well for comics I usually read, but of course we can experiment with your suggestion. If it should feel more natural, I'd welcome such changes, obviously.

[Ark]

Thank you. I committed a few changes and a new file called smart_scrolling.py. There are lots of comments there. I hope that helps understanding the code.

Note that a SmartScrolling object is double page mode agnostic. This is intentional. It also does not have a built-in concept of reading directions, you have to specify it explicitly using the orientation parameter. Oh, and it should work with any number of dimensions, by the way.

I attached a demo file that shows how this works. Don't hesitate to ask if you have trouble understanding the code.

I don't know how to integrate my code to the existing one right now. But before I do so, I would like to know what you think about the code.

Off topic:

1. Which IDE do you recommend to a Python developer?
2. How to deal with this translation template thing?
3. How to use these unit tests?

Sorry, I'm not used to these concepts in Python. It would be nice to explain it here briefly or to refer to some documentation.

The program scrolls down about half of the visible space. This way, the content the user has already read (since he wants to scroll down further) has scrolled past now, but a bit of the lower portion (which is the upper portion now) still remains on screen for easier orientation (I stole the 50% idea from vi).

Actually, I used a 100% viewport shift only to make the calculations easier for this example. The demo code I attached uses 75%. (As described in the code, this is only the maximum percentage that will be used.)

[Oddegamra]

I'm a bit confused by ORIENTATION. Basically, 1 is forward and -1 is backwards, and ORIENTATION[0] is the x- and ORIENTATION[1] the y-axis? I ran the test script a few times, and the output looked reasonable enough for [1, 1] and [-1, 1] when starting at either [0, 0] or [47, 0] (i.e. forward direction only for now). I also quite appreciate your comments on the function definitions.

A few questions:

• Apparently, x-axis is always processed before y-axis. Would it be possible to add a switch to invert this? That is, proceed down the y axis first, then go down the x axis after? MComix has a switch right now to do so ("Invert smart scrolling direction"). People told me in the past such behavior was useful for scans that combine two pages into a single image, instead of delivering them separately. So, you'd first scroll down the first page and only then scroll sideways to the second page.
• Since double page mode is considered one large image at the moment, I'd imagine that the code invoking the smart scroller would have to make a few more distinctions in case of two images being displayed, since scrolling two pages as it they were one probably won't make much sense in practice. After all, bringing the second page into view while the first is still only half done would be a bit strange. Right now, MComix' smart scrolling only works reliably if both pages to not fit in the same viewport horizontally. So, it doesn't really work for most fit modes. The problem here is that the scroller determines its current position by looking at the scroll bars' values. If the page cannot be scrolled horizontally, it will simply go down and switch to the next page. Obviously, this doesn't really help in double page mode, as most of the time, you will want to scroll down once, read to the bottom of page 1, than go back up and restart from page 2.

Maybe some object in the call chain to the smart scroller should be stateful and keep track of where it has scrolled in the past, and where it should scroll in the future. I realize that this is probably a very difficult problem to deal with, as a number of other factors play into the direction that should be scrolled at any given time. Is the user on page 1, or on page 2? How should the current page be determined if both pages fit into the view port completely? When does the scroller switch to page 2? What if the user scrolls back at any given time? Or, worse, what if he drag-scrolls the image manually? Where to continue afterwards?

Well, my biggest hope for this whole endeavor is that moving the whole smart-scrolling code into a distinct controller might simplify the calling portion of the code sufficiently to leave some space for more improvements for double-page mode.

Oh, I ended up ranting almost exclusively about double-page mode... Well, in any case, to get this thing rolling, event.py:smart_scroll_down/up would be the starting point. The viewport's size is already available, and the image's dimension should be in main.py:left_image/right_image. The orientation can be deduced from _window.is_manga_mode and the function (i.e. scroll down/scroll up). The current position - well, as said above, the code right now utilizes the scrollbars to determine the position (main.py:_vadjust and _hadjust).

That leaves the actual scrolling. main.py's scroll method only scrolls relative to the current position, so maybe a new function that scrolls to an absolute position. Alternatively, there's already scroll_to_fixed, which accepts string/enum values to go to a certain point. We could do if isinstance(horiz, str) and elif isinstance(horiz, int), so that either a value in px or an enum value is accepted as scrolling target.

For your other questions:

1. Which IDE do you recommend to a Python developer?

If only I knew. Personally, I use gvim. There are a number of other, larger applications that advertise Python support, but I haven't gotten much use out of them. Most cannot do what vim cannot either, so it's pretty much personal preference.

1. How to deal with this translation template thing?

The .pot template is created from all .py files and strings surrounded by _() using xgettext from the gettext package. I use a shell script to update the template and dependent translation files occasionally:

#!/bin/sh

VERSION=$(grep VERSION mcomix/constants.py | sed -e "s/VERSION = //" -e "s/'//g")

xgettext -LPython -omcomix.pot -pmcomix/messages/ -cTRANSLATORS \
    --from-code=utf-8 --package-name=MComix --package-version=${VERSION} \
    --msgid-bugs-address=oddegamra@gmx.org \
    mcomix/*.py mcomix/archive/*.py mcomix/library/*.py

for pofile in mcomix/messages/*/LC_MESSAGES/*.po
do
    # Merge message files with master template
    msgmerge -U --backup=none ${pofile} mcomix/messages/mcomix.pot
done

The resulting .po files can be compiled with msgfmt translation1/mcomix.po -o translation1/mcomix.mo, but I usually only compile these if someone actually updated the translation.

1. How to use these unit tests?

You can run setup.py test, which will start all unit tests found within the test directory (i.e. classes inheriting from unittest.TestCase). It is also possible to run only one class by executing setup.py test -s test.zoom.ZoomModelTest. Some tests no longer run (i.e. zoom.py), but I guess that is to be expected after all the work that went into said file.

[Ark]

I'm a bit confused by ORIENTATION. Basically, 1 is forward and -1 is backwards,

Basically, yes.

[…] and ORIENTATION[0] is the x- and ORIENTATION[1] the y-axis? […]
Apparently, x-axis is always processed before y-axis. Would it be possible to add a switch to invert this? That is, proceed down the y axis first, then go down the x axis after?

The mapping of the axes is merely arbitrary and thus up to the caller. That is, if you want to read vertically first, simply

1. swap all the argument[0]'s and argument[1]'s before you call scroll,
2. call scroll and
3. swap the result[0]'s and result[1]'s of the return value again.

At least I hope that something like this will do the trick. (I never tested it, though.)

Since scroll does not need to know about this issue, you should handle it in another function or method that needs to know about axis mapping. Otherwise, the code would get unnecessarily complicated. Note that the scroll function is not limited to what MComix actually needs. This is intentional since this way of coding leads to way better abstractions and modularity.

The _bresenham_sums function, on the other hand, actually is an example of very bad design: It knows much about its caller. It knows that the caller needs a finite list of denom+1 partial sums of the deltas. The highest abstraction I can think of right now, however, would be a stateful function (aka method) that decides whether or not to add a 1 to the quotient. (Such an abstract version would belong to tools.py, by the way.) But because of laziness, I didn't do so but made the identifier of the _bresenham_sums function start with an underscore.

I hope that this explains the design choices I made.

Concerning that double page mode issue: I think of an abstraction that handles an arbitrary (but finite) number of pages, not only 1 or 2. I hope that such an abstraction reduces the number of bugs and leads to cleaner code. Furthermore, this abstraction might help understanding how to handle an infinite sequence of images. (This is needed to implement the feature described in [#76]. At least that is what I think the creater of this ticket refers to.)

Thank you for all the other explanations you gave. By the way, I prefer Geany for coding in Python.

[Ark]

THIS IS ONLY A TEST.

For some reason I didn't receive an e-mail about my previous post. Maybe this test post triggers an e-mail.

EDIT: It did.

[Ark]

I'd like to make event.py use the code I wrote. But currently, both main.py and event.py look way too messy to do so right now.

Actually, the methods _smart_scroll_down and _smart_scroll_up in event.py try to perform calculations with too much parameters at once: scrolling percentage, inverted scrolling, manga mode, double page mode, forwards/backwards scrolling. Actually, these methods should not calculate anything but map the GUI events to MComix' API calls. (At least that is what I think all methods in event.py should do.) Besides, both methods look quite alike. And what is that small_step param anyway?

Looks like there is still a lot of work to do. I have an idea how to handle double-page mode and single-page mode (and how to emulate infinite scrolls). But no code yet. Maybe I should learn about the way GTK draws components (and images) and how to create custom drawing methods. This might help designing APIs that are easy to use for GTK objects. But MainWindow.draw_image is more like a mess than a useful example. Any recommendations?

PS: I added a new version of my scrolling demo code that works with the changes I made in r926.

[Oddegamra]

Well, scrolling up and down essentially is similar except for the directions that are taken while scrolling. From what I remember, mostly the calls to scroll_to_fixed are any different. small_step is used for mouse-wheel scrolling. Space usually scrolls by a large amount, so the mouse wheel event handler sets small_step to a not-null value to indicate that only a small increment should be performed.

As for actual on-screen drawing, as opposed to letting GTK do all the drawing: MComix probably doesn't have anything of that sort as an example. From what I understand, you'd usually start with a gtk.DrawingArea (this would replace MainWindow.left_image and right_image), connect this area's expose event, and then retrieve the gtk.gdk.Window from said area in the expose event handler to draw onto. Expose event is normally triggered when an area becomes visible, or when an area has been damaged (by moving the mouse over it, moving other windows in front of it, and so on). Thus, the code in expose-event should be quite fast, so any image scaling, processing and enhancing should be done somewhere else in advance.

gtk.gdk.Window implements gtk.gdk.Drawable, which is where you will want to look to find all relevant drawing methods. Drawable.draw_pixbuf, essentially. You take a rectangle from a source Pixbuf and copy it onto the drawable. So, you'd have to scale the source pixbuf to a size appropriate for the viewport, and then copy the visible portion of the image (dependent on zoom and scrolling) to the DrawingaArea. At least, that is how I imagine the basics would need to look like. I have no doubts that it will be way more complicated than that, if you want to have a dynamic number of images on screen at once.

[Ark]

Thank you for your explanations. I hope that I can put them to good use.

I attached a patch file that demonstrates the (hopyfully) smarter scrolling behaviour. Note that the perceived "speed" stays the same for the respective axis. That is, it prevents that many-giant-steps-but-then-a-single-baby-step-near-the-end-of-the-page behaviour.

However, I don't want to commit these modifications yet because they break tons of features: The patch works only(!) in single-page mode and it ignores scaling (zoom to 100%) and it ignores "invert smart scroll". Oh, and the patch is very ugly. But it should be sufficient for a few tests (regarding window resizing, space bar usage, mouse wheel usage, manga mode).

[Oddegamra]

Hm, I see. First thing I did was reduce 'smart scroll percentage' from 0.5 to 0.45. 0.5 seems a bit too extreme now, but that might just be my imagination. Well, at least in "Fit to width" mode. I also gave it a spin in "Custom zoom mode", which worked just fine. The scrolling progression also worked quite as expected. One problem, besides the ones you mentioned: When I switch to "Fit to width", scrolling up (Shift-Space, mouse wheel) no longer works, and neither does switching pages. In this case, I assume _window._vadjust.value to be 0. Maybe this confuses the scrolling code.

[Ark]

Thanks for testing. The reason for the misbehaviour you just described is the same one I described in my previous post: Due to "Fit to width", you, in general, zoomed to something different from 100%. The demo code, however, only works when using identity scale. (The problem is that the demo code "reads" the scrollbar positions and suggests new ones (note that the code always thinks you scaled to 100%), but since there are no scrollbars actually used, no scrollbar is moved. In the next step, the scrollbar's position is re-read, but it didn't move and it won't move, so the demo code will always get the same input and will always produce the same output.)