CodeEditApp / CodeEditSourceEditor

A code editor view written in Swift powered by tree-sitter.
https://codeeditapp.github.io/CodeEditSourceEditor/documentation/codeeditsourceeditor
MIT License
510 stars 78 forks source link

🐞 Rendering Invisible Text #131

Closed thecoolwinter closed 1 year ago

thecoolwinter commented 1 year ago

Right now, CodeEditTextView attempts to render every line of text at once. This leads to a large reduction in performance for files that extend beyond the bounds of the visible text as that text is always rendered. This isn't a problem for syntax highlighting, as the highlighter only uses the text that is visible, but any text that is being rendered that isn't visible is wasted CPU resources.

The root cause is this line in STTextView, where TextKit asks a NSTextViewportLayoutControllerDelegate for the bounds to render. In the current implementation, STTextView returns the entire bounds of the text view, effectively telling TextKit to render the entire document.

https://github.com/krzyzanowskim/STTextView/blob/deab1ae18d45a0c865aefc897645ebca882d0190/Sources/STTextView/STTextView%2BNSTextViewportLayoutControllerDelegate.swift#L8

    public func viewportBounds(for textViewportLayoutController: NSTextViewportLayoutController) -> CGRect {
        // Return bounds until resolve bounds problem
        return bounds

The comments in the implementation say that the view lays out the entire document because layout fragments outside the viewport are broken, which if I'm not mistaken is the expected result. One of the benefits of TextKit2 is that it only lays out text in the viewport, so any layout fragments outside the viewport will of course be broken. This also, of course, breaks the ruler view.

The solution for the ruler view is to calculate line heights starting at the first rendered line, instead of the actual first line. If we align the first rendered lines in both the text view and the ruler view, it will appear that all drawn lines are lined up with their corresponding text lines. This will also make the ruler view more performant.

I've already been able to fix the viewport calculation problem using a custom delegate class, and @ben-p-commits mentioned on Discord they'd be willing to take on the ruler view. Both changes will have to be merged at the same time so this issue is to track this.

krzyzanowskim commented 1 year ago

I've already been able to fix the viewport calculation problem using a custom delegate class

as far as I'm concerned, simply re-enabling viewportBounds is not enough. The frame has to update, and it doesn't work properly. Anyway I enable the viewport on main branch https://github.com/krzyzanowskim/STTextView

Related discussion: https://github.com/krzyzanowskim/STTextView/discussions/12