Make Y offset to be a multiple of line height #45

Open
opened 2015-05-04 05:52:36 -05:00 by nippur72 · 4 comments
nippur72 commented 2015-05-04 05:52:36 -05:00 (Migrated from github.com)

There is a possible incorrect behavior I've found I would like to discuss:

When you navigate with page down, the caret is forced into view by TextView.MakeVisible(), but that often makes the scroll Y offset to be unaligned respect to text lines, causing a partially covered top line (very noticeable with big fonts).

IMO, the correct behavior should be forcing newScrollOffset.Y to be a perfect multiple of DefaultLineHeight, so that the first line is always "in sync". This is what all other editors mostly do (e.g. VS).

I was thinking something like this:

newScrollOffset.Y = Math.Floor(newScrollOffset.Y / DefaultLineHeight) * DefaultLineHeight;

But I don't know if that fix is correct to, because of this DefaultLineHeight comment:

/// Gets the default line height. This is the height of an empty line or a line containing regular text.
/// Lines that include formatted text or custom UI elements may have a different line height.

What do you think?

There is a possible incorrect behavior I've found I would like to discuss: When you navigate with `page down`, the caret is forced into view by `TextView.MakeVisible()`, but that often makes the scroll Y offset to be unaligned respect to text lines, causing a partially covered top line (very noticeable with big fonts). IMO, the correct behavior should be forcing `newScrollOffset.Y` to be a perfect multiple of `DefaultLineHeight`, so that the first line is always "in sync". This is what all other editors mostly do (e.g. VS). I was thinking something like this: ``` C# newScrollOffset.Y = Math.Floor(newScrollOffset.Y / DefaultLineHeight) * DefaultLineHeight; ``` But I don't know if that fix is correct to, because of this `DefaultLineHeight` comment: ``` C# /// Gets the default line height. This is the height of an empty line or a line containing regular text. /// Lines that include formatted text or custom UI elements may have a different line height. ``` What do you think?
nippur72 commented 2015-05-10 06:04:23 -05:00 (Migrated from github.com)

the fix I mentioned seems to work nicely (I am using it in my fork), but has some side-effects I am not sure of.

I think along with it, there's scroll bar dragging and mouse wheeling that need to be aligned too.

the fix I mentioned seems to work nicely (I am using it in my fork), but has some side-effects I am not sure of. I think along with it, there's scroll bar dragging and mouse wheeling that need to be aligned too.
nippur72 commented 2015-05-11 05:49:38 -05:00 (Migrated from github.com)

Ok, I made another fix, I've moved the alignment operation within IScrollInfo.SetVerticalOffset(),
so that every change in Y offset is aligned with DefaultLineHeight. Seem to work better than before.

Ok, I made another fix, I've moved the alignment operation within `IScrollInfo.SetVerticalOffset()`, so that every change in Y offset is aligned with `DefaultLineHeight`. Seem to work better than before.
nippur72 commented 2015-05-11 10:36:36 -05:00 (Migrated from github.com)

Also:

  • I'm using Round in place of Floor
  • Fixed the impossibility to scroll to last line (by dragging scroll bar) if not a multiple of line height
Also: - I'm using `Round` in place of `Floor` - Fixed the impossibility to scroll to last line (by dragging scroll bar) if not a multiple of line height
nippur72 commented 2015-05-11 10:55:25 -05:00 (Migrated from github.com)

I closed my own pull request, as it had too many uncontrollable side effects.

I leave the issue open, in case someone else has a better solution.

I closed my own pull request, as it had too many uncontrollable side effects. I leave the issue open, in case someone else has a better solution.
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-icsharpcode/AvalonEdit#45
No description provided.