josueBarretogit / manga-tui

Terminal-based manga reader and downloader with image support
https://crates.io/crates/manga-tui
MIT License
439 stars 11 forks source link

Images aren't displayed in the correct place or at the correct size #26

Closed Gipson62 closed 1 week ago

Gipson62 commented 2 weeks ago

I'm on Windows, using Wezterm, and just installed manga-tui with cargo and Wezterm with the :simple-windows: Windows (setup.exe) to install it. I then run it and here's what I get. There's nothing in the log file and I didn't touch anything in the config file.

image

josueBarretogit commented 2 weeks ago

In order to display images the font-size of the terminal needs to be know, and on windows it's hard to get that information, so I put arbitrary values.

Gipson62 commented 2 weeks ago

Thanks, that makes so much more sense now! Hope one day it'll be possible to fix it 😅 tho it's still a really great tool to dl mangas. You did an amazing job!

josueBarretogit commented 2 weeks ago

Thanks, that makes so much more sense now! Hope one day it'll be possible to fix it 😅 tho it's still a really great tool to dl mangas. You did an amazing job!

Thanks for your appreciation! for more information about this see this issue, basically I also have a windows machine but switched to linux because my windows 11 got very slow, rust-analyzer would use up like 1 to 2 gb of ram (alone), so it was very difficult coding on it and using the windows crate to get the font size is really difficult, every function is unsafe and couldn´t find documentation or tutorials.

Gipson62 commented 2 weeks ago

I tried searching for answers, but yeah the only way to get the size of a terminal would be to know the font size, but it doesn't seem straightforward at all... tho, one temporary fix could be to use a constant value and later fix it if you find any solution (and a little disclaimer for Windows users to not resize their font or else everything will break) But that doesn't feel like a real fix sadly

Gipson62 commented 2 weeks ago

I just spent 2 hours trying to figure out a way and... imho Windows has a shitty API. It doesn't even do what it says 😞. Whenever I ask for a size or whatever, it only returns me (rows, columns) and when asking for the size of the font I get it in logical unit which doesn't make sense and is unusable.... I'll try tomorrow again, but not sure if I can get any results sadly

Gipson62 commented 2 weeks ago

After more searching, I found this: https://github.com/microsoft/terminal/issues/6395#issuecomment-640260972 which is kinda sad... cuz it means it's literally impossible to retrieve the font size via the official Windows API

josueBarretogit commented 2 weeks ago

After more searching, I found this: microsoft/terminal#6395 (comment) which is kinda sad... cuz it means it's literally impossible to retrieve the font size via the official Windows API

so what's the purpose of GetConsoleFontSize if it doesn't get the font-size, thanks for taking your time looking into this btw

Gipson62 commented 2 weeks ago

I guess it's for backwards compatibilities, it was supported on some old Windows, and they dropped it cuz "it was a bad design decision". But ye, the function still exists and that's annoying asf. I'll still try to see if it's possible with another way tho.

Gipson62 commented 2 weeks ago

Ok, so, after a lot of researches I found out there was a settings.json where you could specify the fontSize (in point) and you could translate this somewhat into pixels with the DPI (GetDpiFromSystem()). But that means, you'll have to create a terminal profile for the terminal used by the users (not sure if Wezterm or so supports it) and/or go fetch the data directly through their APIs if it's supported.

C:\Users\<YOUR_USERNAME>\AppData\Local\Packages\Microsoft.WindowsTerminal_8wekyb3d8bbwe\LocalState\settings.json

It's not a complete fix tho and won't work if the user resizes the window, but it's somewhat working (somewhat...)

Gipson62 commented 2 weeks ago

On my computer, this works somewhat decently

12 & 24 will be replaced with correct value, but they work for someone who has a 125% scaling on their computer (it's DPI related)

fn main() {
   unsafe {
        let h_console: HANDLE = GetStdHandle(STD_OUTPUT_HANDLE);
        if h_console.is_null() {
            eprintln!("error get_std_handle");
            return;
        }
        let mut info = CONSOLE_SCREEN_BUFFER_INFO {
            dwSize: windows_sys::Win32::System::Console::COORD { X: 0, Y: 0 },
            dwCursorPosition: windows_sys::Win32::System::Console::COORD { X: 0, Y: 0 },
            wAttributes: 0,
            srWindow: SMALL_RECT {
                Left: 0,
                Top: 0,
                Right: 0,
                Bottom: 0,
            },
            dwMaximumWindowSize: windows_sys::Win32::System::Console::COORD { X: 0, Y: 0 },
        };
        if GetConsoleScreenBufferInfo(h_console, &mut info) == 0 {
            eprintln!("error getting screen buffer info");
            return;
        }

        let pixel_width = 12 * info.dwSize.X as i32;
        let pixel_height = 24 * info.dwSize.Y as i32;
    }
}

When it'll work correctly, I'll do a PR, rn I still need to find a way to replace the literal value with stuff taken from the Windows API so it works on every devices.

josueBarretogit commented 2 weeks ago

Ok I'm going to update the test workflow so that it runs on windows, if you have any questions about the code please let me know

Gipson62 commented 2 weeks ago

Happy news, it does seem to somewhat work better:

Before

image

After

image

It still doesn't work when checking an actual manga. I don't know why... And I'd like to know where do you display manga pages in the actual code, so I can check if there are any issues there

Gipson62 commented 2 weeks ago

Ok so, they do seem to display, but not directly, sometimes you need to switch multiple times the page to get the page you want to display. It looks like this: image

As if the draw function or something similar isn't called each time to remove the previous chapter and put the new one in place

josueBarretogit commented 2 weeks ago

It was like this before on kitty terminal, what a did is replace StatefulImage with Image, in the manga reader page its still using StatefulImage since it easier to resize the image depending on it's dimensions

It still doesn't work when checking an actual manga. I don't know why... And I'd like to know where do you display manga pages in the actual code, so I can check if there are any issues there

go to: src/view/pages/reader.rs

This is the code responsible for reading each manga panel


impl Component for MangaReader {
    type Actions = MangaReaderActions;

    fn render(&mut self, area: Rect, frame: &mut Frame<'_>) {
        let buf = frame.buffer_mut();

        let layout =
            Layout::horizontal([Constraint::Fill(1), Constraint::Fill(self.current_page_size), Constraint::Fill(1)]).spacing(1);

        let [left, center, right] = layout.areas(area);

        Block::bordered().render(left, buf);
        self.render_page_list(left, buf);

        Paragraph::new(Line::from(vec!["Go back: ".into(), Span::raw("<Backspace>").style(*INSTRUCTIONS_STYLE)]))
            .render(right, buf);

        match self.pages.get_mut(self.page_list_state.selected.unwrap_or(0)) {
            Some(page) => match page.image_state.as_mut() {
                Some(img_state) => {
                    let (width, height) = page.dimensions.unwrap();
                    if width > height {
                        if width - height > 250 {
                            self.current_page_size = 5;
                        }
                    } else {
                        self.current_page_size = 2;
                    }
                    let image = StatefulImage::new(None).resize(Resize::Fit(None));
                    StatefulWidget::render(image, center, buf, img_state);
                },
                None => {
                    Block::bordered().title("Loading page").render(center, frame.buffer_mut());
                },
            },
            None => Block::bordered().title("Loading page").render(center, frame.buffer_mut()),
        };
    }
Gipson62 commented 2 weeks ago

It's weird... if you just change the image, it'll be displayed as a tiny image in the bottom right of the screen and if you force the draw call by zooming in and out, it'll be displayed correctly 🤔 . But the images are actually in their correct place and with the correct size.