-
Notifications
You must be signed in to change notification settings - Fork 37
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
Luarock magick abandoned #124
Comments
I don't like that we have to use the magick rock, and I've been thinking of alternatives. 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 Option 2 I want to like rocks in Neovim, but the experience so far hasn't been fun. 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 🙏 : Edit: just updated my neovim 😂 Edit 2: turns out lazy.nvim does some weird things, if I remove it the rock loads fine :-? |
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
This seems the correct thing to do
Are you sure the And in any case, the performance difference will be really small.
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 So you cannot put 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
I think you would reinvent the wheel and spend a lot of time fixing bugs for very marginal if even measurable performance improvements. |
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🥰🥰 |
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. |
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. |
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. |
The maintainer responded to a PR yesterday.
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. |
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) |
i was planning to add crop support in the coming days, testing from this project would be useful. |
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 🤷♂️ but we're not there yet, will explore it in a few months. |
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. |
@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. |
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! :) |
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. |
@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. |
What I'll do is make a "processor" abstraction, where the magick bindings will be just an implementation and:
|
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:
--lua-version=5.1
argument to luarocks or it won't install)The text was updated successfully, but these errors were encountered: