Skip to content

Integrate libvips#1

Open
Rodrigodd wants to merge 30 commits intomainfrom
integrate-libvips
Open

Integrate libvips#1
Rodrigodd wants to merge 30 commits intomainfrom
integrate-libvips

Conversation

@Rodrigodd
Copy link
Copy Markdown
Owner

@Rodrigodd Rodrigodd commented Oct 26, 2024 •
edited
Loading

This PR aims to integrate libvips into this project to get a better algorithm for downsampling images (mangas frequently use half-tone shading, which is very susceptible to aliasing when downsampling). As a bonus, it will allow moving all image format implementations upstream, decreasing the maintenance burden.

Roadmap:

  • Refactor CMake to use ExternalProject (as Komelia currently does).
  • Write automated tests (kinda hard to know if all libraries are include in the final library without testing it directly).
    • Run the tests on CI? I would need to setup a emulator there.
  • Link all libraries statically, if possible
  • Integrate libvips (by copying Komelia, again). This will be done by adding a decoder_vips that accepts all formats it can, and implementing resize using a Lanczos filter or similar. This resize alone will not be enough to remove all aliasing; this will need to be addressed in real-time in subsampling-scale-image-view to fully fix it.
    • Compile and link libvips
    • Add decoder_vips.h/cpp
    • Implement cropBorders
    • Implement resize with filtering.
    • Benchmark the implementation.
    • Handle ICC profiles properly (currently just converting to sRGB in RGBA8888 format).
  • Remove all no-longer-needed decoders, as libvips replaces them. Or let them remain, but disabled by default? Each decoder is currently guarded by an option, though I don't know what this is useful for.
    • Restructure the library to be simpler and take more advantage of libvips.

@Rodrigodd Rodrigodd force-pushed the integrate-libvips branch 4 times, most recently from 63ff960 to c281f7f Compare October 29, 2024 17:35
@Rodrigodd Rodrigodd force-pushed the integrate-libvips branch 4 times, most recently from 831f6db to c9a3e63 Compare November 12, 2024 23:12
@AntsyLich
Copy link
Copy Markdown

AntsyLich commented Nov 14, 2024

Leaving some comments:

  1. We probably don't need to do a full copy of Komelia for the decoder methods and such but this can be discussed later.
  2. For the last task I think we should drop the existing decoders in favor of vips

This was referenced Nov 26, 2024
Closed
Open
@AntsyLich
Copy link
Copy Markdown

AntsyLich commented Nov 30, 2024

Can you rename the packages from tachiyomi.decoder to dev.mihon.image.decoder

@Rodrigodd
Copy link
Copy Markdown
Owner Author

Rodrigodd commented Nov 30, 2024

@AntsyLich yes, I can. I will just do a final refactor of the code and submit a PR to your fork, with that change.

@Rodrigodd
Copy link
Copy Markdown
Owner Author

Rodrigodd commented Nov 30, 2024

@AntsyLich should it really be dev.mihon.image.decoder? I think it would make more sense to be app.mihon.image-decoder, because you guys own mihon.app, and the project is already called image-decoder. Asking just to be sure, I am fine either way.

@AntsyLich
Copy link
Copy Markdown

AntsyLich commented Nov 30, 2024

we also own mihon.dev and packages can't have - in them. Alternatively we can use _ but image_decoder doesn't sound pleasing or imagedecoder

@Rodrigodd
Copy link
Copy Markdown
Owner Author

Rodrigodd commented Nov 30, 2024

Ok, I will use dev.mihon.image.decoder then.

@Rodrigodd Rodrigodd mentioned this pull request Dec 1, 2024
Open
3 tasks
@Rodrigodd
Update to Gradle 8.9 and NDK 26.3
53221ea
@Rodrigodd
Refactor CMake project to use ExternalProject
d591161
@Rodrigodd
ci: Inspect and archive library file
51810e3 
Also only run assembleDebug for now, full assemble is too slow.
@Rodrigodd
Install more recent version of meson
0650d26
@Rodrigodd
Refactor CMake project to use ExternalProject
6c0fa4a 
Also rely on the existing build tool of each external project, instead
of patching CMake into them.

Most `ExternalProject_Add` wrappers were copied from Komelia, at
[https://github.com/Snd-R/Komelia/tree/8b938397e5c8743d4e995d2cc16203367de6f67d/image-decoder/native/cmake],
but modified to make sure all libraries are staticaly linked.
@Rodrigodd
Add documentation to decoder_base
6b03a3d 
Took me a while to figure out how the API work
@Rodrigodd
Add decoder libvips
07ec50c
@Rodrigodd
Remove all decoders minus libvips
e4bccd3
@Rodrigodd
Resize full image and then decode region
98871cd 
Was trying to optimize by cropping the image to the desired region, and
then resizing only that, but libvips seems to already be smart enough,
so that is not necessary.
@Rodrigodd
Write to Bitmap pixels directer
ac21786 
The intermediate buffer is not necessary, the `decode` function will
already take care of outputing in the correct format.
@Rodrigodd
Add documentation and minor refactors
df5e52e
@Rodrigodd
Remove loops from tests
43f07fd 
Was looping to detect memory corruption. Not needed anymore.
@Rodrigodd
Rename package to dev.mihon.image.decoder
63165cc 
And groupId/artifactId as `dev.mihon:image-decoder`
Rodrigodd and others added 7 commits December 7, 2024 20:27
@Rodrigodd
Remove ImageInfo struct
c196d56 
Only `bounds` were being used, so I only keeped that.
@Rodrigodd
Rename java/ to kotlin/
b40a998
@Rodrigodd
Add .cache to gitignore
95e6a19 
This folder is genrated by clangd
@wwww-wwww @Rodrigodd
lmcs2 for rgb, cmyk, no profile
218f3f3
@Rodrigodd
Fix test path
94ad0de
@Rodrigodd
fix: Initialize VIPS_INIT only once
121b012 
Mihon was crashing somethimes crashing when decoding the first image. I
notice Mihon was decoding two images at the same time, which made me
think it could be a threading issue. Reading libvips documentation, it
states that it is safe, but VIPS_INIT must only be called in a
single-thread. I was calling VIPS_INIT for every decoder created, which
I believe was the issue.

This commit moves VIPS_INIT to JNI_OnLoad, which is called only once
when the library is loaded.
@Rodrigodd
ci: Run tests on emulator
863efc9
@Rodrigodd
ci: Remove build step before running emulator
f7dea5a 
The emulator is failing to create due to lack of space. Maybe the
previous build is using too much? I removed the build step, but it will
still be built after the emulator is created, let's see if it will still
break.
@Rodrigodd
ci: Free disk space
3b374c9
@Rodrigodd
ci: remove autoremove
be01ad1
@Rodrigodd
ci: Setup Android SDK
0e34112
@Rodrigodd
ci: Run :benchmark in dryRunMode
a71fc01 
This will just check if the benchmark can run without errors.
@Rodrigodd
Try to cache CMake output
e78dbd8 
I am not very confident that building with the cache will work, even
more if we modify the CMakeLists.txt file between builds. But waiting
>20min per build is very painful (and there are both Debug and Release
builds).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Rodrigodd @AntsyLich @wwww-wwww