Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid drawing hidden text in DocView:draw_line_text #1298

Merged
merged 2 commits into from Jan 19, 2023

Conversation

Guldoman
Copy link
Member

@Guldoman Guldoman commented Jan 1, 2023

With this PR, we no longer send hidden (tokenized) text to the renderer.

Non tokenized text or text tokenized as long strings is still a problem, which can be solved by splitting the tokenized text into smaller pieces.

Here there might be a performance impact caused by having to call font:get_width, but this needs testing.

@Guldoman Guldoman changed the title Avoid drawing invisible text in DocView:draw_line_text Avoid drawing hidden text in DocView:draw_line_text Jan 2, 2023
@jgmdev
Copy link
Member

jgmdev commented Jan 6, 2023

Does this means ability to open a file with really long lines and not having the editor choke?

@Guldoman
Copy link
Member Author

Guldoman commented Jan 6, 2023

It surely helps, but only on lines where the tokens aren't extremely long.
We should split very long tokens in the tokenizer for those cases (like plaintext).

@adamharrison
Copy link
Member

As discussed on discord:

It is probably better to do a quick clip check in the rencache for the first bit, to avoid the double width computation; also has the added benefit of just generally probably slimming down the rencache. We should know the current clip in C at the time of sending the command, so I can't think of a reason why that wouldn't work.

The check was previously done with the window rect, so this will reduce 
a bit more the number of commands sent.
@Guldoman
Copy link
Member Author

Thanks to Adam's suggestions:

  • Removed the Lua check to avoid sending the draw commands for strings "to the left" of the DocView as it's done on the C side (and also avoids the font:get_width issue). It now just stops early.
  • Tightened the check done when adding draw commands to exclude the commands that would fall outside the latest clip. It was previously done only on the window rect, so this should skip a few more draw commands.

@adamharrison
Copy link
Member

Looks good. Tested, and works fine. Still chokes on large files, but as you say, almost assuredly better, and don't see any rendering errors. Merging.

@adamharrison adamharrison merged commit 9308dfd into lite-xl:master Jan 19, 2023
takase1121 pushed a commit to takase1121/lite-xl that referenced this pull request Aug 19, 2023
* Stop drawing text past the `DocView` edge in `DocView:draw_line_text`

* Don't add draw commands if they fall outside the latest clip

The check was previously done with the window rect, so this will reduce 
a bit more the number of commands sent.
takase1121 pushed a commit to takase1121/lite-xl that referenced this pull request Aug 19, 2023
* Stop drawing text past the `DocView` edge in `DocView:draw_line_text`

* Don't add draw commands if they fall outside the latest clip

The check was previously done with the window rect, so this will reduce 
a bit more the number of commands sent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants