Closed tfuxu closed 1 year ago
fast-colorthief works very well as a drop in replacement, thanks for the suggestion. However, I ran into some issues adding it to the flatpak manifest: flatpak-pip-generator seems to be unable to add the pypi link. I think this is because the project is missing source distribution files on pypi. I'm not familiar enough with flatpak manifests to get the git repo working as a dependency directly
I got it working. The first issue, lack of source distribution I was able to workaround using req2flatpak, an tool very similar to flatpak-pip-generator
, but it priorities built distribution (.whl
files) instead of source distribution.
It has an added benefit that building a flatpak is now blazingly fast, because instead of building all of the dependencies (where Pillow is a really big library, and it can take up to ~4 minutes to compile its C part), it just unpacks .whl
files and installs them to the final Flatpak package.
This is the command I've used to generate Flatpak requirements using req2flatpak
:
req2flatpak --requirements-file requirements.txt --target-platforms 38-x86_64 39-x86_64 310-x86_64 311-x86_64 > pypi-dependencies.json
The second issue I stumbled across is that it appears that fast-colorthief
isn't as drop-in of a replacement as I anticipated it to be, but it fortunately only has some minor changes from the original (it doesn't uses a class to store image's URI, has additional numpy
dependency, get_color()
was renamed to get_dominant_color()
) and I was able to adapt Paleta codebase to those changes without any problem.
Also my recommendation, as I was able to quickly review your code, is to remove match...case
statements as they have been added in Python 3.10, but all of the dependencies for Paleta and GTK 4 would allow users of Python 3.8 to run it, if not those new logic statements.
I've created a PR to accommodate my changes: https://github.com/nate-xyz/paleta/pull/10
I'm working on a rust rewrite right now, so we should get similar performance increases from colorthief-rs https://github.com/RazrFalcon/color-thief-rs
Oh, so you are rewriting Paleta to Rust? That's pretty cool!
This may lower the amount of potential contributors, as there are less people who know Rust and gtk-rs-core
crate, but I see the point of porting Paleta to it. Everything will be definitely faster than when written in interpreted language, and GTK Rust crate looks much more maintained than a Python library (and has an tutorial that isn't 3+ years outdated).
You should consider publishing an entry on TWIG about that, as this may attract more Rust users to your project. You should also inform users here, in repo, that you are currently rewriting Paleta, so they can know about it and don't start on implementing new features in a old codebase.
Also, do you have some estimates when you'll should be done with a rewrite? I've been working on fixing translations, as some elements aren't translated even if they are in gettext _()
closure, but I don't know if my changes would be relevant anymore if you are working on that rewrite.
It'll be done within a week at the latest, I have the drop page working with extraction multithreaded, just need to implement the palette page & the dialog windows. Hopefully by tomorrow or monday just depends on how much time I can find. I think the translations should work if we go through and manually fix everything as long as I don't change the strings, but I haven't tried i18n stuff with rust yet so I don't know for sure
I think the translations should work if we go through and manually fix everything as long as I don't change the strings, but I haven't tried i18n stuff with rust yet so I don't know for sure
From what I checked you can just use gettextrs
crate and use gettext borrow like that:
&gettext("This test is translatable!")
But I don't know how you could go with formatting translatable text.
I'm going off of what amberol does: https://gitlab.gnome.org/World/amberol/-/blob/main/src/i18n.rs
I pushed to a branch: https://github.com/nate-xyz/paleta/tree/rust_rw
Try it out and let me know if you find any issues. Still not sure how to generate pot file, meson + xgettext doesn't seem to work out of the box with rust. I think I need to use xtr https://crates.io/crates/xtr according to gettext-rs https://crates.io/crates/gettext-rs
Didn't tested rewrite yet, but I found how a way to generate translation files. You simply need to use ninja -C <builddir> paleta-update-po
command.
Tested Rust rewrite, compiles without any problems, loads images properly in any of the three possible ways and the palette extraction is now blazingly fast.
The only problem I've noticed is that saving process can take ~4 seconds for 5 colors and almost 20 seconds for a palette of 30 colors, so some optimization should definitely be taken into consideration here.
Also, translations works! Formatted strings still aren't translated, but this is, as we know, expected.
I'm working on fixing the saving speed, I think I just need to use transactions properly with rusqlite
Ok, try this commit: https://github.com/nate-xyz/paleta/commit/16df683c53804de8e6bcf9b415c216f778613ca4
Yes, transactions fixed the issue with saving, you should use them also in delete_palette
function, as it has the same problem with bigger palettes.
Cool, now it works properly for other palette operations too.
I guess I can close this issue, as 0.3.0 has been released.
@tfuxu I didn't update po files w/ v0.3.0, will update soon
There is an faster version of
colorthief
Python library, namedfast-colorthief
written in C++ and usespybind
to bind it as a Python library. From a benchmark presented in README, it says that Fast Colorthief is nearly 15x faster when used with a 1200x1200 pixels image.Such a boost would be beneficial for more advanced users, like photographers with cameras that can produce in 4K or higher, but it can also benefit users that just want to get color palette quickly from a high-definition photo.
From what I quickly checked, the original and the C++ version have the same function names, and probably also all of the methods, so
fast-colorthief
should work like a drop-in replacement for originalcolorthief
library.