akazukin5151 / koneko

Browse pixiv in the terminal using kitty's icat to display images (in the terminal!)
https://koneko.readthedocs.io/en/latest/
GNU General Public License v3.0
51 stars 3 forks source link

Icat image rendering inconsistencies #34

Closed VGrol closed 1 year ago

VGrol commented 1 year ago

I'm running into some issues with the rendering of images. When starting Koneko, I get the following issue; The welcome picture displays over the text.

image

A similar issue that I can't resolve, seems to be mode 3 display, each row of artists displays over the other, effectively rendering several rows of artists, all at the same [z] coordinates. Meaning only one row is visible.

I tried seeing if ueberzug functions, but seeing as that repo was deleted, I can't get it to function. None of the variables in the config seem to affect this, even extreme values. The other display modes work fine for me however. The only variable that "seems" to change anything is scroll_display, if it is enabled, I can see each row being drawn one by one, over the other. Where as when it is off, it seems to render instantly.

I'm running the following versions

Koneko 0.12.4
Kitty 0.26.5,
Sway (Wayland) 1.7
Arch Linux 5.15.80-1-lts
akazukin5151 commented 1 year ago

Thanks for the bug report. The welcome picture is definitely weird because the text is hardcoded to print 30 spaces to offset it to the left. Perhaps your terminal is wrapping the text? I'll have a look

I'm pretty sure the mode 3 behavior is intended, you just need to scroll up. Have you tried that? IIRC that mode was notorious for display offsetting issues, so putting each row on a new "screen" was easier with the current architecture. The (~hypothetical~) Haskell rewrite shouldn't have this problem

I wasn't aware that ueberzug was no longer supported. Perhaps you can try a fork like this one? https://github.com/WhiteBlackGoose/ueberzug-latest. I'll have a look and update the dependencies accordingly

VGrol commented 1 year ago

Is there a setting I should look for to determine or make sure the text isn't wrapped by kitty?

Scrolling up just scrolls the entire row down, with more empty space coming down. It doesn't "unstack" them or anything of the sort.

I'll have a look if I can get around to trying the ueberzug fork.

Side note, I noticed that, because I installed via pip, the pixiv-url.desktop file ran into some issues because it was assuming a global programcall was installed. Since it was not, I had to reference the source of the pixiv handler.

So changing oneko-url-login %u in the pixiv-url.desktop to $HOME/.local/bin/koneko-url-login resolved it. Might be related to the OS though, as Arch generally doesn't like program definitions all that much. Not sure if I want me to make a second issue for that or not, but I figured I'd mention it.

akazukin5151 commented 1 year ago

For the welcome screen, I think it's because the image is too large for this font size. I'll add a config to customize this value and the welcome image size. Meanwhile, if you want to test this theory, try increasing the number of spaces from 30 to a larger number here

Edit: FYI, the manual is incorrect, the config assistant should be lscat 1 8 instead of lscat 1 7, but if none of the settings worked then this isn't relevant. Fixed in ad73ba6

akazukin5151 commented 1 year ago

Checkout 9f4d382 and run pip install .. In the config add

[welcome_screen]
spaces_to_offset = 30
image_size = 600

And either increase the spaces or decrease the image size, or both. This should fix the welcome screen issue


As for the mode 3 issue, scroll_display is definitely buggy but I can't reproduce your problem. (Another FYI, use use_ueberzug = off and scroll_display = on to make debugging easier. Let's fix one bug at a time). I've updated kitty to 0.26.5. I wonder if it's a wayland issue? I've never tested with wayland. For me, kitty's icat has a side effect of moving the cursor around, which affects printing text. icat images also moves as you scroll. Ueberzug images does not move as you scroll and does not move the cursor. The behavior you described sounds like ueberzug's behavior, so I suggest trying ueberzug.

akazukin5151 commented 1 year ago

As for the pixiv-url.desktop issue, the XDG Desktop specification states:

The executable program can either be specified with its full path or with the name of the executable only. If no full path is provided the executable is looked up in the $PATH environment variable used by the desktop environment.

So koneko assumed that $HOME/.local/bin is in $PATH, which IMO is reasonable but apparently not in a standard I can find. It wouldn't hurt to use the full path anyway, and it's better than adding yet another step for the user. Thanks for pointing this out!

