Conversation
| viewportHeight /= 2; | ||
| const qreal ls = m_doc->getLineSpacing(); | ||
| const qreal ls = m_doc->getLineSpacing(); | ||
| const int totalLines = qRound(m_doc->height() / ls); |
There was a problem hiding this comment.
it should probably not be qRound.
qCeil ?
There was a problem hiding this comment.
definitely different from old, so please use that (or functional identical )
There was a problem hiding this comment.
from a numerical point of view height() = ls * totalLines. Due to rounding errors calculating totalLines from this equation will likely not give an exact integer but a real that is very close to an integer. qRound would fix this. Adding 0.5 and dropping the fractional part (convertion to int) should do the same. But original solution adds 1. for some reason I don't understand. I revert this.
There was a problem hiding this comment.
Observations:
-
m_doc->height()andlshave fractions. But interesting enough the result ofm_doc->height() / lsis a real that represents an exact integer. Let x be the result, then the following shows that x, qFloor(x), and qCeil(x) are the same (i.e. x is an integer) even with more than 786 thousand lines. This is independent oflsbeing 1, 1.25, or 17.4375.Examples:
ls: 10.75 height() : 494.5 height()/ls : 46 46 46 ls: 53.6875 height() : 2469.63 height()/ls : 46 46 46 ls: 1.34375 height() : 1.05677e+06 height()/ls : 786433 786433 786433 ls: 17.4375 height() : 1.37134e+07 height()/ls : 786433 786433 786433The exact calculation of the last line for overscrolling to the top
totalLines = qRound(m_doc->height() / ls)does not rely totally on this observation by using qRound. -
viewport()->size().height()is an integer (pixels), but dividing it bylsgives a real:ls: 17.4375 height(): 479 height() / ls: 27.4695The integer part of this result is used for pageStep.
| const qreal ls = m_doc->getLineSpacing(); | ||
| const qreal ls = m_doc->getLineSpacing(); | ||
| const int totalLines = qRound(m_doc->height() / ls); | ||
| const int viewLines = qFloor(viewport()->size().height() / ls); |
There was a problem hiding this comment.
different from old code ?
And also dangerous to accidentally take integer calculus
(that is why the calculations start with 1. (1.0) to force the compiler to calculate float)
There was a problem hiding this comment.
To be consequent 1. + (m_doc->height() - viewportHeight) / ls should
be changed to 1. + 1. * (m_doc->height() - viewportHeight) / ls . Really?
Variable ls is not an integer so no integer division happens.
There was a problem hiding this comment.
Okay, I have looked it up, this should be fine.
But I have seen cases, where it makes a difference.
|
thanks |
This PR closes #2944 by changing interpretation of option "Vertical Overscroll" in Adv. Editor config. This will allow the editor to scroll down over the last line until the line reaches the top of the editor. The old behavior of stopping in the middle of the editor is dropped as discussed in #2946.
Please make CHANGELOG.html
Description of Commits
reorder some Special options as depicted in following image (row numbers to the left) to fit better in terms of content:
The final result to the right shows 3 options concerned with parentheses/brackets, two with scrolling and the rest with mouse actions (including option Triple-Click Selection below lower edge of the image). Not visible options to the right are not touched.
change behavior of the option.
changelog
Enhancing Vertical Scroll Bar Settings
The old formulae used have two drawbacks: 1. Page scrolling may conceal rows and 2. Scrolling doesn't "exactly" stop in the middle of the editor (this is not discussed here since this is no longer of relevance).
Page scrolling may omit rows
Old formula
setPageStep(qCeil(1.* viewportSize.height() / ls))applies qCeil to the number of lines visible in the editor. This number is not an integer when the lower edge of the editor covers some fraction of a line. Thus this line is part of the pageStep and the line after it will be the line presented at the top after scrolling one page.Example. In this case the editor shows almost nothing of line 14:
Now scroll down one page with the old formula:
No one (except me) can tell what is written in line 14. And this will happen every time scrolling down one page. The new formula
setPageStep(qFloor(viewport()->size().height() / ls))guarantees that the covered line will be presented again at top after scrolling:Now we know the contents of line 14.