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

Manually merged
dnkl merged 6 commits from generate-alt-random-writes-fixes into master 1 year ago
dnkl commented 1 year ago
Owner

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.
dnkl added the
bug
label 1 year ago
dnkl added 4 commits 1 year ago
17bc2f5070
generate-alt-random: use {width,height} + 1 in randrange()
44b8bd2364
generate-alt-random: wait for SIGWINCH if width/height is 0
dnkl added 1 commit 1 year ago
dnkl added 1 commit 1 year ago
dnkl merged commit 070f102605 into master manually 1 year ago
dnkl commented 1 year ago
Poster
Owner

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.
The pull request has been manually merged as 070f102605.
Sign in to join this conversation.
Loading…
There is no content yet.