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

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

Closed
Gipson62 opened this issue Aug 26, 2024 · 15 comments · Fixed by #32
Closed

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

Gipson62 opened this issue Aug 26, 2024 · 15 comments · Fixed by #32
Labels
bug Something isn't working windows Something that occurs on windows

Comments

@Gipson62
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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 josueBarretogit added bug Something isn't working windows Something that occurs on windows labels Aug 28, 2024
@josueBarretogit
Copy link
Owner

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.

@josueBarretogit josueBarretogit pinned this issue Aug 28, 2024
@Gipson62
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

josueBarretogit added a commit that referenced this issue Aug 29, 2024
@josueBarretogit
Copy link
Owner

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
Copy link
Contributor Author

Gipson62 commented Aug 29, 2024

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
Copy link
Contributor Author

Gipson62 commented Aug 30, 2024

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
Copy link
Contributor Author

Gipson62 commented Aug 30, 2024

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor Author

Gipson62 commented Aug 30, 2024

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
Copy link
Owner

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working windows Something that occurs on windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants