horrible bouncing effect in fullscreen video in xterm

[dankamongmen]

In an XTerm, running ncplayer -bpixel samoa.avi results in a horrible bouncing effect. I believe we've seen this before, and it's due to dropping sixels into the last row of the screen. We recently changed things to make use of the last row for Kitty graphics, but this ought not have affected Sixel. Figure out what's going on.

Also, alacritty isn't showing the frame count on the top row anymore -- probably the same deal. =[

[dnkl]

Looks like you're no longer emitting \E[?8452h. I.e. the cursor is now positioned on the row under sixels.

[dnkl]

#1782 (comment) same bug, I think

[dankamongmen]

oh yeah definitely, good call, so #1786 would fix this

[dankamongmen]

hrmmm, nope, i merged that but am still seeing the issue.

[dankamongmen]

yeah, we're definitely hitting sixel_blit() with 61 target rows, equal to the terminal height.

[dankamongmen]

we're 1403x880 and blitting 1398x880. pretty much none of that makes sense. source is 1484x1080. sigh.

[dankamongmen]

we're only checking for bitmap_bottom_row in make_sprixel_plane(), it appears. we need to check it along all paths. this is broken in the 2.3.4 release, btw, so fixing it might call for a 2.3.5.

[dankamongmen]

hrmmmm...we can't have the bottom row check just live in ncvisual_blit(), because what if we move the plane down afterwards? =[ hrmmm we need a general solution for that. shit.

the correct way to do this is to note when a sprixel plane is being rendered such that it will cross the bottom border, where in the sixel case that border is one row up, and (temporarily) truncate the sprixel. that's for the future, though. and i don't like it as a solution to this problem, because it would involve encoding the sixel twice for every frame.

by the way, turning while((outy + nc->tcache.cellpixy - 1) / nc->tcache.cellpixy > ncplane_dim_y(n)) into while((outy + nc->tcache.cellpixy - 1) / nc->tcache.cellpixy > ncplane_dim_y(n) - !nc->tcache.bottom_row_enabled) does not help, due to post-blit resizing. we want to fill the plane, when we can.

no, we need either cut the plane by one row, or check for the bottom elsewise. the former requires exposing this grotesque restriction to userspace, and making them care about it. perhaps it's better to just clamp sixel_maxy? that would require clamping large sixel_maxy values to the visual area, which has a complication: if we do it, and then we resize larger, we'd not be able to tell the difference between a sixel_maxy capped under the old value, and a true sixel_maxy. i suppose we could keep the unclamped sixel_maxy around as sixel_maxy_pristine, and have a working sixel_maxy? erp.

[dankamongmen]

if we did that, we could drop bottom_row_enabled entirely, and just always go through sixel_maxy when it is non-zero...yeah, this isn't too bad of a solution.

[dankamongmen]

no, we can't kill off bottom_row_enabled, because we'll need it to reclamp sixel_maxy, sigh.

[dankamongmen]

no, because in that case sixel_maxy_pristine is 0, and thus sixel_maxy is 0, ok good.

[dankamongmen]

yep, that fixed at least xterm

[dankamongmen]

kitty still looks fine...

[dankamongmen]

works with xterm with margins of 1 (i.e. we still write to the second-to-last row)...

[dankamongmen]

kitty works with margins=1

[dankamongmen]

both work with margin=2, looks like a good fix.,..

[dankamongmen]

alacritty now draws framecounts...i think we're good!