VGrol commented 1 year ago
[welcome_screen]
spaces_to_offset = 30
image_size = 600

This fixes the welcome screen entirely for me. Somewhere around 70 seems ideal. This seems to fix it irregardless of scroll_display. But I will say that scroll_display absolutely breaks every mode for me, as everything stretches into the z axis for me. So I'm not sure if that is where the problem lies. It probably is what makes the logic of scrolling up for mode 3 work though...

I'll try to see if euberzug works, but yeah, I wouldn't be surprised if wayland really doesn't like icat.

VGrol commented 1 year ago

Hm, some testing later and I'm fairly certain that ueberzug does not work with wayland and I feel a bit dumb for assuming it did, haha.

Also you might want to see if you can actually get the new euberzug repo to work with koneko, because in my case it had some errors with finding the directory. Though troubleshooting on my end might not be worth while due to it's inherent incompatibility with wayland.

VGrol commented 1 year ago

Oh while I'm pointing things out,

have you noticed that the [back] hotkey 'b', only seems to work in the inherent mode? So for example, if I go to mode 5, select image 3, row 1 by hitting 31, I can use b to go back to the page I was on in mode 5. but I cannot use it in mode 5 to go back to the starting screen and switch modes.

It's not really a problem but it's a minor inconsistency that, while there is a back button, it requires a program exit to switch modes.

akazukin5151 commented 1 year ago

Ah, of course ueberzug won't work with wayland because it depends on X11. Same reason why it doesn't work on mac.

Can you run this code and report what you see? I see three images arranged diagonally from top left to bottom right. Also, my screen is small enough that the terminal scrolled down to display the last image.

import pixcat

for x in [0, 30, 60]:
    pixcat.Image('pics/71471144_p0.png').thumbnail(300).show(x=x)

This uses the picture in the koneko dir, but you can use any path to any picture. This is designed to horizontally space out the images so that icat would never decide to "split into multiple rows"

VGrol commented 1 year ago

This is my output on my usual terminal size. image

Interestingly enough, restricting the terminal size and scrolling seems to work and keeps the images in the format above.

akazukin5151 commented 1 year ago

Yes I've noticed that back button inconsistency, but to me it wasn't so critical to fix. Can you confirm what your setting for page_spacing is? And does it match the value that lscat 1 4 recommends?

VGrol commented 1 year ago

I'm sorry, I'm not exactly certain what you mean by lscat 1 4. where should I look for that? As for the page_spacing value in the koneko config.ini, that one is set to 18. Interestingly enough, not a single part of the application is affected by this setting. from 0, to 4, to 40, to even 4000 doesn't seem to do anything.

akazukin5151 commented 1 year ago

Well that's indeed strange, but I might have a solution that bypasses all this even if I can't reproduce it. I should probably sleep and work on this tomorrow though

As for lscat 1 4, it is a terminal command so just type that in your terminal and press enter. lscat -h will explain that 1 4 is for the page spacing config assistant. Not like it matters if it doesn't do anything, but it'll still be needed in the solution above

VGrol commented 1 year ago

Oh right, ran into the same issue as before... lscat isn't recognized so I had to call the $HOME/.local/bin/lscat :) The value seems to be optimal around 63, but as you said, it doesn't really matter.

No worries, I should definitely do the same myself, thanks for all the help so far.

akazukin5151 commented 1 year ago

Checkout 2c337b2, run pip install ., and try again. This reduces reliance on page_spacing to separate images, but not entirely. It relies on correct image_height, images_y_spacing, and page_spacing settings, though it won't completely collapse if page_spacing is somehow broken. Run lscat 1 3 to get recommended settings for the first two, and use 63 for page_spacing.

If page_spacing is broken, images will still display on top of each other (in the same z coordinate), but as long as (image_height + images_y_spacing) * 2 < terminal height, there should be multiple rows.

Remember to set use_ueberzug = off and scroll_display = on, at least for the first test. Let's focus on one bug at a time

VGrol commented 1 year ago

Fantastic, that actually fixes everything on my end. Interestingly enough, it also fixes the other display modes with scroll_display = on.

