[scintilla:feature-requests] #1481 WrapBlock improvements

2 views
Skip to first unread message

Zufu Liu

unread,
Mar 9, 2023, 7:07:39 AM3/9/23
to scintill...@googlegroups.com

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu
Last Updated: Thu Mar 09, 2023 12:07 PM UTC
Owner: Neil Hodgson

A. topLine seems need to be (document line) pcs->DocFromDisplay(topLine):

    const SignificantLines significantLines {
        pdoc->SciLineFromPosition(sel.MainCaret()),
        topLine,
        LinesOnScreen() + 1,
        view.llc.GetLevel(),
    };

B. it might worth to add RetrieveLineLayout() overload with SignificantLines parameter to avoid repeated call inside current RetrieveLineLayout() method, e.g. LineRange, lineCaret and LinesOnScreen() are repeated.

C. the entire LineRange to LayoutLine block for short and long lines looks very similar, could be extracted as a method.

Sent from sourceforge.net because scintill...@googlegroups.com is subscribed to https://sourceforge.net/p/scintilla/feature-requests/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/scintilla/admin/feature-requests/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.

Neil Hodgson

unread,
Mar 9, 2023, 11:38:18 PM3/9/23
to scintill...@googlegroups.com

A. OK, fixed with [ca6c2d].

B. Trying it didn't improve performance even for whole file ASCII monospace. Profiling with monospace points to BreakFinder and memory allocation / free as heavy.

C. The main difference is the locking which only occurs for short lines. Extracting the code into a method requires around 8 parameters and long parameter lists are a code smell with the caller and callee too intertwined to be easily understood. Maybe some other refactoring could help.

Zufu Liu

unread,
Mar 11, 2023, 6:26:59 PM3/11/23
to scintill...@googlegroups.com

B. what about following (adding linesTotal and styleClock into SignificantLines):

std::shared_ptr<LineLayout> LineLayoutCache::Retrieve(Sci::Line lineNumber, const SignificantLines &significantlines, Sci::Position lengthLine) {
    return Retrieve(lineNumber, significantlines.lineCaret,
        static_cast<int>(lengthLine), significantlines.styleClock,
        significantlines.linesOnScreen, significantlines.linesTotal);
}

C. lengthLine >= lengthToMultiThread can be avoided in long lines loop, it can be replaced by linesAfterWrap[indexLarge] == 0 (maybe in a range-based for loop).

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Fri Mar 10, 2023 04:38 AM UTC
Owner: Neil Hodgson

Zufu Liu

unread,
May 26, 2026, 6:31:01 AMMay 26
to scintill...@googlegroups.com

Merge allocation (similar to PositionCacheEntry) for chars, styles and positions on LineLayout can reduce wrap time (tested file with mostly ASCII).
pseudo code:

unsigned length = maxLineLength_ + 2;
size_t lineAllocation = align_up(length, 16); // could be 8 or 16
chars = std::make_unique<char[]>(lineAllocation*2 + length*sizeof(XYPOSITION));
styles = chars.get() + lineAllocation;
positions = styles + lineAllocation;

this doesn't increase memory usage as system memory allocation is 8 or 16 aligned, it increased locality for chars and styles.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Sat Mar 11, 2023 11:26 PM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
May 26, 2026, 5:57:46 PMMay 26
to scintill...@googlegroups.com

How dependable is 'system memory allocation is 8 or 16 aligned'? I could imagine an allocator (perhaps chosen by the application) wanting to be tighter for char blocks. Wouldn't want to crash or invoke UB on other CPUs accessing positions[i].

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Tue May 26, 2026 10:30 AM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
May 26, 2026, 8:15:26 PMMay 26
to scintill...@googlegroups.com

Merging positions into the allocation gets in the way of the change I most want here: halving the size of positions by storing single precision floats instead of doubles. Doubles are currently used just to accomodate running out of precision on wide lines. A main array of floats can be augmented with occasional doubles (perhaps every 1000 positions) with the main array values then being offsets from the previous accurate value.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Tue May 26, 2026 09:57 PM UTC
Owner: Neil Hodgson

Zufu Liu

unread,
May 27, 2026, 6:06:57 AMMay 27
to scintill...@googlegroups.com

