From 3738d2b9e17d1801269599cf5b73ba1165506c53 Mon Sep 17 00:00:00 2001 From: Harshad Vedartham Date: Wed, 28 Aug 2024 10:33:08 -0700 Subject: [PATCH] Tweaks and documentation --- .../frontend/textview/HighlightingEditor.java | 4 + .../textview/SyntaxHighlighterBase.java | 116 +++++++++++------- 2 files changed, 77 insertions(+), 43 deletions(-) diff --git a/app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java b/app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java index f97d0c803..5914b3ae1 100644 --- a/app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java +++ b/app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java @@ -129,6 +129,9 @@ protected void onDraw(Canvas canvas) { // Highlighting // --------------------------------------------------------------------------------------------- + // Batch edit spans (or anything else, really) + // This triggers a reflow which will bring focus back to the cursor. + // Therefore it cannot be used for updating the highlighting as one scrolls private void batch(final Runnable runnable) { try { beginBatchEdit(); @@ -146,6 +149,7 @@ private boolean isScrollSignificant() { // The order of tests here is important // - we want to run getLocalVisibleRect even if recompute is true // - we want to run isScrollSignificant after getLocalVisibleRect + // - We don't care about the presence of spans or scroll significance if recompute is true private boolean runHighlight(final boolean recompute) { return _hlEnabled && _hl != null && getLayout() != null && (getLocalVisibleRect(_hlRect) || recompute) && diff --git a/app/src/main/java/net/gsantner/markor/frontend/textview/SyntaxHighlighterBase.java b/app/src/main/java/net/gsantner/markor/frontend/textview/SyntaxHighlighterBase.java index 669bca217..25de2206b 100644 --- a/app/src/main/java/net/gsantner/markor/frontend/textview/SyntaxHighlighterBase.java +++ b/app/src/main/java/net/gsantner/markor/frontend/textview/SyntaxHighlighterBase.java @@ -5,6 +5,7 @@ * https://www.apache.org/licenses/LICENSE-2.0 * #########################################################*/ + package net.gsantner.markor.frontend.textview; import android.graphics.Canvas; @@ -45,6 +46,49 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +/** + * This is the base class for syntax highlighting, it contains routines for + * managing highlighting spans in the HighlightingEditor. + *

+ * Android's EditText uses a SpannableStringBuilder class to manage it's text, and + * DynamicLayout to to manage the text layout. Both are not very efficient with a large + * number of spans. + *

+ * The approach taken here is to: + * 1. Compute all spans (highlighting) for the text + * 2. Apply only those spans which are currently in the viewport + * 3. Update (remove and reapply) spans when the viewport moves (i.e we scroll) + *

+ * Spans are further divided into two categories: dynamic and static. + * - Dynamic spans are updated as one scrolls, as described above + * - Static spans are applied once and never updated. These are typically used for + * spans which affect the text layout. + * - For example, a span which makes text bigger. + * - Updating these dynamically would make the text jump around as one scrolls + *

+ * Fixup: + * - As the user types we shift all spans to accomodate the changed text. + * - This is done so that dynamically applied spans are applied to the correct region. + * - Fixup is batched and performed only if necessary. + *

+ * Span generation: + * - Derived classes should override generateSpans() to generate all spans + * - New spans are added by calling addSpanGroup() + * - The HighlightingEditor will trigger the generation of spans when the text changes. + * - This is debounced so that changes are batched + * - Span generation is done on a background thread + *

+ * Other performance tips: + * - Performance is heavily dependent on the number of spans applied to the text. + * - Combine related spans into a single span if possible + * - HighlightSpan is a helper class which can be used to create a span with multiple attributes + * - For example, a span which makes text bold and italic + * - Absolutely minimize the number of spans implementing `UpdateLayout` + * - These spans trigger a text layout update when changed in any way + * - Instead consider using a span implementing `StaticSpan` + * - If StaticSpans are present, the text is reflowed after applying them + * - This happens once, and not for each span, which is much more efficient + */ public abstract class SyntaxHighlighterBase { protected final static int LONG_HIGHLIGHTING_DELAY = 2400; @@ -284,14 +328,6 @@ private void clearFixup() { _fixupDelta = 0; } - // The fixup logic offsets all spans after a certain point by a delta - // All spans after = (start + before) are offset by (count - before) - // If we are typing text or deleting text naturally, we can batch these changes - // The delta will be 1 or -1 for each character added or removed - // The start would move back if we are deleting text - // The delta would increase if we are adding text - - public SyntaxHighlighterBase applyAll() { return applyDynamic().applyStatic(); } @@ -306,58 +342,52 @@ public SyntaxHighlighterBase applyDynamic() { * @return this */ public SyntaxHighlighterBase applyDynamic(final int[] range) { - if (GsTextUtils.isValidSelection(_spannable, range) && range.length < 2) { - return this; - } - - applyFixup(); - - final int length = _spannable.length(); - for (int i = 0; i < _groups.size(); i++) { - final SpanGroup group = _groups.get(i); + if (GsTextUtils.isValidSelection(_spannable, range) && range.length >= 2) { + applyFixup(); + final int length = _spannable.length(); + for (int i = 0; i < _groups.size(); i++) { + final SpanGroup group = _groups.get(i); - if (group.isStatic) { - continue; - } + if (group.isStatic) { + continue; + } - if (group.start >= range[1]) { - // As we are sorted on start, we can break out after the first group.start > end - break; - } + if (group.start >= range[1]) { + // As we are sorted on start, we can break out after the first group.start > end + break; + } - final boolean valid = group.start >= 0 && group.end > range[0] && group.end <= length; - if (valid && !_appliedDynamic.contains(i)) { - _spannable.setSpan(group.span, group.start, group.end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - _appliedDynamic.add(i); + final boolean valid = group.start >= 0 && group.end > range[0] && group.end <= length; + if (valid && !_appliedDynamic.contains(i)) { + _spannable.setSpan(group.span, group.start, group.end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); + _appliedDynamic.add(i); + } } } - return this; } public SyntaxHighlighterBase applyStatic() { - if (_spannable == null || _staticApplied) { - return this; - } + if (_spannable != null && !_staticApplied) { + applyFixup(); - applyFixup(); + boolean hasStatic = false; + for (final SpanGroup group : _groups) { + if (group.isStatic) { + hasStatic = true; + _spannable.setSpan(group.span, group.start, group.end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); + } + } - boolean hasStatic = false; - for (final SpanGroup group : _groups) { - if (group.isStatic) { - hasStatic = true; - _spannable.setSpan(group.span, group.start, group.end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); + if (hasStatic) { + reflow(); } - } - if (hasStatic) { - reflow(); + _staticApplied = true; } - _staticApplied = true; - return this; }