dnkl/foot
41
2.0k
Fork
You've already forked foot
243

Mix of fixes for generate-alt-random-writes.py #454

Manually merged
dnkl merged 6 commits from generate-alt-random-writes-fixes into master 2021年04月22日 11:27:40 +02:00
Owner
Copy link

Running foot generate-alt-random-writes.py --sixel would occasionally fail with a ValueError: empty range for randrange() here.

This happened whenever generate-alt-random-writes.py started fast enough for it to do the TIOCGWINSZ ioctl before the foot window had been mapped. At this point, the TIOCGWINSZ response is a hardcoded lines=24, cols=80, width/height=0. The lines/cols are basically placeholders - we don't have a window size yet, thus we don't have an actual lines/cols count yet. Width/height being zero is due to the fonts not yet having been loaded - we don't have a cell geometry yet.

This PR updates the generate-alt-random-writes.py script to detect width/height=0, and then wait for a SIGWINCH signal before trying again. Not all terminals fill in anything in the ws_xpixel and ws_ypixel fields in the TIOCGWINSZ response; generate-alt-random-writes.py will now hang on these terminals. But this is ok, because a) it isn't intended to be run on anything else, and b) it used to crash in those terminals anyway.

Alternatives: if we decide to implement #453, we could use that in the PKGBUILD. Perhaps rather than an alternative, it should be considered an enhancement to this PR; with this PR, we typically don't get the final window size (when run on a tiling compositor) - we get an intermediate window size: foot's initial-widow-size-{pixels,chars}. For our purposes, generating profiling data, this should be ok.

Running `foot generate-alt-random-writes.py --sixel` would occasionally fail with a `ValueError: empty range for randrange()` [here](https://codeberg.org/dnkl/foot/src/commit/4044b6fa99d6cd464ad926993059c6e76fc81d14/scripts/generate-alt-random-writes.py#L179). This happened whenever `generate-alt-random-writes.py` started fast enough for it to do the `TIOCGWINSZ` ioctl **before** the foot window had been mapped. At this point, the `TIOCGWINSZ` response is a hardcoded lines=24, cols=80, width/height=0. The lines/cols are basically placeholders - we don't have a window size yet, thus we don't have an actual lines/cols count yet. Width/height being zero is due to the fonts not yet having been loaded - we don't have a cell geometry yet. This PR updates the `generate-alt-random-writes.py` script to detect width/height=0, and then wait for a `SIGWINCH` signal before trying again. Not all terminals fill in anything in the `ws_xpixel` and `ws_ypixel` fields in the `TIOCGWINSZ` response; `generate-alt-random-writes.py` will now hang on these terminals. But this is ok, because a) it isn't intended to be run on anything else, and b) it used to crash in those terminals anyway. Alternatives: if we decide to implement https://codeberg.org/dnkl/foot/issues/453, we could use that in the PKGBUILD. Perhaps rather than an alternative, it should be considered an enhancement to this PR; with this PR, we typically don't get the **final** window size (when run on a tiling compositor) - we get an intermediate window size: foot's `initial-widow-size-{pixels,chars}`. For our purposes, generating profiling data, this _should_ be ok.
When randomizing the sixel width and height, use width/height + 1 in
the call to randrange(). This ensures the width/height is never 0, and
that the maximum width/height is half the window size, not slightly
less than that.
If width/height (that is, the *pixel* values) are 0, that means we are
early (or that foot is slow) - we’ve managed to reach this point
before the foot window has been mapped.
Or rather, before foot has loaded the primary fonts and calculated the
cell geometry. But that, and the initial mapping of the window is
tightly coupled.
To handle this case, detect when width or height is 0, an then wait
for SIGWINCH before trying again.
dnkl manually merged commit 070f102605 into master 2021年04月22日 11:27:40 +02:00
Author
Owner
Copy link

Perhaps rather than an alternative, it should be considered an enhancement to this PR; with this PR, we typically don't get the final window size (when run on a tiling compositor) - we get an intermediate window size: foot's initial-widow-size-{pixels,chars}

Just realized that an --wait-for-mapped option wouldn't really improve the situation; we wouldn't know we're on a tiling WM and that we'll get a second configure event soon again. I.e. we'd have to spawn the client application after the first configure event, which corresponds to the intermediate window size.

> Perhaps rather than an alternative, it should be considered an enhancement to this PR; with this PR, we typically don't get the final window size (when run on a tiling compositor) - we get an intermediate window size: foot's initial-widow-size-{pixels,chars} Just realized that an `--wait-for-mapped` option wouldn't really improve the situation; we wouldn't know we're on a tiling WM and that we'll get a second configure event soon again. I.e. we'd have to spawn the client application after the **first** configure event, which corresponds to the intermediate window size.
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
dnkl/foot!454
Reference in a new issue
dnkl/foot
No description provided.
Delete branch "generate-alt-random-writes-fixes"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?