Fix Arm Neon optimization detection when building for iOS

[TheMostDiligent]

When cross-compiling for iOS, CMake does not define the CMAKE_OSX_ARCHITECTURES variable. As a result, this branch of CMake script does not get executed, so PNG_ARM_NEON_OPT is not defined as well as source files for Neon are not added to the project.

The PNG_ARM_NEON_OPT then gets defined here which results in linker errors as source files are not in the project.

[TheMostDiligent]

Is it possible that you merge this minor change?
It fixes link errors in XCode

[jbowler]

You have an incorrectly set up cmake toolchain file; the toolchain file is what is setting the Apple specific stuff.

[TheMostDiligent]

You have an incorrectly set up cmake toolchain file; the toolchain file is what is setting the Apple specific stuff.

We are not using any toolchain file. We follow cross-compiling for iOS provided by CMake: -DCMAKE_SYSTEM_NAME=iOS.
We do not define -DCMAKE_OSX_ARCHITECTURES, but this is not an error and the build should not fail.

Is there any concern with the proposed change? It makes the CMake build script more reliable in any case.

[jbowler]

The change isn't necessary; it should be fixed by this PR #575

csparker247 may care to comment but please test #575 anyway; I don't know if Seth has an iOS test system and I certainly don't. My understanding is that XCode and Android Studio both supply appropriate toolchains but the old cmake code required rewrites and edits for every different environment. #575 should fix that.

[TheMostDiligent]

@jbowler thanks! I will test #575

[jbowler]

@ctruta: duplicate of #372, fixed by #575

[jbowler]

@ctruta fixed in libpng-1.8 (#575 has been merged), no comments from @TheMostDiligent May need #614 but I don't think it does and that is already merged.

I believe that the TARGET stuff is no longer used by the cmake build and that all the source files (i.e. for all architectures) are included in all builds but the original complaint that the optimizations weren't being detected (because CMakeLists.txt was disabling them) has now been fixed in all my builds. I.e. when including #614 (#614 may have fixed an issue with the NEON palette optimizations.)

I suggest closing this as "fixed" unless there is more input.

[TheMostDiligent]

@jbowler Sorry for delay.
I tried to build libpng at 76e5ec2 and I get the following link errors:

Undefined symbols for architecture arm64:
  "_png_do_expand_palette_rgb8_neon", referenced from:
      _png_do_expand_palette in libpng16d.a[9](pngrtran.o)
  "_png_do_expand_palette_rgba8_neon", referenced from:
      _png_do_expand_palette in libpng16d.a[9](pngrtran.o)
  "_png_init_filter_functions_neon", referenced from:
      _png_init_filter_functions in libpng16d.a[10](pngrutil.o)
  "_png_riffle_palette_neon", referenced from:
      _png_do_read_transformations in libpng16d.a[9](pngrtran.o)
ld: symbol(s) not found for architecture arm64

[jbowler]

@jbowler Sorry for delay. I tried to build libpng at 76e5ec2 and I get the following link errors:

The changes are only in libpng18. Try HEAD (of libpng18) which is currently d9f13d8 The actual changes end with this commit:

82f43f7

[TheMostDiligent]

How does versioning work? Looks like master=libpng16, what about 17 and 18? Why are they separate?

[jbowler]

How does versioning work? Looks like master=libpng16, what about 17 and 18? Why are they separate?

1.x is not ABI compatible with 1.(!x). These are effectively major releases because they break ABI compatibility. The version numbers weren't used consistently - 1.2 -> 1.4 was a massive API change, 1,4 should really have been 2.0 (IMO). 1.x.patch is a patch release; it must be ABI compatible (there is dispute over the meaning of the word compatible) and API compatible (but some 1.6 jumps were not...)

1.5 and 1.7 were mainly done by me as "development" releases but that was sort-of-unofficial. 1.7 was never released (it's still in "alpha"). It was a major ABI change but retained almost complete API compatibility.

Each 1.x release maintains build compatibility up to a point. This mainly depends on the maintainer but the idea at one time was that people who build operating system distributions shouldn't have to mess with the build options in the middle of a release.

Each 1.x release maintains source file compatibility up to a point; source files don't get moved around. This was probably just a feature of the source control system used over a number of releases; it was basically done by #if's on the version number so things had to stay in the same place in the same files. This may be moot now since git mostly handles renames ok.

1.8 is currently "alpha"; this is why build changes, ABI changes and at least some API changes are going in there. The door on those closes when 1.8.0 is released.

[TheMostDiligent]

Thanks for the explanation!
I think it is worth adding this to the readme because I'm certainly not the one who has this question.

Branch libpng18 does work fine, and there were no API issues. So should we switch to 18 or stick to 16 until 18 is released?

[jbowler]

So should we switch to 18 or stick to 16 until 18 is released?

Using unreleased software is something only developers involved, directly or often indirectly, should do. There's a reason it is unreleased. I don't use unreleased software other than things I am actively working on. I do use the latest-and-greatest; the latest released versions of everything that my development operating system distribution (gentoo "~" - the development version) provides.

I think that is probably a "no" :-)

[TheMostDiligent]

@jbowler Thanks!

I suggest closing this as "fixed" unless there is more input.

I confirm it is fixed in libpng18, but since it is a dev branch, I think it makes sense to close this issue once the fix makes it into master.