This is a do-over of parts of commits b32a60548 and f1de276ce.
The key ideas are:
if edit.update_font_settings receives a font object, it's already of the requested font_height
if update_editor_box receives a font, it's already of the requested height scale(20)
if B receives a font, it's already of the requested height scale(20)
whether B receives a font or not,
all the editors on Surface will share a single font object when it's done, and
the font object will have the right size
A sets the font and passes it in to all the above functions.
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.
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.
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.
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?
This makes the keyboard navigation feel more solid.
I'm not bothering to do the same for panning with the mouse. Mostly
because I'm lazy, but also as an escape hatch in case I find myself
wanting to move the top lower down the screen or something. But it'll
still snap back the first time I pan using the keyboard. Let's see how
much it matters.
There's one remaining issue: the DOM is changing between mouse press and
mouse release, which is confusing the select handler.
Options:
* some sort of hacky thing to ignore the first mouse release
* put the file picker handlers on mouse release rather than mouse
press
There's some precedent for the latter option even if it might get
confusing in the presence of drag operations. But that seems confusing
either way. Just be consistent.
This will make things more consistent in the long term, but I realize
one major cost: our button abstraction doesn't work well with luaML and
compute_layout. So we need something to replace it.
This required a little more restructuring of Global_state. It's not flat
now, it's hierarchical again, but in a different way.
after commit 3d89b8eb9d:
post
\- reply A
\- reply B
\- reply C
after commit f9f7dab9b7:
post
reply A
reply B
reply C
after this commit:
row
\- col
\- indent 0
\- post
\- col
\- indent 1
\- reply A
\- col
\- indent 2
\- reply B
\- col
\- indent 1
\- reply C
The indents are just invisible rectangles of different widths with 0
height.
One change I had to make was to initialize_editor. Neither luaML and
driver load lines from disk, but that's arguably the common scenario to
support.