Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for multiframe KITTY_SELFREF made it slooooooooooooooow #2161

Open
dankamongmen opened this issue Sep 12, 2021 · 6 comments
Open

fix for multiframe KITTY_SELFREF made it slooooooooooooooow #2161

dankamongmen opened this issue Sep 12, 2021 · 6 comments
Assignees
Labels
bitmaps bitmapped graphics (sixel, kitty, mmap) bug Something isn't working perf sweet sweet perf

Comments

@dankamongmen
Copy link
Owner

The fix for #2143 led to unacceptable slowdown in the yield demo. On "grimes", my T580 laptop, the 2.4.0 release runs in 53.33s. Current master runs in 117.74s, almost a 100% slowdown! erp!

@dankamongmen dankamongmen added bug Something isn't working perf sweet sweet perf bitmaps bitmapped graphics (sixel, kitty, mmap) labels Sep 12, 2021
@dankamongmen dankamongmen added this to the 3.0.0 milestone Sep 12, 2021
@dankamongmen dankamongmen self-assigned this Sep 12, 2021
@dankamongmen
Copy link
Owner Author

backing off to KITTY_ANIMATION from KITTY_SELFREF recovers almost all of the performance; I'm disabling KITTY_SELFREF until we've dealt with this. i think i've got some ideas over in the comments of #2143.

@dankamongmen
Copy link
Owner Author

though guess what...bitmapstates has some broken frames for KITTY_ANIMATED, lolololol. nothing's simple. (wipes look broken)

@dankamongmen
Copy link
Owner Author

so ideas include (these all need inspection of the protocol to see if they'd work):

  • writing the new frame to somehow replace the original frame, maintaining wipes (v. good)
  • writing the new frame and then reflecting the existing wipes rather than reblitting them (less good)

@dankamongmen
Copy link
Owner Author

though guess what...bitmapstates has some broken frames for KITTY_ANIMATED, lolololol. nothing's simple. (wipes look broken)

we've got these just about handled with my latest fix.

@dankamongmen
Copy link
Owner Author

ok, figured out the KITTY_ANIMATION deal with bitmapstates. right now, we break on the third-from-last frame, aka the first "full square on right". we nail the last frame, the second "full square on right". we're sending r=2,c=1 to get this result. if i change it to r=1,c=2, we invert, nailing the first one, and missing the second. so yeah, we're not copying a nulled-out version into our auxvecs -- we're doing that correclty. we are writing a first frame with nulled cells. r=2,c=2 misses on both. r=1,c=1 misses all manner of things. r=1,c=2 breaks intro when we get to the right.

so i think we need to recognize whether we blitted the original frame with nulls. if we did, use r=1,c=2. otherwise, use r=2,c=1. wipe ought work either way, since it's not sensitive in this way to the original blit. we need this information on a per-sprixcell basis. we could just store this as an extra byte in each auxvec. i think that would get it.

@dankamongmen
Copy link
Owner Author

yep, works perfectly, w00t!

dankamongmen added a commit that referenced this issue Sep 13, 2021
Much as we needed to do in the accounting core for
kitty animation, whether we blitted a sprixcell of
alpha nulls or instead wiped it affects how we
rebuild under KITTY_ANIMATION, which right now is
all Kitty >= 0.20.0 since KITTY_SELFREF is disabled.
This gets KITTY_ANIMATION working perfectly on
bitmapstates. I've accomplished this by stashing a
bool onto the end of each KITTY_ANIMATION-style
auxvector, set high if it originated in a reblit's
null alphas, and low if it was zorched by a wipe.
Set {c, r} arguments to animation directive based
on whether this is set. See #2161.
@dankamongmen dankamongmen removed this from the 3.0.0 milestone Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bitmaps bitmapped graphics (sixel, kitty, mmap) bug Something isn't working perf sweet sweet perf
Projects
None yet
Development

No branches or pull requests

1 participant