Closed rmartine-ias closed 9 months ago
Thanks for the detailed ticket, this is awesome! A few thoughts:
viuer
doesn't support gifs. You can display gifs in viu
but it's because it renders each frame in a loop. So this is not great because it would only work on some terminals.
viu
's print_from_file
basically reads the image and calls print
with it so this is actually worse than what we have today as we read each image only once.viuer
will use but we can't because the API in viuer
is way too simplistic. So if we wanted to go this route we'd have to either duplicate the logic to determine this or create a ticket/PR in viuer to have a nicer API. e.g. something where you create a "printer" or something and you can tell which protocol you'll use.flush
es stdout). It's possible there's more happening though, like viuer doing syncs or the terminal simply taking time to show the image.
Anyhow, I think this would be a great addition but I'm mostly concerned about a) this only working in iterm2 and us having issue determining when to use each function in viuer b) the fact that the sync seems to be wrong, but we could probably solve this.
For a), does viuer::is_iterm_supported
work? Playing gifs could be a progressive enhancement where supported.
I figured out the layout issue; I'm dumb -- it occurs when a block quote (or other element) wraps and this isn't taken into account for image placement (happens for non-gifs as well, even without this patch):
Also, as written this code breaks presenting the demo from the git root, because the doge.png
path is relative to demo.md
, not the CWD. (cd
ing into examples/
fixes this)
For a), does viuer::is_iterm_supported work?
Yep, that should work. Looks like wezterm supports the iterm2 protocol so I should be able to test this on linux too.
it occurs when a block quote (or other element) wraps
Hmm the wrap around is expected but this should update the cursor location when that happens. I'll have a look.
because the doge.png path is relative to demo.md
Yeah this is intentional. I don't really want your presentation to break/work depending on what your cwd
is. Making it relative to the presentation file makes it consistently work (or not).
Thanks for uncovering all these issues!
Okay both gif support and the preformatted block wrap around bug are merged. Can you double check please? Thanks!
Both work great! The overlapping is completely fixed, but the size without wrapping seems a bit smaller than before.
Edit: Aha, this happens when a wrap has started but doesn't go to the next actual line
I was trying to display a gif with presenterm in iTerm2, and found it rendered the first frame, but didn't animate it. It would be very useful if I could embed gifs in presentations.
viuer
is able to handle this case, but it seems to need to print from a file, as otherwise it encodes the image as a PNG. Here's how theviu
tool does it.This code has issues, but allows presenterm to display animated gifs in iTerm2. It also significantly speeds up loading slides that have large images on them, for some reason.
```diff diff --git a/src/builder.rs b/src/builder.rs index 5c9021d..a1f95a5 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -367,7 +367,7 @@ impl<'a> PresentationBuilder<'a> { fn push_image(&mut self, path: PathBuf) -> Result<(), BuildError> { let image = self.resources.image(&path)?; - self.chunk_operations.push(RenderOperation::RenderImage(image)); + self.chunk_operations.push(RenderOperation::RenderImage {image, path}); self.chunk_operations.push(RenderOperation::SetColors(self.theme.default_style.colors.clone())); Ok(()) } diff --git a/src/diff.rs b/src/diff.rs index 47bd691..2aaa972 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -71,7 +71,7 @@ impl ContentDiff for RenderOperation { (RenderText { alignment: original, .. }, RenderText { alignment: updated, .. }) if original != updated => { false } - (RenderImage(original), RenderImage(updated)) if original != updated => true, + (RenderImage { image: original, .. }, RenderImage{ image: updated, ..}) if original != updated => true, (RenderPreformattedLine(original), RenderPreformattedLine(updated)) if original != updated => true, (InitColumnLayout { columns: original }, InitColumnLayout { columns: updated }) if original != updated => { true diff --git a/src/presentation.rs b/src/presentation.rs index 6c75430..36de14f 100644 --- a/src/presentation.rs +++ b/src/presentation.rs @@ -5,7 +5,7 @@ use crate::{ theme::{Alignment, Margin, PresentationTheme}, }; use serde::Deserialize; -use std::{fmt::Debug, rc::Rc}; +use std::{fmt::Debug, rc::Rc, path::PathBuf}; /// A presentation. pub(crate) struct Presentation { @@ -361,7 +361,7 @@ pub(crate) enum RenderOperation { RenderLineBreak, /// Render an image. - RenderImage(Image), + RenderImage { image: Image, path: PathBuf}, /// Render a preformatted line. /// diff --git a/src/render/engine.rs b/src/render/engine.rs index 16a467c..9812e93 100644 --- a/src/render/engine.rs +++ b/src/render/engine.rs @@ -13,7 +13,7 @@ use crate::{ style::Colors, theme::Alignment, }; -use std::{io, mem}; +use std::{io, mem, path::PathBuf}; pub(crate) struct RenderEngine<'a, W> where @@ -54,7 +54,7 @@ where RenderOperation::JumpToBottomRow { index } => self.jump_to_bottom(*index), RenderOperation::RenderText { line: texts, alignment } => self.render_text(texts, alignment), RenderOperation::RenderLineBreak => self.render_line_break(), - RenderOperation::RenderImage(image) => self.render_image(image), + RenderOperation::RenderImage{ image, path } => self.render_image(image, path), RenderOperation::RenderPreformattedLine(operation) => self.render_preformatted_line(operation), RenderOperation::RenderDynamic(generator) => self.render_dynamic(generator.as_ref()), RenderOperation::RenderOnDemand(generator) => self.render_on_demand(generator.as_ref()), @@ -132,10 +132,10 @@ where Ok(()) } - fn render_image(&mut self, image: &Image) -> RenderResult { + fn render_image(&mut self, image: &Image, path: &PathBuf) -> RenderResult { let position = CursorPosition { row: self.terminal.cursor_row, column: self.current_rect().start_column }; MediaRender - .draw_image(image, position, self.current_dimensions()) + .draw_image(image, path, position, self.current_dimensions()) .map_err(|e| RenderError::Other(Box::new(e)))?; // TODO try to avoid self.terminal.sync_cursor_row()?; diff --git a/src/render/media.rs b/src/render/media.rs index 0767b7b..2f8cec3 100644 --- a/src/render/media.rs +++ b/src/render/media.rs @@ -1,6 +1,6 @@ use crate::render::properties::WindowSize; use image::{DynamicImage, ImageError}; -use std::{fmt::Debug, io, rc::Rc}; +use std::{fmt::Debug, io, rc::Rc, path::PathBuf}; use viuer::ViuError; use super::properties::CursorPosition; @@ -41,6 +41,7 @@ impl MediaRender { pub(crate) fn draw_image( &self, image: &Image, + image_path: &PathBuf, position: CursorPosition, dimensions: &WindowSize, ) -> Result<(), RenderImageError> { @@ -73,12 +74,13 @@ impl MediaRender { let start_column = dimensions.columns / 2 - (width_in_columns / 2) as u16; let start_column = start_column + position.column; let config = viuer::Config { + width: Some(width_in_columns), x: start_column, y: position.row as i16, ..Default::default() }; - viuer::print(image, &config)?; + viuer::print_from_file(image_path, &config)?; Ok(()) } } ```https://github.com/mfontanini/presenterm/assets/107639398/16a1377c-0425-4619-befa-59d028454f8f
Some things that the above code wants:
Testing/fallback for sixel support -- I couldn't get building
presenterm
to findlibsixel
so didn't go too far here.viu
has a fallback method it uses.Support for kitty protocol; I didn't test this at all
Correct positioning/resizing; this layout bug appears if there's not enough space:
Understanding of why this speeds up loading large PNG images, to make sure those gains are realized (i.e. maybe don't switch to only printing from file for gifs, but maybe for all iterm2 images?)
When you navigate to a slide with a gif, it takes a second for the gif to load in. Are there easy fixes for this? (Delay showing slide, have stand-in rectangle displayed so gif popping in is less jarring)
cat output
and get the gif displayed, and it still took ~1s.