Menu
▾
▴
#1557 Simplify and optimize WrapLine()
closed
5
2025-06-17
2025-05-19
No
Split out from [feature-requests:#1417].
- following block is redundant:
if (!foundBreak) {
// Try moving to start of last character
if (p > 0) {
lastGoodBreak = CharacterBoundary(p, -1);
}
}
after
// backtrack to find lastGoodBreak
Sci::Position lastGoodBreak = p;
if (p > 0) {
lastGoodBreak = CharacterBoundary(p, -1);
}
lastLineStart <= lastGoodBreak <= numCharsBeforeEOL, following code only update lastGoodBreak to value larger then lastLineStart.
- remaining use of
CharacterBoundary()can be changed to usepdoc->NextPosition().
Patch change
CharacterBoundaryto usepdoc->NextPosition(), remove the redundant block, and changeafterWrap = CharacterBoundary(lastGoodBreak, 1).Committed as [1b7610]. Also removed last write of
foundBreakwhich was never read.Related
Commit: [1b7610]
I just want to revert the change for
CharacterBoundary()as currentlyMovePositionOutsideChar()is faster thanNextPosition()(see [feature-requests:#1558]).Related
Feature Requests: #1558
Patch revert
CharacterBoundary()toMovePositionOutsideChar(), currentNextPosition()version looks more readable though.I'm not all that worried about the performance degradation and expect that
NextPositioncan be made faster.The performance degradation should not be observed for general UTF-8 file.
For file contains very long lines,
LineLayout::AddLineStart()is the bottleneck, following changes will improve the speed a lot:1. Change growth factor, e.g. to 1.5.
+ 4here is to ensure the code works whenlines=1 && lenLineStarts=0.2. minimum
lenLineStarts(Wrap::Charfor ASCII) can be estimated by divide line width with wrap width, optimallenLineStartscan be estimated by multiple a factor (e.g. 1.25, 1.5).rough changes:
Last edit: Zufu Liu 2025-05-21
I end up with just
const int newMaxLines = lines + 20 + lines/2;, as long line is very rare, reduce initial 10~20 allocations does not speed up a lot.I was thinking that the total width / line width approximation would avoid over-allocation for a file containing short paragraphs. It is only
20*4bytes though so maybe not significant when there has to be an allocation and the rest ofLineLayoutis taking so much space.The computation is unnecessary for manual written code file (already breaks into short lines), only benefits program generated data file with huge lines, and change growth factor will defect the slower.
Use 2x growth factor (libc++, libstdc++) yields simple code than 1.5x (MSVC), also reduces total allocations.
Last edit: Zufu Liu 2025-05-23
OK, except for the
std::swapwhich appears to be less conventional thanstd::move. [bc402a]Related
Commit: [bc402a]
use
swap()orstd::swap()fixes bad code gen (twooperator delete[]calls) for= std::move().I don't see double deletion occurring with either debug or release MSVC 2022 17.13.7.
In terminal, compile
cl /utf-8 /W4 /c /EHsc /std:c++20 /O2 /FAcs /DNDEBUG /I../include PositionCache.cxx, then findvoid LineLayout::AddLineStartinside PositionCache.cod:Last edit: Zufu Liu 2025-05-24
generated code with
swapis shorter and contains only oneoperator delete[]:Another patch simplified
Editor::WrapLines()andLineLayout::Resize().For
LineLayout, callFree()is redundant inside destructor andResize(), removedbidiData->Resize()asbidiDatais already freed.Without any destructor code, can be subjected to rule-of-zero.
deleted unused stuffs, but not convert the constructor to use default member init.
Rewrite
LineLayout::SubLineFromPosition()to avoid out of bound access whenline = lines - 1.That function appears incorrect as lines is ascending and the test is
lineStarts[line] <= posInLinewhich may break early. Example:This method is only active for bidirectional mode so will have limited impact.
swap comparison to
posInLine < lineStarts[line]looks would fix the bug.or
lineStarts[line] > posInLine.earlier return inside
LineLayout::SubLineFromPosition()is still not fixed.OK: [dbf195], [a67bc8].
Related
Commit: [a67bc8]
Commit: [dbf195]
That seems to need to also invert the
posInLineadjustment toThe only place this is needed is for the VoiceOver feature on macOS with
bidirectional=1to ensure that the VoiceOver 'cursor' selects whole sublines instead of a narrow rectangle over 2 line starts.