3rd / image.nvim

🖼️ Bringing images to Neovim.
MIT License
1.13k stars 49 forks source link

Luarock magick abandoned #124

Open raffaem opened 9 months ago

raffaem commented 9 months ago

Luarock magick (luarock site, github repo) seems abandoned.

Last commit was Mar 11, 2022, almost 2 years ago.

It works only with lua==5.1 (not lua>5.1).

Since the default version of lua for arch linux is 5.4, the command to install it on arch linux is actually:

sudo pacman -Syu lua51
luarocks --local --lua-version=5.1 install magick
  1. How it will go in the future?
  2. Should the actual command be put in the README (certainly tou need the --lua-version=5.1 argument to luarocks or it won't install)
  3. Would something like luarocks nvim plugin gonna work to install it?
3rd commented 9 months ago

I don't like that we have to use the magick rock, and I've been thinking of alternatives. It's definitely a big part of what made this project possible initially, but rocks are super finicky, sometimes Neovim loads them, sometimes it doesn't, some days a master build of Neovim won't load magic at all, and a lot of people have trouble installing and even monkey-patching it.

I'm currently rewriting the plugin from scratch, but don't know what to do about magick yet, no many ideas.

Here's what we need:

Option 1 I could write a separate utility to do the image processing we need, probably in Go for portability and speed. Feature wise this would be perfect as we could implement everything we need ourselves, and it would be much more performant. The only issue is that I don't know if requiring people to build/install an additional binary in their system is ok, and it feels better just to pull down a plugin.

Option 2 We could use the CLI version of ImageMagick by spawning shell commands. This would probably be slower than using the bindings, and, again, I'm not comfortable with having multiple places that spawn shell commands, especially when we have to feed strings that come from a document that possibly wasn't authored by the user who is opening it.

I want to like rocks in Neovim, but the experience so far hasn't been fun. I'm lucky to be using NixOS, where other people went through the pain of making installing rocks for Neovim easy, but I get how frustrating it is, before I moved to this home-manager setup I had to monkey-patch the library.

Do you think you'd be ok / it would be better if you just built/installed a go program that did all the image processing instead of dealing with the rock?

Also pinging for opinions/ideas :pray: : @benlubas @SigmaRichards @teto @jmbuhr

Edit: just updated my neovim :joy: image

Edit 2: turns out lazy.nvim does some weird things, if I remove it the rock loads fine :-?

raffaem commented 9 months ago

I could write a separate utility to do the image processing we need, probably in Go for portability and speed.

There are plenty of image processing libraries and utilities. I wouldn't write my own. (1) no need for it (2) you will have to spend months and years fixing bugs

We could use the CLI version of ImageMagick by spawning shell commands.

This seems the correct thing to do

This would probably be slower than using the bindings

Are you sure the magick rock is not calling the CLI utility under the hood anyway?

And in any case, the performance difference will be really small.

I'm not comfortable with having multiple places that spawn shell commands, especially when we have to feed strings that come from a document that possibly wasn't authored by the user who is opening it.

This is interesting. You can check if the path is a file that really exist and send it over. If it's not a file, exit.

I don't know about Lua, but Python for example allows you to subprocess.run(["command","arg"]).

So you cannot put arg="' rm -rf / and expect it to work, because that entire string is read as a command line parameter ((that is, the initial ' doesn't close anything).

I bet there is something similar for LUA too.

I looked for "magick" in luarocks, but all the results seem outdated, including the ones listed here.

If we can spur the magick command asynchronously, I would go with that.

Do you think you'd be ok / it would be better if you just built/installed a go program that did all the image processing instead of dealing with the rock?

I think you would reinvent the wheel and spend a lot of time fixing bugs for very marginal if even measurable performance improvements.

bew commented 9 months ago

What about completely offloading the work to programs like timg or chafa (or others), that knows very well how to convert images to either terminal symbols or real images/animation.. I think it would avoid much of wheel re-inventing and instead might lead thise tools to be usable as a backend. 🤔

Parsifa1 commented 9 months ago

I think it would be better to reimplement a utility program. I have not been able to successfully use this plug-in so far (most likely due to luarock), and I cannot get any error information. I think a go program would be better🥰🥰

raffaem commented 9 months ago

Can we take a step back and detail why we need ImageMagick in the first place?

Because this plugin doesn't seem to need it: https://github.com/adelarsq/image_preview.nvim

3rd commented 9 months ago

Can we take a step back and detail why we need ImageMagick in the first place?

Because this plugin doesn't seem to need it: https://github.com/adelarsq/image_preview.nvim

That one uses something like Kitty's icat kitten for wezterm, it's a utility that reads the pixels from the images and renders them. We are tied to PNG because we integrated Kitty's graphics protocol directly: https://sw.kovidgoyal.net/kitty/graphics-protocol/#transferring-pixel-data

raffaem commented 9 months ago

Can we take a step back and detail why we need ImageMagick in the first place? Because this plugin doesn't seem to need it: https://github.com/adelarsq/image_preview.nvim

That one uses something like Kitty's icat kitten for wezterm, it's a utility that reads the pixels from the images and renders them. We are tied to PNG because we integrated Kitty's graphics protocol directly: https://sw.kovidgoyal.net/kitty/graphics-protocol/#transferring-pixel-data

And why we can't do the same, with increase in performance (no need for image conversion) and dropping of imagemagick dependency?

3rd commented 9 months ago

Can we take a step back and detail why we need ImageMagick in the first place? Because this plugin doesn't seem to need it: https://github.com/adelarsq/image_preview.nvim

That one uses something like Kitty's icat kitten for wezterm, it's a utility that reads the pixels from the images and renders them. We are tied to PNG because we integrated Kitty's graphics protocol directly: https://sw.kovidgoyal.net/kitty/graphics-protocol/#transferring-pixel-data

And why we can't do the same, with increase in performance (no need for image conversion) and dropping of imagemagick dependency?

The kittens can only render images in the shell, we need to render images at certain locations, crop them, resize them, etc.

raffaem commented 9 months ago

Then I don't understand how that plugin works.

In effect, it looks like the screenshot in the showcase is opening a new shell?

In any case, it doesn't seem to work for kitty.

What about this one: https://github.com/mbpowers/nvimager?

This doesn't seem to use ImageMagick as well

3rd commented 9 months ago

Then I don't understand how that plugin works.

In effect, it looks like the screenshot in the showcase is opening a new shell?

In any case, it doesn't seem to work for kitty.

What about this one: https://github.com/mbpowers/nvimager?

This doesn't seem to use ImageMagick as well

That one uses ueberzug, which we support, but it's much slower than Kitty and doesn't have crop support.

jmbuhr commented 9 months ago

I think sticking with magick is the way to go because it handles all our image needs (even for things we don't yet do but might in the future like e.g. gifs) and is actively maintained. Writing your own tool is certainly possible, but I think will grow in complexity a lot. Once you start adding filetypes to convert you'll have to write so many parsers and ImageMagick can already do it all.

The question to me is just how to interface with it.

They list a bunch of interfaces in various languages here: http://www.imagemagick.org/script/develop.php, but some of them like the lua rock may not be up to date. The lua one uses the foreign function interface, so it hooks into magick's C api (http://www.imagemagick.org/script/magick-wand.php) direclty, which I believe would have the best performance.

However, I do wonder how critical performance of the interface is, when using the CLI would allow us to decouple from Magick internals, so that would be a lot less work than e.g. maintaining the lua bindings ourselves.

benlubas commented 9 months ago

abandoned

The maintainer responded to a PR yesterday.

It works only with lua==5.1 (not lua>5.1).

Since the default version of lua for arch linux is 5.4, the command to install it on arch linux is actually...

Yes, this command should be put in an installation guide. I'm pretty sure nvim embeds 5.1 so I don't see a problem.

Is there currently a problem with the library? (Other than it's hard to install). Or are we just scared it will break one day and the maintainer won't be there?


As far as installation problems go, I really think that's a shortcoming of nvim package managers at the moment. Something like rocks.nvim or the aforementioned nix makes installing this plugin much easier.

In the near future, it might be worth asking people to install this plugin with rocks instead of their normal package manager.

benlubas commented 9 months ago

I tend to agree that calling shell commands when we have FFI bindings is not something we should do. This plugin should be as performant as possible (bc we're already fighting the terminal)

jstkdng commented 9 months ago

That one uses ueberzug, which we support, but it's much slower than Kitty and doesn't have crop support.

i was planning to add crop support in the coming days, testing from this project would be useful.

Poly2it commented 7 months ago

If you're going to write your own image manipulation utility, please consider using a language like C which already compiles on most computers.

3rd commented 7 months ago

If you're going to write your own image manipulation utility, please consider using a language like C which already compiles on most computers.

I'm thinking of Go / Zig :man_shrugging: but we're not there yet, will explore it in a few months.

luanlouzada commented 7 months ago

I'm having the same issue right now. I installed luaenv to see if I could downgrade the Lua version without affecting other things I use in the current version.

And luaenv on Arch Linux simply doesn't have luaenv install.

I don't know if I did something wrong.

I had already given it a chance because I use Alacritty and would have to configure Kitty from scratch. Now, with Magick presenting another hurdle, I'll wait for a solution.

benlubas commented 7 months ago

@luanlouzada you can now use the luarocks.nvim plugin to install the rock.

{
  "vhyrro/luarocks.nvim",
  priority = 1001,
  opts = { 
    rocks = { "magick" },
  },
},

Obviously refer to the luarocks.nvim README still. And you will still need the image magick c binary b/c the rock uses it.

vhyrro commented 7 months ago

Just chiming in on the convo :p

@3rd rocks can be annoying sometimes, but I still think going with it and making the experience more stable is the lesser evil here.

Creating a custom library for image interactions is in something other than lua is basically creating the same portability problem but under different circumstances. The current state of Neovim plugin management just isn't quite ready for such advanced external binary management yet, not without sufficient knowledge and time from the user's end.

There are two approaches that might help you out here (learned these the hard way when migrating Neorg to use luarocks):

There's also something to be said about rocks.nvim, but until that has gained sufficient traction I would not rely on that as an installation method.

I haven't read through all of the issues, but if you have a specific issue with luarocks/magick you're always free to give me a ping, I can probably help out here and there! :)

glyh commented 5 months ago

Kitty's graphics protocol is render agnostic. Tier 1 PNG support exists merely as a tool of acceleration. You can pass the whole matrix of pixels to kitty and it can render with no problem. I don't know about efficiency though, as to whether convertion to PNG and then pass to kitty is faster or the other approach. Did you test it out?

https://sw.kovidgoyal.net/kitty/graphics-protocol/#rgb-and-rgba-data

EDIT: It seems we need to rescale, nevermind.

flexagoon commented 5 months ago

@3rd wouldn't it be possible to create your own Lua FFI bindings for magick? It doesn't seem to hard and you don't have to implement all magick calls, only the ones this plugin needs.

3rd commented 5 months ago

What I'll do is make a "processor" abstraction, where the magick bindings will be just an implementation and:

kiyoon commented 1 month ago

I've re-packaged the luarocks package for neovim, and modified it slightly so that it will locate the local installation of magick as well.

Instead of using luarocks, you can just install this plugin.

https://github.com/kiyoon/magick.nvim

justinmk commented 3 weeks ago

add another one that calls magick through the regular CLI

Yes, please! Bindings are a completely irrelevant and unnecessary installation step/dependency. Showing an image(s) in a viewport is not a performance-sensitive operation.