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

Conversation 1 Commits 6 Files Changed 2

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.

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()

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.

da5a3bae3e

generate-alt-random: assert that lines/cols/width/height are > 0

44b8bd2364

generate-alt-random: wait for SIGWINCH if width/height is 0

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.

10e512f14f

generate-alt-random: do ioctl(TIOCGWINSZ) on /dev/tty, not stdout

dnkl added 1 commit 1 year ago

a9236129f6

generate-alt-random: don’t run TIOCGWINSZ ioctl if --cols and --rows were used

dnkl added 1 commit 1 year ago

033e1bd6ae

changelog: generate-alt-random-writes.py --sixel fixes

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.

The pull request has been manually merged as 070f102605.

Sign in to join this conversation.

No reviewers

bug

No Milestone

No Assignees

1 Participants

Notifications

Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

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