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.
All tests seem to be passing now.
One asymmetry: I'm setting node.editor.top in one branch but not the
other. Let's see if that comes back to bite us..
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.
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.
Ugh, it's been in front of my eyes all along. The logs have been
repeatedly showing a recurrence within a single frame:
key press -> update Viewport.y from screen_top -> A -> B -> update_editor_box -> update screen_top from Viewport.y
We need both those updates, but both should never occur at the same
time to the same node.
I think this is why screen panning works if I take out the scale inside
update_editor_box: the scale has already happened "once" in
updating Viewport.y, and it doesn't converge if we perform it a second
time in a frame. But the solution is just to do one or the other to a
node, never both.
I knew this (learned it the hard way) when I first built pensieve.love,
and I had an assertion to avoid it. But my assertion was brittle, and it
kept failing, and I took it out, and then I slowly took out all the code
that prevented this recurrence.
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.
Panning with a cursor inside a node is working fine.
The relationship between y_of_schema1 and schema1_of_y is preserved.
I understand why I shouldn't scale the y in the call to schema1_of_y.
I'm also feeling a little more confident about why lines.love shouldn't
use coordinate transforms. The problem is that text gets blurry if it
starts at non-integer coordinates. We're forced to get into special
cases.
There's still the outstanding issue that the surface y coordinate of
each screen line is not consistent as you pan around (and the editor
starts off-screen above the viewport).
If you have just text boxes it's only noticeable when one box's top
margin is visible and another is not. Then the text in the two moves
relative to each other.
This is a big change, and I'd have to first modify lines.love to use the
coordinate transforms. And that fork doesn't really need a principled
coordinate system.
Unscaled y makes cursor scrolling more stable, but it seems to be
compensating for an error elsewhere. The location of the top margin of
text is not stable, and I only notice when I can see the bottom margin
of the bounding box. (When the top margin is visible we never enter that
branch in update_editor_box.)
I should just use LÖVE's standard translation and scaling transforms for
the surface! Don't know why I didn't think of that.
scenario:
* zoom in or out
* focus cursor in a large box of text that overflows viewport both above
and below
* position cursor near bottom, hit down arrow repeatedly. cursor should
remain in viewport, with the viewport panning as necessary.
* position cursor near top, hit up arrow repeatedly. cursor should
remain in viewport, with the viewport panning as necessary.
Again, I don't really understand when I'm supposed to scale coordinates
vs not. But at least we have a manual test pinned down now, and it
passes.
I spent 4 days agonizing over a bug in driver.love, working up the
courage to think about it, and then a whole day trying to make sense of
scrolling and deeply questioning my right to tell anyone anything about
programming.
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?