deadpixi / sam

An updated version of the sam text editor.
Other
436 stars 47 forks source link

Scrolling with the scroll wheel moves lines off the top of the window #45

Open ckeen opened 7 years ago

ckeen commented 7 years ago

This happens with the experimental branch. If you open a file and use the mouse wheel to scroll down until the end, two lines will "move to the left" as if the characters get removed.

The buffer content is left intact, so this seems to be a fencing problem when handling scrolling

ckeen commented 7 years ago

The problem seems to occur when you move the line with the current cursor position to the top and beyond.

deadpixi commented 7 years ago

Please try the latest code in master and close this issue if it appears resolved. Thanks!

Screwtapello commented 7 years ago

I can still reproduce this with commit 9f7a29e, or at least something that resembles the description in the first comment.

Steps to reproduce:

  1. Create a new buffer

  2. Paste the following text into it:

    1abcdefg 2 3 4

    The line with 4 should be the last line in the file (no trailing \n causing a blank line afterward)

  3. Reshape the buffer so only the first three lines are visible (the line with 4 is scrolled off the bottom). It should look like this:

    1abcdefg 2 3

  4. Rotate the scroll wheel one "notch" to scroll down by a line

Expected result:

Actual result:

Notes:

deadpixi commented 7 years ago

Hm...okay. I'll keep looking at it. Thanks!

siebenmann commented 7 years ago

This happens for me as well with basically any file that exceeds the window size. Based on some crude diagnostics, it appears to be happening in cmdscrolldown's second option, where it calls horigin with l->origin +1.

My old change used a much more elaborate way of computing the new position, which may not be accurate after all of the code changes; it went for l->origin + frcharofpt(&l->f, Pt(l->f.r.min.x, l->f.r.min.y + l->f.fheight)). I believe that this worked reliably in my testing.

deadpixi commented 7 years ago

@siebenmann That worked even with the code changes, and is the obvious and correct way to do things. :)

@ckeen and @siebenmann and @Screwtapello Please try the latest code and in master and see if it works. Thanks!

Screwtapello commented 7 years ago

Works for me!

siebenmann commented 7 years ago

Unfortunately, samterm will now scroll the last line all the way off the top of the screen. The code I used in my version to deal with this is as follows (I'm including the comment to make the logic clearer, since the actual code may not be the best way to do it):

                /* Scroll forwards one line by setting the origin to one
                   line beyond the current origin, using the font height.
                   We only scroll if the new position is before the last
                   character, so the last line is always visible on screen.
                   Also: we stop scrolling up once the end of the file is
                   visible on the screen (this is the p1 check). <cks> may
                   reconsider this at some point.
                */
                p0 = l->origin + frcharofpt(&l->f, Pt(l->f.r.min.x, l->f.r.min.y + l->f.fheight));
                p1 = l->origin + frcharofpt(&l->f, Pt(l->f.r.min.x, l->f.r.max.y - l->f.fheight/2));
                if (p0 < tot && p1 < tot)
                        horigin(t->tag, p0);

(This is old code since it's using the older two-argument version of horigin(); some adaptation may be necessary, and there may now be better ways to measure things here.)

siebenmann commented 7 years ago

Oh oops, I forgot to include what tot is:

long tot = scrtotal(l);

This whole chunk of code was used in the original version of cmdscrolldownline in commit 21076f0, although cmdscrolldownline been reformed since then.

ckeen commented 7 years ago

I am unsure about the intended behaviour. Should scrolling stop when the last full page is displayed, i.e. the last line of the file is at the bottom of the buffer. Or should the scrolling end when the last line is hitting the top of the buffer... Both are valid behaviour imho...

Screwtapello commented 7 years ago

Right now, jumping down by left- or right-clicking on the scroll-bar, or by scrolling with the mouse-wheel, will scroll until the last line of the file is off the top of the buffer.

Steps to reproduce:

  1. move the insertion point to the end of the buffer
  2. hit enter, then "a", then enter, then "b", so the last line is just "b" with no trailing newline
  3. scroll down

Expected result:

Actual result:

Meanwhile, scrolling with the page-down key always leaves the last two lines of the file visible at the top of the window (in the example above, that would be a line with "a" and a line with "b", and the insertion point after "b".

I don't much mind where scrolling stops, but it should probably be consistent.