scroll_display = on seems to be responsible for allowing page changes on mode 3, because that seems to break without, but other than that regardless of that setting, the entire app functions fantastically for me. Thanks for all the hard work ^^

akazukin5151 commented 1 year ago

That's good to hear. Tomorrow I'll clean up the other issues raised, like absolute path of lscat and ueberzug fork. I'll also look at scroll_display = off. Thanks for your patience!

VGrol commented 1 year ago

No worries.

I'm also doing some testing and I think the images_x_spacing might be broken. All off the following below is with scroll_display = on Based on lscat 1 2 and lscat 1 3, I should use the following settings;

image_width = 32
image_height = 17
images_x_spacing = 8
images_z_spacing = 3

Now interestingly enough, my previous settings;

image_width = 40
image_height = 20
images_x_spacing = 1
images_z_spacing = 2

Were pretty much the same total value, but I'm noticing that images_z_spacing is uh... finnicky at best. It seems to apply to the entire row. with absurd values like 30, it shoves images next to eachother, while creating a fairly large amount of empty space before starting the image.

The same happens in mode 3, where the artist and their 3 most recent works of art are just all shoved into a row of 4 adjacent images. It seems changing the value seems to mostly control the entirety of the 3 pictures, not them individually.

Example: images_x_spacing = 1 image

images_x_spacing = 10 image

images_x_spacing = 30 image

and finally, leaving it at 0, but with cranked up image_width image

Ideally I'd also like for a way to add a small bit of empty space on the entire x axis, so I manually center koneko in the middle of the terminal. I tried it with a negative images_x_spacing but that uh... wasn't fantastic. it involved a lot of cutoff images.

-- EDIT --- Some further testing after the fact, I'm fairly certain the issue of "having to scroll up" now also applies to mode 5, where as it seems to fetch a few too many images for "full" screen and then only display the bottom 2 rows, requiring you to scroll up to see the first full page, just like mode 3 did. But all of that can be avoided with fullscreen, where it seems to work entirely fine. Both for image quantitity, and not having to scroll up for mode 5. Still required for 3 however.

scroll_display = off does fix this, but introduces other issues, like not being able to scroll back to images.

I'm also fairly certain gallery_print_spacing does not work at all, and images_y_spacing only seems to work for some modes. (does not affect mode 3 atleast) Interestingly enough, the only way I could get the spacing to turn out to be even is with images_y_spacing at -1 and image_height at +2px over what lscat recommends.

I'm personally not too fussed, because mode 3 is what I'm most interested in and that seems to work fantastically, besides having to scroll up. but I figured I'd report my findings. It seems to just be fundamentally borked in wayland tbh.

akazukin5151 commented 1 year ago

Thank you for the report. Nothing like a good challenge!

images_z_spacing doesn't exist, did you mean images_y_spacing? And did you use z instead of y in the previous comments? For me, x is the horizontal position, y is the vertical position, and z is the depth that determines whether an image is on top or under another image


You should run lscat 1 8 to run all config assistants, because they're actually linked. For example, your selection in the thumbnail size assistant (lscat 1 1) means your image_height must be different, and possibly images_y_spacing too. (You can also run lscat for even more functionality, for example lscat mode 2 (lscat 2) lets you browse the cache offline, so you don't have to scrape pixiv every time you test)

With that said, the settings are indeed very sensitive to small changes, and small deviations will compound into larger ones. But the behavior as images_x_spacing increases is definitely not intended.


The scroll_display setting is badly named and apparently undocumented. Technically it does enable/disable the printing of newlines, but that's not the point. What it actually does is to avoid the need to scroll the terminal window. When on, it will scroll the window to display new rows, and you might have to scroll up to see the "hidden" images. When off, it will display as much images as possible in the current view and stop. You have to press arrow up/down to see the other images. Scrolling with arrow up/down is analogous to scrolling around in a long single webpage, while "scrolling" with n/p is the same as clicking the next/previous page button in pixiv. So:

where as it seems to fetch a few too many images for "full" screen and then only display the bottom 2 rows, requiring you to scroll up to see the first full page

This is intended behavior when scroll_display = on. I'm curious what you mean by fullscreen though, is it just maximizing the terminal window to fill the entire screen? The FAQ in the README did say:

