Skip to content
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

Update dependencies #35

Merged
merged 10 commits into from
Mar 1, 2024
Merged

Update dependencies #35

merged 10 commits into from
Mar 1, 2024

Conversation

LouisDISPA
Copy link
Contributor

@LouisDISPA LouisDISPA commented Feb 26, 2024

Hello, thank you for this amazing work. It works on my machine with only a small issue. (the color profile for thumbnails)
I quickly updated the dependencies to see if this would fix it but sadly it did not.

Here is a PR with the updates, you are free to do whatever you want with it.
If you have any request of modification feel free to ask. I have some spare time.

Copy link
Owner

@saschanaz saschanaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, thank you for the chore! Yeah, I think fixing the issue needs some more investigation (I think the issue might just be about emitting the 128bit float pixel because I guess that's a rare use case. I wonder the new jxl-oxide supports lower precision number types?

Some nits:

@LouisDISPA
Copy link
Contributor Author

Welcome, I fixed the nits!
I also added the cmd to restart explorer.exe.

I think fixing the issue needs some more investigation (I think the issue might just be about emitting the 128bit float pixel because I guess that's a rare use case. I wonder the new jxl-oxide supports lower precision number types?

Good to know, I don't have a lot of experience on image processing but I might try to have a look.

@LouisDISPA
Copy link
Contributor Author

I pushed a last commit that remove some unwrap by using pattern matching. The logic is the same but if you prefer the old code I can revert it.

@saschanaz
Copy link
Owner

I pushed a last commit that remove some unwrap by using pattern matching. The logic is the same but if you prefer the old code I can revert it.

I do like them but diffing is now harder, can we do that in a separate PR?

@LouisDISPA
Copy link
Contributor Author

I do like them but diffing is now harder, can we do that in a separate PR?

Perfect, I removed the last commit and clean the commit history.
I'll open another PR with the unwrap clean.

Thanks for the quick response and to still manage this repo!

@LouisDISPA LouisDISPA mentioned this pull request Feb 29, 2024
Copy link
Owner

@saschanaz saschanaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! A few more nits:

LouisDISPA and others added 2 commits February 29, 2024 23:58
Co-authored-by: Kagami Sascha Rosylight <saschanaz@outlook.com>
@LouisDISPA
Copy link
Contributor Author

LouisDISPA commented Feb 29, 2024

Looking good! A few more nits:

Done!

@saschanaz saschanaz merged commit 451fa74 into saschanaz:main Mar 1, 2024
2 checks passed
@saschanaz
Copy link
Owner

Thank you so much! So helpful 👍🙏

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.

None yet

3 participants