I think application can't provide class specific operator new for language builtin primary type like char. for custom global operator new (which has no type info), if alignment for it's returned pointer is less than builtin default operator new, the application is already broken.
for builtin operator new, the returned pointer seems has alignment __STDCPP_DEFAULT_NEW_ALIGNMENT__ ( I think it's >= alignof(std::max_align_t) >= alignof(double)).
https://en.cppreference.com/cpp/memory/new/operator_new

for win32 msvcrt/ucrt, the alignment is MEMORY_ALLOCATION_ALIGNMENT (https://learn.microsoft.com/en-us/windows/win32/api/heapapi/nf-heapapi-heapalloc#remarks). glibc seems is 16 (for int128/float128 or better simd?).

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Wed May 27, 2026 12:15 AM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
May 27, 2026, 7:35:47 PMMay 27
to scintill...@googlegroups.com

For 32-bit MSVC builds __STDCPP_DEFAULT_NEW_ALIGNMENT__ and MEMORY_ALLOCATION_ALIGNMENT are 8 which is still fine.

clang has a -fnew-alignment option but that seems more oriented at increasing default alignment.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Wed May 27, 2026 10:06 AM UTC
Owner: Neil Hodgson

Zufu Liu

unread,
May 28, 2026, 6:14:29 AMMay 28
to scintill...@googlegroups.com

Here are changes (contains some naive timing), tested SciTE the benefit for merge memory allocation seem is less than adding if (linesAfterWrap[indexLarge] == 0) { check for long lines.

Attachments:

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Wed May 27, 2026 11:35 PM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
May 29, 2026, 7:33:23 PMMay 29
to scintill...@googlegroups.com

Changing if (lengthLine for long lines seems reasonable but would indicate that the code being moved, likely the pdoc->LineRange, is costly.

I've thought about this in the past and a method that returns the starts of 2 consecutive lines would help. A variant of the called Partitioning::PositionFromPartition could return 2 values without performing some of the code twice. There'd have to be some extra end and step handling but it may be a win.

Alternatively, early in WrapBlock store the line starts for the whole range (+1 more) then use that.

Moving LineLayout member initialisation into the declaration is OK. There's a trade-off here in exposing internal decisions in a header versus hiding them from other code.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Thu May 28, 2026 10:14 AM UTC
Owner: Neil Hodgson

Zufu Liu

unread,
May 30, 2026, 9:28:27 AMMay 30
to scintill...@googlegroups.com

likely the pdoc->LineRange, is costly.

For general code document, most line is short, the check avoid checking all line range twice.

There's a trade-off here in exposing internal decisions in a header versus hiding them from other code.

They are flagged by clang-tidy's modernize-use-default-member-init check.

lengthToMultiThread can be changed to computed with durationWrapOneByte.ActionsInAllowedTime().

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Fri May 29, 2026 11:33 PM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
May 31, 2026, 9:35:57 PMMay 31
to scintill...@googlegroups.com

Committed LineLayout member initialisation and wrap threading choice changes with [4709bc] and [9e085a].

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Sat May 30, 2026 01:28 PM UTC
Owner: Neil Hodgson

Zufu Liu

unread,
Jun 3, 2026, 10:30:09 AMJun 3
to scintill...@googlegroups.com

bool callerMultiThreaded for EditView::LayoutLine() method can be changed to a flag enum, so if (vstyle.edgeState == EdgeVisualStyle::Background) block (maybe others) can be omitted (not measured the time) for non-significant llTemporary and llLarge.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Mon Jun 01, 2026 01:35 AM UTC
Owner: Neil Hodgson

Zufu Liu

unread,
Jun 4, 2026, 8:30:24 AMJun 4
to scintill...@googlegroups.com

Skip the find column block indeed makes wrap for EdgeVisualStyle::Background faster, but since Insert(ll->edgeColumn); is used in BreakFinder, result ll->lines may diff?

I implemented an optimization in Notepad4 to skip layout short non-significant lines:

// LineLayout::wrapWidthMinimum=20, from EditView::LayoutLine()
// Representation::maxByteLength=3 for C0 and hex blob that maps 1 byte to 3 characters
const int wrapWidth = std::max(model.wrapWidth, LineLayout::wrapWidthMinimum);
const int maxByteLength = std::max(Representation::maxByteLength, model.pdoc->tabInChars);
const int maxLineLength = static_cast<int>(wrapWidth / vstyle.aveCharWidth);
const int minLineLength = maxLineLength / maxByteLength;

when lengthLine < minLineLength skip layout the line.
when lengthLine < maxLineLength most likely the line will fit on single line, following can used to do a quick check (this optimization is not used Notepad4, as performance is not better, not sure whether due to layout monospaced ASCII is very faster).

bool IsSingleLine(const EditModel &model, const char *chars, unsigned numCharsInLine, int maxLineLength) noexcept {
    const int tabInChars = model.pdoc->tabInChars - 1;
    maxLineLength -= numCharsInLine;
    unsigned index = 0;
    do {
        const char ch = chars[index++];
        if (ch == '\t') {
            maxLineLength -= tabInChars;
        } else if (ch < ' ' || ch >= '\x7F') {
            // treat as control or hex blob
            maxLineLength -= Representation::maxByteLength - 1;
        }
    } while (index < numCharsInLine && maxLineLength > 0);
    return maxLineLength > 0;
}

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Wed Jun 03, 2026 02:29 PM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
Jun 4, 2026, 6:16:23 PMJun 4
to scintill...@googlegroups.com

Due to widely varying character widths and the use of multiple fonts, aveCharWidth is not a safe way to determine how many characters can fit on a line.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Thu Jun 04, 2026 12:30 PM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
Jun 4, 2026, 7:33:35 PMJun 4
to scintill...@googlegroups.com

If there is a significant performance gain from ignoring short lines, it may be safer to constrain this to just the really common cases.

For example, isprint(+tab)-ASCII-only lines with no representations except for the line end. Then finding the maximum character width - possibly per-style or per character but a global max over all isprint ASCII for all styles may be OK. Some letter pairs may kern wider so there could be a check for maximal kerning addition included in the maximum character width (test all pairs and cache result). Restricting to ASCII avoids complex shapers and other features. Ligatures are commonly narrower than their characters but there could potentially be wider ligatures.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Thu Jun 04, 2026 10:16 PM UTC
Owner: Neil Hodgson

Zufu Liu

unread,
Jun 5, 2026, 2:43:38 AMJun 5
to scintill...@googlegroups.com
  • labels: scintilla, layout --> scintilla, layout, wrap

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout wrap

Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Thu Jun 04, 2026 11:33 PM UTC
Owner: Neil Hodgson

Zufu Liu

unread,
Jun 5, 2026, 2:57:14 AMJun 5
to scintill...@googlegroups.com

Due to widely varying character widths and the use of multiple fonts

A safe implementation could find the max width (inside FindMaxAscentDescent()).

Then finding the maximum character width - possibly per-style or per character

That would too complex may only makes proportional ASCII short lines faster. the idea is based on East Asian Width and DBCS encoding that wide character requires more bytes to encode, DBCS encoding with monospaced font using SurfaceGDI, each byte (except tab and those have representations) can be treated as has width monospaceCharacterWidth.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout wrap

Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Fri Jun 05, 2026 06:43 AM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
Jun 5, 2026, 7:17:55 PMJun 5
to scintill...@googlegroups.com

A change that just targeted fullwidth (East Asia) characters may be safer since East Asian characters tend to have little shaping/sizing interaction with other characters. That could still have problems caused by font fallback with new, rare, historical, or other-language characters missing from mainstream fonts. The gamut of fullwidth characters could be checked to find a reasonable width. Other writing systems have more complex inter-character relationships. I think some Korean implementations may be more complex with combining or separating syllable blocks.

Sometimes platforms display missing characters with quite large visuals such as a square containing the hex value. This is where Scintilla doesn't have control - its up to the platform libraries, add-ons (like language shapers and fonts), and sometimes user preferences to determine how text runs are drawn.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout wrap
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Fri Jun 05, 2026 06:57 AM UTC
Owner: Neil Hodgson

Zufu Liu

unread,
Jun 5, 2026, 7:28:51 PMJun 5
to scintill...@googlegroups.com

OK, then my optimization for skip short lines (lengthLine < minLineLength) would be unsafe when the line has non-ASCII characters.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout wrap
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Fri Jun 05, 2026 11:17 PM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
Jun 5, 2026, 11:14:30 PMJun 5
to scintill...@googlegroups.com

It would be possible to have Scintilla or a Scintilla platform layer take over more of the layout and drawing tasks to allow more optimization. Some platforms include more low level calls for features like text to glyph conversion and font file access.

I've mostly thought about this for Latin or even just ASCII documents. The core layout elements are reasonably simple: conversion from text to glyphs, a table of widths or advance values and another table of kerning adjustments for character pairs. Other features like ligatures might be ignored. The data could be ingested into Scintilla and its use optimized without calling the platform inside heavy actions like wrapping. This could be controlled with a per-view flag with a low quality fallback on failure (characters outside scope) or possibly a per-line decision on whether to use the internal implementation for simple cases and platform calls for more complex cases. A module that implements CJK layout when supplied with detailed font data (from OpenType or TrueType) may fit in similarly. It seems like much work though, with some per-platform code plus a secondary path through layout and display.

Other projects like word processors and web browsers often work on this more detailed level of text display.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout wrap
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Fri Jun 05, 2026 11:28 PM UTC
Owner: Neil Hodgson

Reply all
Reply to author
Forward
0 new messages