Skip to content

Conversation

@ztmzzz
Copy link
Contributor

@ztmzzz ztmzzz commented Sep 17, 2024

#117
The display of images has been initially implemented, but due to issues with BubbleTea or its UI library (I guess), the image rendering appears strange.
When I print the image directly to the terminal using fmt.Print, it looks fine.
However, when integrated with BubbleTea, the image suffers from unusual cropping.

The workaround I have found so far is to reduce m.preview.Height and scaling the image to a specific resolution. But the image resolution is unrelated to the size of the terminal window.
Below is a table I tested, representing the maximum resolution at which the image can be correctly displayed on the terminal. The resolution of the image is ( windowWidth*scale.x , windowHeight*scale.y ).

type m.preview.Height scale.x scale.y system.scale
kitty 1/2 9 9 100%
kitty 1/4 9 13 100%
kitty 1/2 18 20 200%
kitty 1/4 18 30 200%
sixel 1/2 14 16 200%

The display quality is poor when dealing with very tall images. Sometimes, the image only shows in the upper half of the terminal. However, it performs well with regular square images, which might be sufficient for preview purposes.

Users need to manually modify the configuration file to set ImageDisplay to either kitty or sixel. ImageScaleX and ImageScaleY represent scale.x and scale.y in the table, HeightCut represents m.preview.Height. These values need to be adjusted according to the actual situation. Personally, I recommend setting HeightCut to 2, as there's no noticeable difference compared to other values.

@ztmzzz ztmzzz marked this pull request as draft September 17, 2024 09:27
@ztmzzz ztmzzz marked this pull request as ready for review September 17, 2024 09:56
@savedra1
Copy link
Owner

savedra1 commented Sep 21, 2024

@ztmzzz Apologies for the delay in response and thank you so much for this contribution! Amazing work. I do have one suggestion. I think the image display preferences in the config.json file should be represented as it's own nested object for better organization. Something like this:

  "imageDisplay":  {
     "type": "basic",
     "scaleX": 9,
     "scaleY": 9,
     "heightCut": 2
  }

This can then be added to the main Config struct as a map[string]interface{}.

What do you think? There may have been a reason for this proposed structure that I've missed. Also, it would be good to address the CI failures if possible.

Thank you

@ztmzzz
Copy link
Contributor Author

ztmzzz commented Sep 24, 2024

I have modified the config structure and passed the CI.

@savedra1
Copy link
Owner

@ztmzzz I have just tested locally using kitty, it works very well! Again, thank you for your work on this 💙 I will aim to get the a new release out soon!

@savedra1 savedra1 merged commit d57a340 into savedra1:main Sep 28, 2024
@Marrieto
Copy link

Marrieto commented Oct 2, 2024

I'm looking forward to the update! Thanks @ztmzzz! 🥇

@savedra1 savedra1 mentioned this pull request Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants