-
Notifications
You must be signed in to change notification settings - Fork 55
chore: update to ratatui 0.30.0 #141
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
Conversation
b8d4e31 to
6dba881
Compare
|
Formatting seems to be an issue, i formatted and built with |
|
Have you tried building the examples locally? For me |
Also turns the examples folder into a workspace for easy compiling of all the examples at once and easy updating of shared dependencies.
| // NOTE: need to clone this because of signature and lifetime annotation change in | ||
| // Shader::cell_iter in https://github.com/junkdog/tachyonfx/pull/42/files#diff-5671d25d4f836e2bf332bb7fb82756277aa49796ac1f32565a3615d58ea943df | ||
| // apply effect to each cell in the area | ||
| let cell_iter = self.cell_iter(buf, area); | ||
| let filter_processor = self.filter_processor().cloned(); | ||
| let cell_iter = CellIterator::new(buf, area, filter_processor.as_ref()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
happy to hear better ways if this was skill issue on my part
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can copy the impl from https://github.com/junkdog/beamterm/blob/main/examples/canvas_waves/src/wave_effect.rs - it's been updated to the latest tachyonfx api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't recall what the fix was, possibly the cell_iter.for_each_cell()
|
@orhun fixed the examples and changed into workspace for easier spotting of compiler errors. Haven't ran all of them, but they all compile. |
|
unrelated, but i get very low fps on the demo app, like <=1 |
|
yeah... we're still investigating that low FPS issue. cc @junkdog I think I'll merge this whenever we figure out what's going on. Thanks! |
|
as @j-g00da pointed out on discord, we're missing the "layout-cache" feature for the ratatui dep. it should perform fine once it's added. |
|
yeah, that's a lot better after enabling that feature. seems like a good default to enable in ratzilla? |
|
yeah, i don't think it's viable running without the layout cache atm, at least until the perf issues are solved in ratatui |
|
Yup generally layout cache should only be disabled on no_std targets and/or if you want to optimize for memory. Doesn't make sense at all do disable it on ratzilla. |
|
lgtm |
|
fwiw dom backend on demo still tops out at 20 fps with average at 10 fps |
what fps are you getting with ratatui 0.29? the dom backend doesn't really perform that well when there's a lot going on (iirc, it spends a lot of cpu time in the browser's layout engine when cells change) |
|
looks more like 20-30, the thing i like about dom backend is being able to select text and copy it |
|
i stand corrected, seems like there is a way to copy text in webgl backend too |
yeah, as long as it's enabled with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
- The
Cargo.lockneeds to be updated demo2example has some deprecation warnings- I didn't experience any performance issues
8f560f7 to
6d9bca2
Compare
| [patch.crates-io] | ||
| # remove this after ratatui gets updated upstream | ||
| tui-textarea = { git = "https://github.com/0xferrous/tui-textarea", rev = "b6bf812d1f5edab4f311f56d405a47341e9423cf" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think tui-textarea is not getting any updates, it might make sense to fork it in ratatui org and apply the patch and use that url here instead of my personal fork @orhun
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes... we should definitely do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool will update when it is forked on ratatui org, lmk


No description provided.