ChrisPenner / rasa

Extremely modular text editor built in Haskell
GNU General Public License v3.0
617 stars 42 forks source link

Trim text to window size before rendering #27

Closed ghost closed 7 years ago

ghost commented 7 years ago

I believe this has the same effect as translating, but since rendering is done strictly, this should be more efficient.

ghost commented 7 years ago

I should probably mention, this depends on yi-rope patches in PR right now for being fast.

ChrisPenner commented 7 years ago

Looks great! Thanks for submitting this so quick!

ChrisPenner commented 7 years ago

Hi @siddhanathan ; @robinvd noticed a bug that this introduces; namely that the cursor is displayed in the incorrect position when you've scrolled. This is because the rendering code renders styles by pairing up Rows of text with rows of styles; since we edit the text rows they get out of sync. It's an easy fix; just need to map over the styles and subtract the number of omitted rows from the 'Row' of each style (the types are a bit confusing though); https://github.com/ChrisPenner/rasa/blob/master/rasa-ext-slate/src/Rasa/Ext/Slate/Internal/Render.hs#L89

ghost commented 7 years ago

Cranked up a solution quickly.

diff --git a/rasa-ext-slate/src/Rasa/Ext/Slate/Internal/Render.hs b/rasa-ext-slate/src/Rasa/Ext/Slate/Internal/Render.hs
index 7bfa935..efc113f 100644
--- a/rasa-ext-slate/src/Rasa/Ext/Slate/Internal/Render.hs
+++ b/rasa-ext-slate/src/Rasa/Ext/Slate/Internal/Render.hs
@@ -82,10 +82,14 @@ renderView (width, height) (vw, buf) = appendActiveBar . resize $ textImage
     availHeight = if isActive then height - 1
                               else height

-    trimText = Y.concat . take availHeight . drop (vw^.scrollPos) . Y.lines'
+    trimText = fmap padSpaces . take availHeight . drop (vw^.scrollPos) . Y.lines

     resize = V.resize width availHeight
-    textImage = applyAttrs atts txt
+    textImage = applyAttrs' atts txt
     txt = buf^.text & trimText
-    atts = buf^.styles & fmap (fmap convertStyle)
+    atts = buf^.styles & fmap (fmap convertStyle) & traverse.mapped %~ AttrMonoid
+         & combineSpans & traverse._2 %~ attr'
+         & filter ((\(Coord row _) -> row >= vw^.scrollPos && row <= (vw^.scrollPos + availHeight)) . fst)
+         & fmap (\((Coord row column), a) -> (Coord (row - vw^.scrollPos) column, a))
+    padSpaces = (`Y.append` "  ")
     isActive = vw ^. active

I just inlined the applyAttrs function. Works, but not pretty.

ChrisPenner commented 7 years ago

Fixed in https://github.com/ChrisPenner/rasa/pull/30

I make Span/Range/Coord into bifunctors which cleaned it up a bit; also added some annotations for future readability.