I'm having problems with lscat For the best experience use the terminal in full screen

As for

scroll_display = off does fix this, but introduces other issues, like not being able to scroll back to images.

scroll_display = off "fixes" this by disabling the terminal scrolling behavior at the expense of pagination. Use arrow up/down to scroll. I couldn't find it documented anywhere so apologies!


gallery_print_spacing is only relevant when print_info is on

akazukin5151 commented 1 year ago

For images_x_spacing you should run lscat 1 8 to get the full set of recommended settings. The rule of thumb is that it should be much smaller than image_width, so

leaving it at 0, but with cranked up image_width

is the correct range. That said, 7402102 should fix the weird decrease in spacing when images_x_spacing is increased.

akazukin5151 commented 1 year ago

b495954 might fix the "images_y_spacing not working for some modes", but the labels might still mess things up; in general icat's cursor movements doesn't play well with printing text. Still, it should be slightly less buggy

akazukin5151 commented 1 year ago

For me the ueberzug fork doesn't work, I think because it's missing a dynamic lib like Xshm. Pacman still has ueberzug and it works just fine. I looked at pacman's cache and found Xshm.cpython-310-x86_64-linux-gnu.so, which doesn't seem to be generated/copied/linked in the fork. To be clear, it's not the fork's problem as it didn't change the code, so it was the original ueberzug's problem. I totally understand why the maintainer gave up on maintaining distributions of python and C programs

In the meantime I'll just link that fork (also include a link to this comment). In the long term perhaps it's worth to look at sixel, chafa or lsix

VGrol commented 1 year ago

images_z_spacing doesn't exist, did you mean images_y_spacing? And did you use z instead of y in the previous comments? For me, x is the horizontal position, y is the vertical position, and z is the depth that determines whether an image is on top or under another image

Ah apologies, that is sleep deprivation speaking, Y is Y, Z means X, haha. It is the correct value in the config, I just typed it up wrong.


You should run lscat

I have and while it did give me functional gallery_print_spacing, the values I ended up with align with my config from before, besides the discrepancy of adding a larger image_width and image_height for a more consistent display of images.

With that said, the settings are indeed very sensitive to small changes, and small deviations will compound into larger ones. But the behavior as images_x_spacing increases is definitely not intended.

Might not be a terrible idea to offer a setup wrapper of lscat during first time running, or as an option as startup, mostly because documentation is fairly sparse on it. Though images_x_spacing does just seem to not function for me regardless of mode, even with the most recent commit.


The scroll_display setting is badly named ....

Interesting, I didn't know the arrows functioned like that, I actually would really like that functionality to work with scroll_display = on. If loading previous pages on mode 3 wasn't as broken (images dissapear) with the setting off, I'd probably keep it off myself.

. I'm curious what you mean by fullscreen though, is it just maximizing the terminal window to fill the entire screen? The FAQ in the README did say...

I'm aware, I've been running each test both windowed and full screen, as my monitor is fairly large. The only discrepancy has been that mode 5 seems to load images based off the fullscreen size values. Where as mode 3 functions regardless of fixed/fullscreen. Does the logic for mode 5 pull from the system resolution or something? If so, possibly adding a variable to override that in the config could add to some better fixed size support.


I couldn't find it documented anywhere so apologies! No worries, I actually rather enjoy this functionality, it's a shame rows tend to disappear on mode 3 with it though.


https://github.com/akazukin5151/koneko/commit/7402102b15d0bbcaff8c56f452a96f851fe534aa doesn't seem to change anything on my end actually.

I have a feeling https://github.com/akazukin5151/koneko/commit/b49595497b84d9ec91a0dbc0af79c8f59d2003b6 fixes the weird double rows I was occasionally getting with mode 5, (sometimes the spacing between y rows ended up multiplying and it was weirdly inconsistent. However changing the value around now seems to do nothing now.

VGrol commented 1 year ago

Interestingly enough for the drawing images on mode 5, I seem to get 2 additional pictures to what my screen can fit. Not sure why that is happening, as otherwise in fullscreen, the 7x4 grid basically fits perfectly, then there is a weird 5th row with just 2 images at the bottom.

akazukin5151 commented 1 year ago

the discrepancy of adding a larger image_width and image_height

Does it mean lscat gave you values too small for width and height? I suppose it's a bit confusing as it configures the width/height and then the spacing. For example the first time you're supposed to hit = until the second image just touches the first image. This is the width/height. The second time you can add spacing by pressing = again.

If loading previous pages on mode 3 wasn't as broken (images dissapear) with the setting [scroll_display] off

So if scroll_display = off, going to a previous page in mode 3 causes images to disappear? Which images? What does "page" mean here?

The only discrepancy has been that mode 5 seems to load images based off the fullscreen size values. Where as mode 3 functions regardless of fixed/fullscreen.

Yes, the gallery modes (1, 5, 6) should display as much as possible in the window. Mode 3 is limited to 1 artist profile pic and their 3 previews, so there is always 4 columns no matter the window size.

Does the logic for mode 5 pull from the system resolution or something? If so, possibly adding a variable to override that in the config could add to some better fixed size support.

Do you want to override the number of columns and number of rows? Number of cols and rows depends on the image width/height and x/y spacing in the config, so you could increase the spacing. Alternatively there might be a bug (rounding up instead of down) - what's your image_width, images_x_spacing, and how many columns do you see?

No worries, I actually rather enjoy this functionality, it's a shame rows tend to disappear on mode 3 with it though.

Can you tell which rows are disappearing? The top row in the screen? The bottom? Random? If it's too fast I can insert time.sleeps

7402102 doesn't seem to change anything on my end actually.

Are you sure you're actually running that commit? After checking out you need to run pip install . again (or just pip install -e . once)

VGrol commented 1 year ago

---deleted previous reply--- I'll retest a couple of things and come back, seemed the branch conflict for me solved more than I had thought.

VGrol commented 1 year ago

Does it mean lscat gave you values too small for width and height? I suppose it's a bit confusing as it configures the width/height and then the spacing. For example the first time you're supposed to hit = until the second image just touches the first image. This is the width/height. The second time you can add spacing by pressing = again.

No, the image spacing is fine from lscat, but the images_spacing recommendations don't seem to apply to koneko, so I've had more success with just inflating the image height and width to get the desired extra spacing, as opposed to using the images_spacing variables. But that is neither here nor there, as it's been a personal tweak. I find it works a bit more reliable than images_spacing.


Do you want to override the number of columns and number of rows? Number of cols and rows depends on the image width/height and x/y spacing in the config, so you could increase the spacing. Alternatively there might be a bug (rounding up instead of down) - what's your image_width, images_x_spacing, and how many columns do you see?

I was thinking, that the logic that fits images to the fullscreen terminal seems to work very well. But the moment the window becomes fixed, it works, but nowhere near as neatly. So my question was how koneko ends up fitting all the images to a full screen terminal. Where does it get the data to pull [x] amount of rows and columns?

AKA, what is responsible for display breaking in a resized terminal, as opposed to fullscreen, since Kitty works almost flawlessly regardless of what resolution you give it.


I think the last couple of commits fix pretty much everything on my end with display issues. From overdrawing extra images, to overlapping. So I think you fixed most of the issues.

My only remaining caveat, would be support outside of fullscreen but that is kind of out of scope for this ticket. (Though scope has had no meaning for this ticket to begin with...)

So thank you so much!

akazukin5151 commented 1 year ago

That's good to hear. Apologies for the inconsistent use of branches. I can see that the image spacing settings in lscat don't perfectly match koneko, but they should still correlate.

So my question was how koneko ends up fitting all the images to a full screen terminal. Where does it get the data to pull [x] amount of rows and columns?

It calculates the number of rows as term_height // (img_height + padding) and number of columns as round(term_width / (img_width + padding)). Padding is just the spacing setting. For mode 3, there is always 4 columns (plus one for the label).

It shouldn't "break in a resized terminal", if 'resized' means not fullscreen. If instead it means after resizing then I can see the potential problems, because doesn't re-check the terminal size every time it scrolls (whether by arrow or n/p).

