Basic idea: If the goal is for the cursor to be on the viewport, focus
the code on ensuring that constraint by construction.
Motivation: The downstream driver.love fork still has persistent bugs.
And I'm seeing some inconclusive signs that edit.lua might be failing to
change screen_top some of the time when it needs to. But this only
happens in driver.love, never in lines.love. So the null hypothesis is
that there's some subtle assumption in lines.love that we're violating
when rendering it on a surface.
What do you do with such subtleties? It might actually be
counterproductive to fix them at source. You end up with complexity
upstream that won't actually matter there if it breaks. Which is a
recipe for it to break silently and far away from the downstream fork
that might actually care about it. Or it might confuse people in future
who don't care about the downstream forks, just lines.love.
Maybe it makes sense to modify edit.lua here and take the hit on all
possible future merge conflicts. But considering the cost of tracking
this down, it seems simplest to:
a) come up with the constraint I care about, and
b) modify outside edit.lua, either what it sees or its results, to
preserve the new constraint.
Long ago I used to have this assertion in pensieve.love that the cursor
must be within the viewport, but I ended up taking it out because it
kept breaking for me when I was trying to do real work. It seems clear
that there are possible assertions that are useful and yet
counterproductive. If you can't keep it out of the product in the course
of testing, then it annoys users where ignoring it would be a more
graceful experience. Even when the user is just yourself! So it turns
out this is not a problem only for large teams of extrinsically
motivated people who don't eat their own dog food. No, for some things
you have to fix the problem by construction, not just verify it with an
assertion.
This plan isn't fully working yet in this commit. I've only fixed cases
for down-arrow. I need to address up arrow, and there might also be
changes for left/right arrows. Hmm, I'm going to try to follow the
implementation of bring_cursor_of_cursor_node_in_view() in
pensieve.love.
In the process of doing this I also noticed a bug with page-up/down. It
already existed, but this approach has made it more obvious.
scenario:
position a tall node with its top within the viewport, and extending past bottom of viewport
press page-down
Before this commit we were seeing strange patches of empty space above
the old top.
scenario:
zoom out a couple of times
pan until a tall node's bottom margin is visible
position cursor near top of viewport within it
hit up arrow a few times
Before this commit scrolling the node caused its bounding box to go out
of alignment.
In the process we find a new bug. Scrolling with keyboard is overly
eager to clamp screen_top to bottom of screen when the top used to be
within the viewport.
Until recently, scrolling past the bottom when the margin was visible
would move the cursor correctly but pan the surface to the top of the
viewport. Slightly jarring, but good enough.
There's a misunderstanding. edit thinks the cursor line will be rendered
and doesn't scroll. But edit.draw doesn't actually render the cursor
line.
And this only happens when Viewport.zoom ~= 1.
And lines.love is _definitely_ not seeing this problem.
scenarios:
* Zoom = 1
* pan with mouse: ✓
* pan with up arrow: ✓
* pan with down arrow: ✓
* Zoom < 1
* pan with mouse: ✓
* pan with up arrow: ✓
* pan with down arrow: ✗
* Zoom > 1
* pan with mouse: ✓
* pan with up arrow: ✗
* pan with down arrow: ✓
What ✓ means:
* pan with mouse: lines don't slide relative to the surface
* will still slide relative to the surface when zooming in/out;
that's unavoidable because we want integer pixels for crisp text
* pan with keyboard: at least some part of cursor is always peeking within the viewport
* might still look ugly, with the line containing the cursor almost invisible,
but hitting the down arrow will never pan upwards, or vice versa
Still not working though. I'm pretty much guaranteeing by construction that if
Viewport.y was set from screen_top1, then screen_top1 will not be perturbed.
And yet using scale() inside update_editor_box is incorrect. Hmm..
Well, almost. I'm just reminding myself of the sort of plumbing I need,
not reintroducing the old logic that never worked right and had
undergone n iterations of corruption.
I might have finally hit on the right approach: a hotkey that dumps
information. Doesn't swamp me with data, and also doesn't perturb
anything.
y_of_schema1 returns consistent results as I pan around.
I'm just not actually printing the lines at that y. I'm printing it at
that y/Viewport.zoom.
What might be confusing here is that I started out with a simple mental
model:
* perform computations in surface coordinates (sx,sy)
* render in viewport coordinates (vx,vy)
But for text quality reasons I need to perform many computations in
"scaled surface" coordinates.
Viewport coordinates are both offset and scaled relative to surface
coordinates. Scaled surface = just scaled relative to surface, not
offset.
I don't have a clear mental model here for when to use this.
I did already use it in one place with my simple mental model: you have
to scale distances like rect.w and rect.h but it's incorrect to offset
them. Maybe I'm getting this wrong somehow.
I need to run A() because I'm scaling the font, and that requires
recomputing heights.
Things were different at the start of this project, because I scaled the
font rather than reinitialize it when zooming.
I still see strange artifacts where a box seems overly long as its
bottom border rises above the bottom edge of the viewport. But as it
continues to rise it snaps to the right height for the text.
One benefit: now I don't need to redundantly set font twice in a single
frame.
I don't know why this was so hard, but I don't need this variable
preserve_screen_top_of_cursor_node at all. We only set it when the
cursor is in some node, but we also only check for when the current node
is the cursor. Comparing with a nil cursor node works just as well.
I've also checked that driver.love doesn't need
preserve_screen_top_of_cursor_node. I think it came from pensieve.love,
where I've since taken it out. Did I ever need it even there?
scenario:
- create a new definition with ctrl+n
- type in an initial comment
- hit enter
- type in a definition like 'Foo = nil'
- hit F4
- close and restart the driver
Before this commit, the definition no longer showed up on the surface.
(Adding it a second time worked fine.)
I wish I'd included this fix in merge commit 1267010251 back in July.
I created this bug when I implemented GET*, but it was already latent
before that, when I lazily used get_cmd_from_buffer to get the
definition name from buffer instead of creating two different names with
identical definitions.
I'm not sure why I thought it was necessary to put it in there.
The only difference in behavior from this commit: selecting text and
hitting 'f4' used to delete the selected text before submitting. Now it
no longer deletes. I've never relied on this.
'f4' needs no viewport book-keeping or A/B refreshing, so this should
also be slightly less wasteful.