#1580 Cache IsDynamic() and OverridesTextFore() for Indicator

[Zufu Liu]

IsDynamic() and OverridesTextFore() can be cached using padding bytes around bool under field. not sure whether it has performance bennifit for EditView::DrawForeground(), BreakFinder::BreakFinder() and others, but Scintilla.dll is 1KB smaller with the change.

Rought changes exclude member init changes:

// Indicator.h
void Refresh() noexcept {
    dynamic = !(sacNormal == sacHover);
    overrideTextFore = sacNormal.style == Scintilla::IndicatorStyle::TextFore || sacHover.style == Scintilla::IndicatorStyle::TextFore;
}
bool IsDynamic() const noexcept {
    return dynamic;
}
bool OverridesTextFore() const noexcept {
    return overrideTextFore;
}

// void ViewStyle::Refresh()
bool flagDynamic = false;
bool flagSetFore = false;
for (auto &indicator : indicators) {
    indicator.Refresh();
    flagDynamic |= indicator.IsDynamic();
    flagSetFore |= indicator.OverridesTextFore();
}
indicatorsDynamic = flagDynamic;
indicatorsSetFore = flagSetFore;

[Neil Hodgson]

This mostly just seems to me to be longer source code.

Its likely the increased executable size is from expanding std::any_of. Using any_of is partly a stylistic choice that I like for simple cases as it is more direct. However, it doesn't propagate noexceptwhich is unfortunate. With C++20, these can be shortened using ranges like:

indicatorsDynamic = std::ranges::any_of(indicators,
    [](const Indicator &indicator) noexcept { return indicator.IsDynamic(); });

[Zufu Liu]

No, only merge the two std::any_of() loops doesn't reduce binary size.

bool flagDynamic = false;
bool flagSetFore = false;
for (const Indicator &indicator : indicators) {
    flagDynamic |= indicator.IsDynamic();
    flagSetFore |= indicator.OverridesTextFore();
}
indicatorsDynamic = flagDynamic;
indicatorsSetFore = flagSetFore;

[Zufu Liu]

The background for cache indicator values is that I want (for URL highlighting) a hotspot style like indicator that changes text color and draw underline on mouse hover, this is link/URL style in many websites, e.g. https://github.com/ScintillaOrg/lexilla/issues, https://en.cppreference.com/w/cpp.html, https://learn.microsoft.com/en-us/windows/win32/apiindex/windows-api-list

So IsDynamic() and OverridesTextFore() will getting complex, currently I added bool hoverUnderline into Indicator class (code pushed at https://github.com/zufuliu/notepad4/commits/main/), and draw underline in EditView::DrawForeground() like hotspot.
A better implementation could use IndicFlag attributes:

1. set whether to draw normal/hover underline for IndicatorStyle::TextFore.
2. or change normal/hover text fore color for other shape indicator styles, like links (bold underline on hover?) on https://github.com/resources/articles/what-are-ai-agents.

[Neil Hodgson]

This is like INDIC_EXPLORERLINK which is an extension in Notepad++. I didn't accept this into Scintilla as thought it likely that other combinations would be wanted and a more generic interface would avoid too much code proliferation.

Should the shape and text foreground colours be independent? If they are independent then there should be another colour in a StyleAndColour.

Since it is more likely to want to combine text colour with shapes over combining multiple shapes, there could be a bit flag combined in IndicatorStyle::TextForeModifier=0x1000.

Another approach could be to chain multiple indicators together by adding an int Indicator::next field to add more effects as needed with an out-of-bounds terminator on the last in the chain. hidden/blue-underline; green-text/blue-text; OOB.

Changing text style for indicated text modifies positioning and size so may lead to overdraw/gap/flicker problems when dynamic.

It would be better to concentrate on the feature parameters and external API before worrying about performance. It's likely that performance issues, if they arise, can be better handled at different scopes.

[Neil Hodgson]

A simple improvement here may be to add a secondary sacNormal/sacHover pair to Indicator so that a wide range of combined types are available. Default these to hidden black.

There could be some more APIs but maybe just add a flag (IndicatorNumbers::Secondary=0x1000) to the index to indicate the secondary element.