From 04deb81f71819d6946d598aa5170a6f46d7972eb Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Mon, 20 Feb 2023 16:00:31 +0000 Subject: [PATCH] LibGUI: Validate TextDocument spans when merging them, not when painting TextDocument::merge_span_collections() automatically makes sure that the spans are valid, move forwards, are in order, and do not overlap. This means we don't have to check these things every time TextEditor paints the document. merge_span_collections() now does these checks instead. I am not certain they are still useful, but someone in the past certainly did. I have modified them to take advantage of the operator overloads and Formatter that we now have. --- Userland/Libraries/LibGUI/TextDocument.cpp | 22 +++++++++++++++++++ Userland/Libraries/LibGUI/TextEditor.cpp | 25 ---------------------- 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/Userland/Libraries/LibGUI/TextDocument.cpp b/Userland/Libraries/LibGUI/TextDocument.cpp index 6fb2cee439..354a25225a 100644 --- a/Userland/Libraries/LibGUI/TextDocument.cpp +++ b/Userland/Libraries/LibGUI/TextDocument.cpp @@ -1,12 +1,14 @@ /* * Copyright (c) 2018-2020, Andreas Kling * Copyright (c) 2022, the SerenityOS developers. + * Copyright (c) 2023, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ #include #include +#include #include #include #include @@ -1409,7 +1411,27 @@ void TextDocument::merge_span_collections() } m_spans.clear(); + TextDocumentSpan previous_span { .range = { TextPosition(0, 0), TextPosition(0, 0) }, .attributes = {} }; for (auto span : merged_spans) { + // Validate spans + if (!span.span.range.is_valid()) { + dbgln_if(TEXTEDITOR_DEBUG, "Invalid span {} => ignoring", span.span.range); + continue; + } + if (span.span.range.end() < span.span.range.start()) { + dbgln_if(TEXTEDITOR_DEBUG, "Span {} has negative length => ignoring", span.span.range); + continue; + } + if (span.span.range.end() < previous_span.range.start()) { + dbgln_if(TEXTEDITOR_DEBUG, "Spans not sorted (Span {} ends before previous span {}) => ignoring", span.span.range, previous_span.range); + continue; + } + if (span.span.range.start() < previous_span.range.end()) { + dbgln_if(TEXTEDITOR_DEBUG, "Span {} overlaps previous span {} => ignoring", span.span.range, previous_span.range); + continue; + } + + previous_span = span.span; m_spans.append(move(span.span)); } } diff --git a/Userland/Libraries/LibGUI/TextEditor.cpp b/Userland/Libraries/LibGUI/TextEditor.cpp index a26d6805d6..de050aa33c 100644 --- a/Userland/Libraries/LibGUI/TextEditor.cpp +++ b/Userland/Libraries/LibGUI/TextEditor.cpp @@ -584,42 +584,17 @@ void TextEditor::paint_event(PaintEvent& event) break; } auto& span = document().spans()[span_index]; - if (!span.range.is_valid()) { - ++span_index; - continue; - } - if (span.range.end().line() < line_index) { - dbgln_if(TEXTEDITOR_DEBUG, "spans not sorted (span end {}:{} is before current line {}) => ignoring", span.range.end().line(), span.range.end().column(), line_index); - ++span_index; - continue; - } if (span.range.start().line() > line_index || (span.range.start().line() == line_index && span.range.start().column() >= start_of_visual_line + visual_line_text.length())) { // no more spans in this line, moving on break; } - if (span.range.start().line() == span.range.end().line() && span.range.end().column() < span.range.start().column()) { - dbgln_if(TEXTEDITOR_DEBUG, "span from {}:{} to {}:{} has negative length => ignoring", span.range.start().line(), span.range.start().column(), span.range.end().line(), span.range.end().column()); - ++span_index; - continue; - } - if (span.range.end().line() == line_index && span.range.end().column() < start_of_visual_line + next_column) { - dbgln_if(TEXTEDITOR_DEBUG, "spans not sorted (span end {}:{} is before current position {}:{}) => ignoring", - span.range.end().line(), span.range.end().column(), line_index, start_of_visual_line + next_column); - ++span_index; - continue; - } size_t span_start; if (span.range.start().line() < line_index || span.range.start().column() < start_of_visual_line) { span_start = 0; } else { span_start = span.range.start().column() - start_of_visual_line; } - if (span_start < next_column) { - dbgln_if(TEXTEDITOR_DEBUG, "span started before the current position, maybe two spans overlap? (span start {} is before current position {}) => ignoring", span_start, next_column); - ++span_index; - continue; - } size_t span_end; bool span_consumed; if (span.range.end().line() > line_index || span.range.end().column() > start_of_visual_line + visual_line_text.length()) {