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

ForeignPngFilter #106

Merged
merged 6 commits into from
Dec 29, 2020
Merged

ForeignPngFilter #106

merged 6 commits into from
Dec 29, 2020

Conversation

randyridge
Copy link
Contributor

add ForeignPngFilter

Copy link
Owner

@kleisauke kleisauke left a comment

Choose a reason for hiding this comment

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

Thanks! I've added a few inline suggestions, but otherwise this looks good to me! The reason that this "enum" was not semi-generated is because VipsForeignPngFilter is not an enum, but a bit field.

@jcupitt Do you think the other bindings can also benefit from this? I had a similar issue with wasm-vips, see for example:
https://github.com/kleisauke/pyvips/tree/flags-helper
https://github.com/kleisauke/wasm-vips/blob/master/build/gen_type_declarations.py#L228-L230
https://github.com/kleisauke/wasm-vips/blob/master/build/gen_type_declarations.py#L251-L253
https://github.com/kleisauke/wasm-vips/blob/master/build/gen_type_declarations.py#L283

src/NetVips/Enums.cs Outdated Show resolved Hide resolved
src/NetVips/Enums.cs Outdated Show resolved Hide resolved
src/NetVips/Enums.cs Outdated Show resolved Hide resolved
randyridge and others added 4 commits December 29, 2020 09:26
Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>
Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>
Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>
@jcupitt
Copy link
Contributor

jcupitt commented Dec 29, 2020

Yes, the flags/enum thing is used in some places, but not worked through all the bindings. It might be useful to fix this.

@kleisauke kleisauke merged commit 937f161 into kleisauke:master Dec 29, 2020
@kleisauke
Copy link
Owner

Thank you Randy, this will be in v1.2.5.

@jcupitt As far as I know VipsForeignPngFilter is the only flag/bitfield used in libvips. I noticed that it is also missing in pyvips.

@jcupitt
Copy link
Contributor

jcupitt commented Dec 29, 2020

There are a few unimportant ones, eg.:

john@yingna ~/GIT/libvips/libvips/include/vips (master) $ grep "/*< flag" *.h
foreign.h:typedef enum /*< flags >*/ {
foreign.h:typedef enum /*< flags >*/ {
object.h:typedef enum /*< flags >*/ {
operation.h:typedef enum /*< flags >*/ {

gtk-doc uses the magic /*< .. >*/ comment to spot them, and you can find them by introspection.

I wonder if VipsForeignPngFilter is a true bitfield. Does libpng really support avg | paeth? I suspect it behaves more like a simple enum, though I'm too lazy to check.

@kleisauke
Copy link
Owner

I'll also add the corresponding Enums.ForeignPngFilter? filter = null overload for Pngsave{,Buffer,Target,Stream} allowing you to do this (for example):

var im = Image.NewFromFile("lichtenstein.jpg", access: Enums.Access.Sequential);
im.Pngsave("lichtenstein.png", filter: /*(int)*/Enums.ForeignPngFilter.All);

(it currently requires casting an enum to int, which is a bit clumsy)

@kleisauke
Copy link
Owner

@jcupitt I'm not not entirely sure if you could do avg | paeth, but none | sub | up | avg | paeth is possible (which is just a synonym for all). Anyways, it's defined here:
https://github.com/glennrp/libpng/blob/dbe3e0c43e549a1602286144d94b0666549b18e6/png.h#L1459-L1471

@randyridge randyridge deleted the rr/png branch December 29, 2020 17:46
@kleisauke
Copy link
Owner

I can confirm that avg | paeth also works. It generates a PNG file with exclusively avg or paeth as row filter, see for example:

$ pngcheck -vv lichtenstein.png | grep filters -A 1 | tail -5
    row filters (0 none, 1 sub, 2 up, 3 avg, 4 paeth):
      3 4 4 4 4 4 4 (2415 out of 2421)
--
    row filters (0 none, 1 sub, 2 up, 3 avg, 4 paeth):
      4 3 4 4 4 4 (2421 out of 2421)

@jcupitt
Copy link
Contributor

jcupitt commented Dec 30, 2020

Huh how curious. Thanks for checking.

@kleisauke kleisauke added this to the 2.0.0 milestone Mar 29, 2021
@kleisauke
Copy link
Owner

NetVips v2.0.0 is now available with this improvement included.

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.

3 participants