Line numbers improvements #2090

guanglinn · 2023-08-15T15:15:42Z

Performance improvements
(1) avoid unnecessary loops;
(2) avoid unnecessary parameter updates.
Move the codes to a separate class.

based on #2088, #2062

…ty.md

…rmance improvements.

…al_build

…e line

…cal_build

harshad1 · 2023-08-17T16:11:47Z

IMO lets just merge this soon.

We can do performance experiments and make simplifying changes in follow up PRs if necessary. The split into a contained class makes this very clean and easy to do (thanks @guang-lin 👍 )

guanglinn · 2023-08-17T16:36:48Z

Your opinions are also reasonable. If there is anything inappropriate, you can adjust it. Additionally, it was your suggestions that led to significant improvements in these codes, with many ideas that I had not anticipated. I have gained a lot, thank you @harshad1 .

harshad1 · 2023-08-17T16:40:17Z

app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java

+                        CharSequence deleted = s.subSequence(start, start + count);
+                        matcher = pattern.matcher(deleted);
+                        while (matcher.find()) {
+                            _maxNumber--;


If we are going to keep this I suggest

_maxLineNumber += TextViewUtils.countChar(s, start, start + count, '\n')
_maxLineNumber -= TextViewUtils.countChar(s, start, start + count, '\n')

Simpler, faster, avoids need for creating a matcher and subsequence etc etc.

No need to even do the if (before == 0 && count > 0) if we do this

harshad1 · 2023-08-17T16:45:56Z

app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java

+            _paint.setTextSize(textSize);
+        }
+
+        public int getNumberDigits(int number) {


Min(Integer.toString(number), 5) ?

harshad1 · 2023-08-17T16:48:24Z

app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java

+        }
+
+        public void setTextSize(float textSize) {
+            if (_lastTextSize != _paint.getTextSize()) {


Checking inequality has no value here.

harshad1 · 2023-08-17T17:00:15Z

app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java

+            // Draw the line numbers
+            _paint.setColor(Color.GRAY);
+            final int size = _lineIndex.size();
+            for (int i = 0; i < size; i++) {


The reason I had 2 loops with an ArrayList is because the first loop computes both the maxLineNumber and the number y positions.

If we are going to compute max line number in a text watcher, then this loop can be combined with the first one and we can eliminate the need for the ArrayList and _startNumber etc etc

guanglinn · 2023-08-17T17:10:52Z

I agree with your opinions.

harshad1 · 2023-08-23T22:51:44Z

Can I do anything to help get this merged

guanglinn · 2023-08-24T00:09:24Z

I made some adjustments based on your suggestions, and I will submit later.

app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java

harshad1 · 2023-08-27T22:16:16Z

I am trying to do some performance profiling and here is what I see:

On a smaller file the total time to onDraw (including super) takes ~3000us of which the line numbers part takes ~500us

On a large file (the entire text of the odyssey) if I enable line numbers the app stalls and doesn't work. (I had to disable the line count check). I am still working on understanding why, but clearly something is breaking the performance here.

Ignore this, I accidentally tested the wrong branch :(

harshad1 · 2023-08-27T23:01:26Z

I tried this on my rewrite too. It works with the large file and here are the numbers:

Drawtime is the total time taken by onDraw
Line time is the total time taken by the drawLineNumbers call
iter time is the time taken to iterate over every line of time file to determine y positions and max line number

all numbers are in microseconds and are for a file with 10440 lines and 601017 characters

harshad1 · 2023-08-27T23:37:04Z

After more testing, and looking at the current code. I think this is ready to merge. IMO we should still investigate ways of making line numbers much faster (there is still some lag with very large files). But it is good enough to merge now

guanglinn · 2023-08-28T04:02:58Z

I agree, at least it is fine now, and there may not be significant improvements in the short term.

harshad1 · 2023-08-28T06:59:05Z

Putting this down here for future reference.

One idea is to maintain the newline indices. Possibly by subclassing the DynamicLayout class

This way we would not have to do change tracking in the editor itself and would not have to iterate from the top to find current line numbers

harshad1 · 2023-08-28T07:03:30Z

One other thought (for this pr, not the future). Currently we redraw if the visible rect goes out of the last drawn area (3x current height). We should instead redraw if we go 50% (say) from last drawn area. The current system will not redraw until we are outside the drawn area. We should instead redraw slightly before we are outside the drawn area. Easiest way to do this is make the last drawn rect 25% smaller than the actually drawn area.

gsantner · 2023-09-05T23:20:21Z

Ia there a viable open todo here which would be a reason to not merge it?

I would like to merge everything to master and make sure that other PRs/improvements not get into huge rework needs due the amount of changes :D.

Regarding performance, sure good to improve but at the end the feature is disabled by default anyway - so shouldn't hurt people unless they active enable this :-).

guanglinn · 2023-09-06T01:31:03Z

The other PRs/improvements should all be included, these changes are based on the last commits of other PRs/improvements.
Performance loss may be minor, I have been using it for a while, and I didn't feel some unusual experience, even more than 10,000 lines.

gsantner · 2023-09-11T20:24:17Z

Thanks everybody, I merged everything to master now

harshad1 · 2023-10-02T19:16:06Z

@gsantner @guanglinn This PR appears to have broken image rendering in view mode. Still investigating

guanglinn · 2023-10-03T04:47:21Z

I tried just now, it's true, but it works well in my develop branch. I will help investigate it.

harshad1 · 2023-10-03T04:57:55Z

I believe I have solved the issue. Look at latest commit in

#2106

guang-lin and others added 30 commits July 15, 2023 08:09

add line numbers support

ab04552

fix bug in calculating width of max line number

33b3d78

remove 3 strings of non english translations, update licenses_3rd_par…

af10f41

…ty.md

Bugfix: Mermaid code cannot be rendered when line numbers enabled

a4c7161

Optimized code

488f476

Bugfix: Line numbers cannot behave well with bigger headings enabled

3dd82a4

perf(line numbers): drawing line numbers only near the visible area

e5069a5

fix(line numbers): ClassCastException when disable line wrapping

a0579cc

rework line number settings

289cfb8

automatic code reformat, add license notice for flexmark extension

e118cd0

supress lint due checked with getLineCount

d480f9a

todo.txt improvement

40077ac

feat(line numbers): align line numbers text to the right; minor perfo…

a7fdd30

…rmance improvements.

refactor(line numbers): minor improvements

bc83846

perf(line numbers): performance improvements

dbe456b

refactor(line numbers): minor improvements

57d5935

refactor(line numbers): minor improvements

4cbee78

Token for inserting selection

8d62124

Ignoring token in new file dialog

7ecb268

Tweaks

35692bf

refactor(line numbers): minor improvements

c899628

Merge remote-tracking branch 'coworker/line_numbers_support' into loc…

4900610

…al_build

perf(line numbers): added support for iterating from the first visibl…

b75fdfb

…e line

Merge remote-tracking branch 'guang-lin/line_numbers_support' into lo…

7901b7f

…cal_build

Reworked line numbers for some simplicity

dc1c9f1

Fixes, per file

892cdf4

Converter with line num support

7ae90c2

GetChars made faster

7faa0db

Many small tweaks

c7b0cd7

Unrelated fix for matching directories too

8f18b61

harshad1 reviewed Aug 17, 2023

View reviewed changes

harshad1 mentioned this pull request Aug 23, 2023

Snippet token for inserting selection (closes #2059) #2081

Closed

Li Guanglin added 2 commits August 24, 2023 18:00

perf(line numbers): some improvements based on the suggestions

26f33c1

fix(line numbers): argument 'text' might be null

a7cf4fb

harshad1 reviewed Aug 24, 2023

View reviewed changes

app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java Show resolved Hide resolved

docs(line numbers): correct the comments

34d46ac

Li Guanglin added 2 commits August 29, 2023 11:00

refactor(line numbers): add margin to recalculate the start line

1205e0d

fix(line numbers): solve lag update to border of the gutter

9a13a64

gsantner merged commit 83f5f59 into gsantner:linenumbers Sep 11, 2023
1 check passed

guanglinn deleted the linenumbers branch December 25, 2023 07:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Line numbers improvements #2090

Line numbers improvements #2090

guanglinn commented Aug 15, 2023 •

edited

Loading

harshad1 commented Aug 17, 2023

guanglinn commented Aug 17, 2023

harshad1 Aug 17, 2023 •

edited

Loading

harshad1 Aug 17, 2023

harshad1 Aug 17, 2023

harshad1 Aug 17, 2023 •

edited

Loading

guanglinn commented Aug 17, 2023

harshad1 commented Aug 23, 2023

guanglinn commented Aug 24, 2023

harshad1 commented Aug 27, 2023 •

edited

Loading

harshad1 commented Aug 27, 2023

harshad1 commented Aug 27, 2023

guanglinn commented Aug 28, 2023

harshad1 commented Aug 28, 2023

harshad1 commented Aug 28, 2023

gsantner commented Sep 5, 2023

guanglinn commented Sep 6, 2023

gsantner commented Sep 11, 2023

harshad1 commented Oct 2, 2023

guanglinn commented Oct 3, 2023 •

edited

Loading

harshad1 commented Oct 3, 2023

Line numbers improvements #2090

Line numbers improvements #2090

Conversation

guanglinn commented Aug 15, 2023 • edited Loading

harshad1 commented Aug 17, 2023

guanglinn commented Aug 17, 2023

harshad1 Aug 17, 2023 • edited Loading

Choose a reason for hiding this comment

harshad1 Aug 17, 2023

Choose a reason for hiding this comment

harshad1 Aug 17, 2023

Choose a reason for hiding this comment

harshad1 Aug 17, 2023 • edited Loading

Choose a reason for hiding this comment

guanglinn commented Aug 17, 2023

harshad1 commented Aug 23, 2023

guanglinn commented Aug 24, 2023

harshad1 commented Aug 27, 2023 • edited Loading

harshad1 commented Aug 27, 2023

harshad1 commented Aug 27, 2023

guanglinn commented Aug 28, 2023

harshad1 commented Aug 28, 2023

harshad1 commented Aug 28, 2023

gsantner commented Sep 5, 2023

guanglinn commented Sep 6, 2023

gsantner commented Sep 11, 2023

harshad1 commented Oct 2, 2023

guanglinn commented Oct 3, 2023 • edited Loading

harshad1 commented Oct 3, 2023

guanglinn commented Aug 15, 2023 •

edited

Loading

harshad1 Aug 17, 2023 •

edited

Loading

harshad1 Aug 17, 2023 •

edited

Loading

harshad1 commented Aug 27, 2023 •

edited

Loading

guanglinn commented Oct 3, 2023 •

edited

Loading