As for "support outside fullscreen", I think it should work well non-fullscreen if your monitor is big enough. But if it gets too small (as a rule of thumb, I'll say too small to fit the 4 columns + 1 label in mode 3), it basically requires a "mobile UI". Which I would say is out of scope of the purpose. I'll still have a look at re-checking the terminal size more frequently. Perhaps the hypothetical Haskell rewrite would enable such support, but this is basically on maintenance mode right now

VGrol commented 1 year ago

That's good to hear. Apologies for the inconsistent use of branches. I can see that the image spacing settings in lscat don't perfectly match koneko, but they should still correlate.

No worries, I figured it out, haha. I still think images_x_spacing doesn't work as intended, but atleast it doesn't straight up break display now.

It shouldn't "break in a resized terminal", if 'resized' means not fullscreen. If instead it means after resizing then I can see the potential problems, because doesn't re-check the terminal size every time it scrolls (whether by arrow or n/p).

Interesting, think it might be an issue with sway and the gaps it's creating? (comparable to i3-gaps) I feel as if the entire program is scaled to the fullscreen "traits" of the terminal. I get a ton of empty space outside of fullscreen, and it almost feels like it would 100% exactly match up if I go to fullscreen, which is what happens. The monitor should definitely be big enough, It's 2560x1440 and during "fixed" mode, the terminal is 2320x1200. It really shouldn't be a size constriction.

But I'm entirely fine waiting for the eventual hypothetical Haskell rewrite for that.

akazukin5151 commented 1 year ago

Actually after some testing it does re-check the terminal size, only to re-calculate the number of columns but not the number of rows. I don't understand what you mean by "traits" and "exactly match up". Can you illustrate where is the empty space? My monitor isn't big enough to visualize. Include the steps to reproduce if necessary, such as "open a new window, causing the WM to resize the terminal"

Edit: When I said "But if it gets too small", by "it" I meant the window, not the monitor.

Also you're right, the images_x_spacing isn't working. I'll have a look

VGrol commented 1 year ago

Just as an example. Running mode 5 or artist display in Fullscreen image

Running mode 5 or artist display in fixed (2320 x 1200) image

Basically, there is a ton of empty space to the right on the non-fullscreen version and you can see the rows aligning to the fullscreen setup. And this seems to happen regardless of settings, amount of rows, columns etc. Even if the white space was somehow shared evenly on the left and right side, it'd be a lot "fancier" as a solution than this.


Now mode three used to be just like this as well, but as of the last couple of commits, it seems to have the same blank space regardless of fullscreen or not.

Running mode 3 in Fullscreen (pip repo version) image

Running mode 3 in fixed (2320 x 1200) image

akazukin5151 commented 1 year ago

The fullscreen mode 5 pic makes sense, it's perfectly distributed because of your chosen image width can divide the screen width very neatly (meaning the remainder is small). If you look closely, the space on the right is not equal to the space on the left, because images are laid out left-aligned, and it's a coincidence that your values fit

The fixed size mode 5 is what happens when you reduce the width just enough to make it not able to fit 7 columns. I could try to make it center-aligned but I don't feel like the effort is worth it.

The fixed size mode 3 pic is surprising, I'll have a look. In the meantime 9d97a18 should fix the x_spacing

akazukin5151 commented 1 year ago

Completed

Todo

Unclear

Low priority

I'll get to the last un-checked issue soon, then close this ticket if there are no more immediate replies. Please open a new ticket for the last two issues or any other issues, because this thread is getting rather long to track. Thanks!

akazukin5151 commented 1 year ago

I've improved the documentation, which repeatedly advices the user to run lscat 1 8 for the config assistant. There are two reasons to separate koneko and lscat: 1) lscat will always be offline; 2) the code for lscat is slightly higher quality. But if/when the spaghetti koneko code is fixed (by me or a PR) then I will reconsider.

The documentation also contains the accurate information for the scroll_display setting. I'll add documentation and code quality to the roadmap so that people who modify the code is aware.

If you have any other issues please open a new ticket. Thank you so much for the bug reports and your patience!

akazukin5151 commented 1 year ago

For some reason, using $HOME/.local/bin/koneko-url-login in the desktop file fails for me, even though whereis koneko-url-login says it is indeed there. I'm reverting the absolute path, instead I'll tell users to ensure they are in $PATH (for example, symlinking it to $HOME/.local/bin/)