MRZlib: route compress and decompress streams through zlib-ng (native mode)#5959
Merged
MRZlib: route compress and decompress streams through zlib-ng (native mode)#5959
Conversation
Scoped to the single public function the user requested:
Expected<void> zlibCompressStream( std::istream&, std::ostream&,
const ZlibCompressParams& )
plus its thin int-level overload, which forwards to it. Decompression
(zlibDecompressStream) and every other zlib consumer (libzip's own
deflate path inside compressZip/decompressZip, etc.) stay on stock
zlib -- this PR does NOT swap zlib for zlib-ng globally.
zlib-ng is used in its native "-ng" mode: the library exposes zng_
prefixed symbols and the zlib-ng.h header, the SONAME is libz-ng (not
libz), so it can coexist with stock zlib in the same process without
any ABI overlap or symbol collisions.
Per-platform source of the library:
- Windows vcpkg: zlib-ng port (added to requirements/windows.txt)
- Rocky Linux vcpkg: zlib-ng port (added to requirements/vcpkg-linux.txt)
- macOS Homebrew: zlib-ng formula (added to requirements/macos.txt;
not keg-only, native-mode upstream defaults)
- Ubuntu apt: no standalone package, built from thirdparty/zlib-ng
- Emscripten: no package, built from thirdparty/zlib-ng
thirdparty/CMakeLists.txt gates add_subdirectory(./zlib-ng) on
NOT WIN32 AND NOT APPLE AND NOT MESHLIB_USE_VCPKG, matching the split
above. ZLIB_COMPAT stays OFF so the submodule build also produces
native zng_ / libz-ng / zlib-ng.h, identical to what vcpkg and brew
install.
MRMesh/CMakeLists.txt uses find_library/find_path rather than
find_package(zlib-ng CONFIG) for the same relocatability reason as
libdeflate (upstream config can bake an absolute include path that
breaks under the Ubuntu/Emscripten Docker COPY-shuffle).
Source: split the compress body into a new MRZlibNg.cpp so that
translation unit includes only <zlib-ng.h> and the untouched
zlibDecompressStream in MRZlib.cpp keeps including only <zlib.h>.
Avoids any macro-redefinition ordering question between the two
headers without forcing code to choose one.
Drop the separate MRZlibNg.cpp; fold the zng_-prefixed zlibCompressStream overloads directly into MRZlib.cpp alongside zlibDecompressStream. Both <zlib.h> and <zlib-ng.h> include cleanly in the same TU: the Z_* return-code / flush-mode / strategy constants are defined in both headers with identical values (C preprocessor allows a redefinition with the same replacement list, no warning), and the function names (deflate/inflate vs zng_deflate/zng_inflate) and struct types (z_stream vs zng_stream) are disjoint. The shared zlibToString and windowBitsFor helpers are reused across compress (zng_*) and decompress (stock zlib) since MAX_WBITS equals Z_MAX_WINDOWBITS = 15 and the Z_* code values match. No functional change relative to the previous commit on this branch; just collapses two TUs into one.
This reverts commit e66838a. The merge looked clean under static inspection (Z_* macros identical between zlib.h and zlib-ng.h, deflate/inflate vs zng_deflate/zng_inflate disjoint, z_stream vs zng_stream disjoint) but failed to compile on macOS-arm64 Debug with: /opt/homebrew/include/zlib-ng.h:1079:20: error: typedef redefinition with different types ('uint32_t (*)(void *, const uint8_t **)' vs 'unsigned int (*)(void *, unsigned char **)') Both headers declare callback typedefs in_func and out_func (used by inflateBack) at global scope with the same name and different signatures: zlib.h: typedef unsigned (*in_func)(void*, unsigned char**); zlib-ng.h: typedef uint32_t (*in_func)(void*, const uint8_t**); C++ refuses the redefinition. The typedefs are unconditional in both headers -- there's no feature macro to hide them -- even though MeshLib's code uses neither inflateBack nor the two typedefs. Splitting the compress body into its own TU (MRZlibNg.cpp) so each TU sees at most one of the two headers is the clean fix. Run showing the failure: 24776570063.
zlib-ng's CMakeLists treats an undefined BUILD_SHARED_LIBS as "build
both shared and static targets". Emscripten then demotes the SHARED
one to STATIC, after which both rules emit the same libz-ng.a output
and ninja fails at configure time:
CMake Warning (dev) at zlib-ng/CMakeLists.txt:1165 (add_library):
ADD_LIBRARY called with SHARED option but the target platform does
not support dynamic linking. Building a STATIC library instead.
...
ninja: error: build.ninja:1263: multiple rules generate libz-ng.a
[-w dupbuild=err]
Force BUILD_SHARED_LIBS=OFF only for the zlib-ng add_subdirectory on
Emscripten. Ubuntu apt keeps the default (shared) so MRMesh.so still
picks up libz-ng.so. Local set() -- no CACHE -- keeps the override
scoped to this add_subdirectory and doesn't leak to the later
Emscripten-specific thirdparty libs below.
Seen on run 24776570063, job 72496287138.
The CMake build globs source files automatically, but the MSBuild project is hand-maintained -- the new TU that carries zlibCompressStream against zlib-ng needs to be listed explicitly in both MRMesh.vcxproj (ClCompile) and MRMesh.vcxproj.filters (Source Files\IO, next to MRZlib.cpp).
zlib-ng does not declare Z_MAX_WINDOWBITS -- I hallucinated that name
when writing MRZlibNg.cpp. zlib-ng's zconf-ng.h re-exports MAX_WBITS
(15) under exactly the same spelling stock zlib uses, so the native-
mode header already gives us the constant we need.
Fix the two constexpr initialisers that were using the wrong name,
and tighten the surrounding comment to reflect what the header
actually provides.
Seen on macOS arm64 Debug (run 24777754710, job 72500510814):
/source/MRMesh/MRZlibNg.cpp:25:34: error: use of undeclared
identifier 'Z_MAX_WINDOWBITS'
/source/MRMesh/MRZlibNg.cpp:26:35: error: use of undeclared
identifier 'Z_MAX_WINDOWBITS'
The test pinned stats.compressedSize against sizeof(cRawLevel{1,9})
and sizeof(cWrappedLevel{1,9}), i.e. reference blobs captured from
stock zlib. Each alternative deflate implementation we've tried
produces a valid, lossless output of a slightly different size:
stock zlib: reference (70 / 76 bytes on this corpus)
zlib-ng compat: stock - 1 byte
zlib-ng native: stock + 7 bytes (seen here)
libdeflate: stock ± small amount
All of these are correct. Chasing tolerance windows (+/-4 was enough
for zlib-ng-compat, +/-7 now for native, libdeflate may need more)
is fragile. Replace the exact-size EXPECT_EQ with four engine-
agnostic invariants that still catch any real regression in the
stats API:
- stats.crc32 matches an independent CRC-32 reference (was already)
- stats.uncompressedSize == sizeof( input ) (was already)
- stats.compressedSize == out.str().size() (API internal consistency)
- 0 < stats.compressedSize < sizeof( input ) (non-empty, compressed)
The round-trip check in ZlibCompressTestFixture + ZlibDecompressTest
Fixture still guarantees byte-exact recovery, so any encoder bug that
produces something parseable-but-wrong would still fail there.
Seen on run 24778762984 (feat/zlib-compress-stream-zlib-ng):
macOS arm64 Release, Debug
source/MRTest/MRZlibTests.cpp:160-161:
stats.compressedSize: 77 c.expectedCompSize: 70
stats.compressedSize: 83 c.expectedCompSize: 76
…lib-compress-stream-zlib-ng
…subdirectory The static-only setting I added in d1b60f3 to work around zlib-ng's multiple-rules-generate-libz-ng.a collision on Emscripten leaked to every subsequent add_subdirectory in the same scope. Specifically jsoncpp further down in thirdparty/CMakeLists.txt (line 133) is configured as: set(JSONCPP_WITH_TESTS OFF) set(JSONCPP_WITH_POST_BUILD_UNITTEST OFF) set(BUILD_STATIC_LIBS OFF) add_subdirectory(./jsoncpp) That relies on BUILD_SHARED_LIBS being in its previous (unset or ON) state to produce the shared jsoncpp library. After my leak flipped BUILD_SHARED_LIBS to OFF, jsoncpp had both BUILD_SHARED_LIBS=OFF and BUILD_STATIC_LIBS=OFF -- neither flavour was built. Headers were still installed but the library file was not, so the main Emscripten build failed at configure time with: CMake Error (FindPackageHandleStandardArgs.cmake): Could NOT find JsonCpp (missing: JsonCpp_LIBRARY) (found version "1.9.5") Wrap the set(BUILD_SHARED_LIBS OFF) in save/restore and scope the add_subdirectory(./zlib-ng) call inside that window so the override is visible only to zlib-ng. Same pattern already used for googletest at lines 46-50. Seen on Multithreaded-64Bit, Multithreaded, and Singlethreaded Emscripten legs (run 24781052774).
…estore-from-backup
The save/restore pattern I added in d68fd56b captured "" from an
undefined BUILD_SHARED_LIBS and then "restored" BUILD_SHARED_LIBS to ""
(defined-but-empty). That's a different state than the original
(undefined), and jsoncpp's option(BUILD_SHARED_LIBS ... ON) fallback
does NOT re-initialise an already-defined variable. So the empty value
persisted into jsoncpp's lib_json CMakeLists line 179:
set_target_properties(${OBJECT_LIB} PROPERTIES
OUTPUT_NAME jsoncpp
VERSION ${PROJECT_VERSION}
SOVERSION ${PROJECT_SOVERSION}
POSITION_INDEPENDENT_CODE ${BUILD_SHARED_LIBS} <- expands to ""
)
The empty expansion broke the key/value pairing and produced
"set_target_properties called with incorrect number of arguments",
aborting thirdparty CMake configure on the Emscripten image build.
On every Emscripten MeshLib build before this PR, BUILD_SHARED_LIBS
was undefined when thirdparty/CMakeLists.txt reached the zlib-ng
block (confirmed: zlib-ng is added before libzip/jsoncpp, and nothing
upstream sets the variable). So unset() after the add_subdirectory
is both correct and simpler than the if(DEFINED)/backup/restore
dance, and it matches the state jsoncpp's option() wants to see.
Seen on run 24800811051, job 72582653120.
This was referenced Apr 22, 2026
STATUS_DLL_NOT_FOUND (exit code -1073741515 / 0xC0000135) at
MeshViewer.exe launch kills the Run Start-and-Exit Tests step with
zero useful output — the Windows loader aborts before main() and
CI sees only a bare ##[error]Process completed with exit code ...
Observed on the zlib-ng branch run 24802707812, job 72596857457
(msvc-2019 Debug CMake with the x64-windows-meshlib-iterator-debug
triplet): vcpkg installed zlib-ng successfully but libz-ng.dll
didn't end up next to MeshViewer.exe. Three other Windows legs in
the same run with the default triplet succeeded, so the gap is
specifically in the iterator-debug triplet's DLL-copy chain.
Add two unconditional diagnostic steps so the next run of any
Windows job self-documents the state the loader will see:
1. After vcpkg-integrate-install, "Diagnostic — vcpkg installed
tree": lists the contents of
C:\vcpkg\installed\<TRIPLET>\{bin,debug\bin,lib,debug\lib}
so we see which DLLs vcpkg actually installed and under what
names (e.g. libz-ng.dll vs libz-ngd.dll). Confirms or rules out
"vcpkg didn't install the package" as the cause.
2. Right before "Run Start-and-Exit Tests", "Diagnostic — output
bin DLL inventory + MRMesh imports":
- Lists all .dll/.exe under source\x64\<CONFIG>\ so we see
which DLLs were copied next to MeshViewer.exe
- Runs dumpbin /dependents on MeshViewer.exe, MRMesh.dll,
MRTest.exe so we see which DLL names the loader is actually
looking for at process start
- Dumps the PATH so we can tell whether the vcpkg install
dir would be a fallback lookup location
Together, these three data points are enough to turn any
DLL-not-found failure into a one-line diagnosis.
Both steps have `if: always()` and `continue-on-error: true`, so
they run even when the main Build step failed (often when you need
the diagnostic most) and never mask a real failure themselves.
Net cost: ~30 lines of pwsh output per Windows job when everything
works; ~100 lines when a DLL is missing. Trivial relative to the
multi-gigabyte Windows CI logs already emitted.
Fedr
added
disable-build-ubuntu-x64
disable-build-ubuntu-arm64
disable-build-linux-vcpkg
disable-build-macos
disable-build-emscripten
skip-image-rebuild
…brary
The diagnostic output from run 24834309936 shows two linked problems on
the msvc-2019 Debug CMake x64-windows-meshlib-iterator-debug leg:
1. Debug MRMesh.dll imports from zlib-ng2.dll (the RELEASE variant)
instead of zlib-ngd2.dll (the matching Debug variant) that vcpkg
also installed. Every other MRMesh.dll dependency resolved to its
d-suffixed debug DLL (zlibd1.dll, spdlogd.dll, tbb12_debug.dll,
tiffd.dll, fmtd.dll) -- those use find_package(... CONFIG) which
exports IMPORTED_CONFIGURATIONS=DEBUG;RELEASE with distinct per-
config IMPORTED_LOCATION properties.
2. Neither zlib-ng2.dll nor zlib-ngd2.dll was copied next to
MeshViewer.exe by vcpkg's applocal-copy step. Applocal keys on
imported-target metadata; a raw find_library() result doesn't
carry the Debug/Release-aware config map that applocal expects,
so the copy falls through.
Together: Debug MeshViewer.exe launches, loader walks MRMesh.dll's
import table, looks for zlib-ng2.dll in the output dir and on PATH,
finds neither (vcpkg's install bin isn't on PATH), aborts with
STATUS_DLL_NOT_FOUND (0xC0000135 / exit -1073741515) before main().
Fix both by using find_package(zlib-ng CONFIG REQUIRED) on Windows and
linking the imported target zlib-ng::zlib (vcpkg's zlib-ng port ships
the proper config package). Keep the existing find_library / find_path
path for everything else -- Ubuntu apt and Emscripten build zlib-ng
from thirdparty/zlib-ng and its generated config has the same non-
relocatable-include-path bug we hit with libdeflate under the Docker
multi-stage COPY-shuffle, and those platforms are single-config so the
Debug/Release picking issue doesn't apply.
Only the Windows find is gated; macOS and Linux keep their current
working behaviour.
…on script Upstream zlib-ng 2.3.3's CMakeLists.txt defines HAVE_SYMVER and passes -Wl,--version-script=zlib-ng.map on non-Apple, non-AIX Unix. Both together tag every exported symbol in libz-ng.so.2 with ZLIB_NG_2.0.0 / ZLIB_NG_2.1.0 version nodes, which land in DT_VERNEED of anything linking against it. auditwheel's manylinux policy database has no entry for the pair (libz-ng.so.2, ZLIB_NG_*), so the Linux-vcpkg NuGet wheel-repair step fails with "too-recent versioned symbols" even though no actual symbol is too recent — auditwheel's generic phrasing for any (lib, version- tag) pair it can't place in a known policy. This blocks #5959 on the Linux-vcpkg Clang 20 Release leg. Contrast with stock libz.so.1: auditwheel has that library explicitly on the manylinux allowlist with its ZLIB_1.2.0 tags pre-registered, so it passes trivially. zlib-ng is not on any allowlist. We don't exercise zlib-ng's ABI-versioning machinery — MeshLib's consumers rebuild against whatever libz-ng we ship — so neutralizing both knobs is safe. The overlay port replaces the guarding `if(NOT APPLE AND NOT CMAKE_SYSTEM_NAME STREQUAL AIX)` with `if(FALSE)`, skipping the HAVE_SYMVER define and the --version-script linker flag in one edit. Upstream's zlib-ng.map file is left on disk but never wired into the build. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ild for overlay zlib-ng port)
This was referenced Apr 24, 2026
Fedr
added a commit
that referenced
this pull request
Apr 24, 2026
The test currently compares `stats.compressedSize` and `out.str().size()` against `sizeof( cRawLevel* )` / `sizeof( cWrappedLevel* )`. Those reference blobs were captured from stock zlib, and the exact compressed byte count drifts from one deflate implementation to another (stock zlib vs zlib-ng compat vs zlib-ng native vs libdeflate -- all lossless, but each picks its own block-split / Huffman / match-length heuristics). Once we swap any part of the compress path to a non-zlib engine, this assertion fails for reasons that have nothing to do with correctness. Replace with engine-agnostic assertions: - CRC matches the reference CRC of the input (decoder-defined, engine- independent). - Uncompressed size matches the input size (tautology that still guards against stats wiring regressions). - `stats.compressedSize == out.str().size()` -- API consistency between the stats block and the actual stream, regardless of engine. - `0 < stats.compressedSize < sizeof( cInput )` -- sanity: we produced a non-empty payload smaller than the input. Split out of #5959 so the test relaxation can land independently of the zlib-ng routing. Pure test change -- no source / build / CI impact. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The overlay port added in this PR installs lib/cmake/zlib-ng/zlib-ng-config.cmake, which removes the original reason for the manual per-config IMPLIB+LOCATION block on Windows. Vcpkg's debug+release dual-build populates IMPORTED_*_DEBUG/RELEASE on the imported target, so applocal-copy picks the correct DLL per configuration without further help. Upstream zlib-ng v2.3.3 doesn't add an in-tree alias for zlib-ng::zlib (only the install-time EXPORT carries the namespace), so add the alias right after add_subdirectory in thirdparty/CMakeLists.txt to unify the two consumption paths under one target name. Net: 91 lines removed, 14 lines added in source/MRMesh and thirdparty.
oitel
reviewed
Apr 27, 2026
|
|
||
| # zlib-ng from our submodule on Ubuntu apt + Emscripten (no package available); | ||
| # Windows/macOS/vcpkg pick it up via requirements/*.txt and find_package. | ||
| IF(NOT WIN32 AND NOT APPLE AND NOT MESHLIB_USE_VCPKG) |
Contributor
There was a problem hiding this comment.
The script is never called for Windows or vcpkg builds.
Comment on lines
+88
to
+89
| # zlib-ng from our submodule on Ubuntu apt + Emscripten (no package available); | ||
| # Windows/macOS/vcpkg pick it up via requirements/*.txt and find_package. |
Comment on lines
+9
to
+30
| # MeshLib: strip zlib-ng's GNU symbol version script and the matching .symver | ||
| # pragmas in its C sources. | ||
| # | ||
| # Upstream's CMakeLists.txt defines -DHAVE_SYMVER (which turns on __asm__( | ||
| # ".symver foo, foo@@ZLIB_NG_2.0.0") pragmas in zbuild.h) and passes | ||
| # -Wl,--version-script=zlib-ng.map to the linker whenever the target is | ||
| # non-Apple, non-AIX UNIX. Both together tag every exported symbol in | ||
| # libz-ng.so with ZLIB_NG_2.0.0 / ZLIB_NG_2.1.0 version nodes, which end up | ||
| # in DT_VERNEED of anything linking against libz-ng. | ||
| # | ||
| # auditwheel's manylinux policy database has no entry for (libz-ng.so.2, | ||
| # ZLIB_NG_*), so the MeshLib NuGet wheel-repair step fails with "too-recent | ||
| # versioned symbols" even though no actual symbol is too recent. We don't | ||
| # exercise zlib-ng's ABI-versioning machinery (our consumers rebuild against | ||
| # whatever libz-ng we ship), so we neutralize both knobs by flipping the | ||
| # guarding condition to FALSE. Upstream's zlib-ng.map file is left on disk | ||
| # but never wired into the build. | ||
| vcpkg_replace_string( | ||
| "${SOURCE_PATH}/CMakeLists.txt" | ||
| "if(NOT APPLE AND NOT CMAKE_SYSTEM_NAME STREQUAL AIX)" | ||
| "if(FALSE) # MeshLib: symbol versioning disabled, see thirdparty/vcpkg/ports/zlib-ng/portfile.cmake" | ||
| ) |
Contributor
There was a problem hiding this comment.
Prefer using --exclude libz-ng.so.2 with auditwheel instead.
Comment on lines
+36
to
+45
| case Z_OK: return "ok"; | ||
| case Z_STREAM_END: return "stream end"; | ||
| case Z_NEED_DICT: return "need dict"; | ||
| case Z_ERRNO: return "errno"; | ||
| case Z_STREAM_ERROR: return "stream error"; | ||
| case Z_DATA_ERROR: return "data error"; | ||
| case Z_MEM_ERROR: return "mem error"; | ||
| case Z_BUF_ERROR: return "buf error"; | ||
| case Z_VERSION_ERROR: return "version error"; | ||
| default: return "unknown code"; |
Contributor
There was a problem hiding this comment.
Why was this block reformatted?
Comment on lines
+5
to
+9
| // zlib-ng in native mode: the zng_ prefix on every symbol keeps it ABI- | ||
| // distinct from stock zlib (which libzip still links as before), and | ||
| // <zlib-ng.h> re-exports the MAX_WBITS / Z_* constants we need under the | ||
| // same spelling as <zlib.h>, so this TU doesn't include stock zlib's | ||
| // header at all. |
| stream.next_in = reinterpret_cast<uint8_t*>( inChunk.data() ); | ||
| stream.avail_in = (unsigned)in.gcount(); | ||
| assert( stream.avail_in <= (unsigned)inChunk.size() ); | ||
| stream.avail_in = static_cast<unsigned>( in.gcount() ); |
Contributor
There was a problem hiding this comment.
Why were all C-style casts replaced with static_casts? These changes overload the pull request and the resulting commit.
Comment on lines
+106
to
+107
| # zlib-ng exports zlib-ng::zlib only at install time; alias for in-tree use. | ||
| add_library(zlib-ng::zlib ALIAS zlib) |
Contributor
There was a problem hiding this comment.
There's no in-tree use of zlib.
Comment on lines
+68
to
+71
| # zlib-ng (native mode, zng_ prefix) powers MRZlib's compress/decompress streams. | ||
| # Sourced via find_package on vcpkg/Homebrew (where the port installs a CMake | ||
| # config), or via add_subdirectory + ALIAS in thirdparty/CMakeLists.txt on | ||
| # Ubuntu apt and Emscripten. |
| # Sourced via find_package on vcpkg/Homebrew (where the port installs a CMake | ||
| # config), or via add_subdirectory + ALIAS in thirdparty/CMakeLists.txt on | ||
| # Ubuntu apt and Emscripten. | ||
| IF(NOT TARGET zlib-ng::zlib) |
Comment on lines
+73
to
+75
| find_package(zlib-ng CONFIG REQUIRED) | ||
| ENDIF() | ||
| target_link_libraries(${PROJECT_NAME} PRIVATE zlib-ng::zlib) |
Contributor
There was a problem hiding this comment.
Move them to other unconditional dependencies.
…onfig
The Emscripten consumer-side example build was failing with:
CMake Error at CMakeLists.txt:41 (add_executable):
Target "Collision" links to target "zlib-ng::zlib" but the target was
not found. Perhaps a find_package() call is missing for an IMPORTED
target, or an ALIAS target is missing?
`source/MRMesh/CMakeLists.txt` switched MRMesh's link line to
`zlib-ng::zlib`, and on the static-archive Emscripten path that name
gets re-exposed to consumers (PRIVATE deps become INTERFACE for static
libs since they need final-link resolution). Consumers that do
`find_package(MeshLib)` then need `zlib-ng::zlib` to resolve.
Add `cmake/Modules/Findzlib-ng.cmake`, modeled on Findhidapi.cmake /
FindJsonCpp.cmake: pkg-config-aware probe, find_path/find_library,
version extracted from `ZLIBNG_VERSION "X.Y.Z"` in zlib-ng.h, UNKNOWN
IMPORTED target named `zlib-ng::zlib` matching the upstream-installed
target.
Add `find_dependency(zlib-ng)` to MRMeshConfig.cmake.in so the consumer
side sees the module and creates the imported target before MRMesh's
exported targets reference it.
The Find module is consumer-side only -- MRMesh's own build still uses
`find_package(zlib-ng CONFIG REQUIRED)` so Windows continues to pick
up vcpkg's per-config IMPORTED_IMPLIB_{DEBUG,RELEASE}+IMPORTED_LOCATION
shape that applocal-copy needs. The Find module's UNKNOWN IMPORTED
target is single-config and would lose Debug-vs-Release DLL selectivity
on the Windows iterator-debug triplet.
oitel review comments:
source/MRMesh/MRZlib.cpp (5 comments):
- Drop the block comment before `#include <zlib-ng.h>`.
- Restore original windowBits comment (no zlib-ng addition).
- Restore original memLevel comment + static_assert.
- Restore zngToString to multi-line case style.
- Restore the designated-initializer form for zng_stream
(`{ .zalloc = Z_NULL, ... }` instead of `{}`) -- explicit init,
matches the original z_stream form.
- Revert all C-style cast -> static_cast changes; the casts are
off-topic for this PR and bloat the diff.
source/MRMesh/CMakeLists.txt (3 comments):
- Drop the comment block before find_package(zlib-ng).
- Drop the `IF(NOT TARGET zlib-ng::zlib)` defensive check; by the
time MRMesh configures, thirdparty has been installed and zlib-ng's
CMake config is on disk, so find_package always succeeds.
- Fold `find_package(zlib-ng REQUIRED)` and `zlib-ng::zlib` into the
unconditional dependency block, matching the JsonCpp / Eigen / TBB
pattern.
thirdparty/CMakeLists.txt (3 comments):
- Drop the leading comment (the IF condition is self-documenting and
the comment misleadingly mentioned Windows/vcpkg paths that don't
reach this file).
- Drop `add_library(zlib-ng::zlib ALIAS zlib)`. The bare `zlib` target
is never used in-tree; the alias was a band-aid for a configure
scenario that doesn't actually occur in our build flow.
thirdparty/vcpkg/ports/zlib-ng/portfile.cmake (1 comment):
- Drop the vcpkg_replace_string that stripped zlib-ng's GNU symbol
versioning. Address the auditwheel "too-recent versioned symbols"
failure at the wheel-repair step instead, via `--exclude libz-ng.so.2`
in scripts/wheel/build_wheel.py and scripts/nuget_patch/fake_whl_helper.py.
Contributor
Author
|
Thanks for the review. All 13 comments addressed in 2c23d5f. Summary by file:
The Findzlib-ng.cmake module (added in 4d254c8 for the consumer-side propagation) stays in place — it's what |
… EMSCRIPTEN
Two fixes for the failures observed on run 24994434955:
(1) Windows build STATUS_DLL_NOT_FOUND on Debug legs:
source/MRMesh/CMakeLists.txt: `find_package(zlib-ng REQUIRED)` ->
`find_package(zlib-ng CONFIG REQUIRED)`. With our Findzlib-ng.cmake on
CMAKE_MODULE_PATH, MODULE mode wins, and its UNKNOWN IMPORTED target
with a single IMPORTED_LOCATION pointing to the .lib doesn't trigger
applocal-deploy on Windows multi-config -- vcpkg ships separate
zlib-ng2.dll and zlib-ngd2.dll for Release/Debug, so applocal needs
per-config IMPORTED_LOCATION (.dll) to copy the right one next to
the .exe. Forcing CONFIG mode uses vcpkg's installed zlib-ng-config
which has the proper SHARED IMPORTED + IMPORTED_IMPLIB_{DEBUG,RELEASE}
+ IMPORTED_LOCATION_{DEBUG,RELEASE} shape. Other platforms also have
zlib-ng's installed config (Homebrew on macOS, our submodule's
install(EXPORT) on apt/Emscripten after thirdparty install) so CONFIG
mode works uniformly.
(2) Linux/macOS consumer-side examples fail with "Could NOT find
zlib-ng" when find_package(MeshLib) runs find_dependency(zlib-ng):
source/MRMesh/MRMeshConfig.cmake.in: move `find_dependency(zlib-ng)`
from the unconditional block into the existing EMSCRIPTEN-gated block,
alongside find_dependency(libzip) and the other static-only private
deps. zlib-ng is PRIVATE in MRMesh's target_link_libraries; for shared
MRMesh.so on Linux/macOS, PRIVATE deps don't propagate to interface,
so consumers should never reference zlib-ng::zlib. The exception is
static-archive builds (Emscripten) where the linker has to resolve
the private deps at the consumer's final link.
The Findzlib-ng.cmake module stays in place: it's still used on the
consumer side for Emscripten where vcpkg/Homebrew configs aren't
available, and the simple UNKNOWN IMPORTED shape is sufficient there
(Emscripten is single-config, so the multi-config DLL concern doesn't
apply).
The Linux dockerfiles already pass `--overlay-ports MeshLib/ports` to `vcpkg install`, which makes vcpkg pick up our overlay zlib-ng port (2.3.3) at thirdparty/vcpkg/ports/zlib-ng/. install.bat on Windows did NOT pass that flag -- it only passed `--overlay-triplets` -- so vcpkg fell back to the upstream zlib-ng port bundled with whatever vcpkg version is in use: - vcpkg 2024.10.21 (msvc-2019 legs): upstream zlib-ng@2.1.5, which doesn't install lib/cmake/zlib-ng/zlib-ng-config.cmake. - vcpkg 2026.03.18 (msvc-2022 legs): newer upstream zlib-ng port that DOES install the config -- works without our overlay. Source/MRMesh/CMakeLists.txt's `find_package(zlib-ng CONFIG REQUIRED)` therefore failed only on the msvc-2019 CMake leg with: Could not find a package configuration file provided by "zlib-ng" with any of the following names: zlib-ngConfig.cmake / zlib-ng-config.cmake Adding `--overlay-ports "%~dp0vcpkg\ports"` to both `vcpkg install` calls in install.bat makes our overlay's 2.3.3 port apply on Windows too. The overlay calls `vcpkg_cmake_config_fixup(CONFIG_PATH lib/cmake/zlib-ng)` which produces zlib-ng-config.cmake; find_package(CONFIG) finds it. Other listed overlay ports (clip, laz-perf, opencascade-minimal, openctm) were already being picked up via vcpkg's automatic overlay discovery? No -- looks like they were being built from the upstream ports too. So this fix may also start applying those overlays on Windows; if any of them weren't intended to override, drop them from thirdparty/vcpkg/ports.
Set VCPKG_LIBRARY_LINKAGE=static for the zlib-ng port specifically in both x64-linux-meshlib and arm64-linux-meshlib triplets. zlib-ng's symbols then compile directly into libMRMesh.so instead of going through a separate libz-ng.so.2 with DT_NEEDED + DT_VERNEED references. Side effect (intended): fixes the auditwheel "too-recent versioned symbols" failure on Linux-vcpkg manylinux_2_28 wheel-repair. The DT_VERNEED entries on libMRMesh.so pointing at libz-ng.so.2's ZLIB_NG_2.0.0 / 2.1.0 version nodes go away because there's no longer a runtime dependency on libz-ng.so.2 -- its symbols are now part of libMRMesh.so itself. Other libs in the wheel that linked zlib-ng (libMeshLibC2*.so etc.) still get the same static absorption since they too go through MRMesh's target_link_libraries(... PRIVATE zlib-ng::zlib). Windows triplets stay dynamic -- DLL distribution model is fine with the SHARED IMPORTED + per-config IMPORTED_LOCATION shape vcpkg's zlib-ng config provides, and Windows wheels don't go through auditwheel.
…z-ng.so.2
(1) thirdparty/CMakeLists.txt: `IF(NOT WIN32 AND NOT APPLE AND NOT
MESHLIB_USE_VCPKG)` -> `IF(NOT APPLE)`. The Windows + vcpkg-Linux
paths never reach this file (Windows uses vcpkg directly, Linux-vcpkg
uses Docker pre-built thirdparty image), so the NOT WIN32 / NOT
MESHLIB_USE_VCPKG checks were dead. Per the reviewer's earlier
comment "The script is never called for Windows or vcpkg builds."
(2) scripts/wheel/build_wheel.py + scripts/nuget_patch/fake_whl_helper.py:
drop the `--exclude libz-ng.so.2` flag from auditwheel calls. With
Linux-vcpkg now static-linking zlib-ng (the prior commit's triplet
change), libz-ng.so.2 doesn't exist as a separate file in the wheel
-- its symbols are absorbed into libMRMesh.so directly. There's
nothing for auditwheel to exclude, so the flag became a no-op.
Fedr
added a commit
that referenced
this pull request
Apr 28, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
oitel
approved these changes
Apr 28, 2026
|
|
||
| REM Install vcpkg core dependencies | ||
| vcpkg install vcpkg-cmake vcpkg-cmake-config --host-triplet %VCPKG_DEFAULT_TRIPLET% --overlay-triplets "%~dp0vcpkg\triplets" --debug --x-abi-tools-use-exact-versions || goto :error | ||
| vcpkg install vcpkg-cmake vcpkg-cmake-config --host-triplet %VCPKG_DEFAULT_TRIPLET% --overlay-triplets "%~dp0vcpkg\triplets" --overlay-ports "%~dp0vcpkg\ports" --debug --x-abi-tools-use-exact-versions || goto :error |
Contributor
There was a problem hiding this comment.
Not necessary to specify overlay ports here, only on the next line.
Contributor
Author
There was a problem hiding this comment.
Done in a313fba8 — dropped --overlay-ports from the bootstrap vcpkg install vcpkg-cmake vcpkg-cmake-config line; kept it on the package install line where overlay zlib-ng@2.3.3 actually applies.
Comment on lines
+5
to
+8
| # Static-link zlib-ng so its symbols compile into libMRMesh.so directly. | ||
| # Avoids DT_VERNEED entries on libMRMesh.so pointing at libz-ng.so.2's | ||
| # ZLIB_NG_2.0.0/2.1.0 version nodes, which auditwheel's manylinux_2_28 | ||
| # policy database doesn't recognise. |
Contributor
There was a problem hiding this comment.
Replace with something like:
# Have to build zlib-ng as a static library due to the auditwheel issue:
# https://github.com/pypa/auditwheel/issues/613
Comment on lines
+5
to
+8
| # Static-link zlib-ng so its symbols compile into libMRMesh.so directly. | ||
| # Avoids DT_VERNEED entries on libMRMesh.so pointing at libz-ng.so.2's | ||
| # ZLIB_NG_2.0.0/2.1.0 version nodes, which auditwheel's manylinux_2_28 | ||
| # policy database doesn't recognise. |
Contributor
There was a problem hiding this comment.
Replace with something like:
# Have to build zlib-ng as a static library due to the auditwheel issue:
# https://github.com/pypa/auditwheel/issues/613
Comment on lines
+18
to
+20
| # zlib-ng is PRIVATE on the build side -- only needed by consumers when | ||
| # MRMesh is a static archive (Emscripten), where private deps become | ||
| # interface deps because static archives don't pre-resolve symbols. |
(1) thirdparty/install.bat: drop --overlay-ports from the bootstrap
`vcpkg install vcpkg-cmake vcpkg-cmake-config` line; the package
install on the next line is the only one that needs to see overlay
ports.
(2) thirdparty/vcpkg/triplets/{x64,arm64}-linux-meshlib.cmake: replace
the verbose comment about DT_VERNEED / manylinux_2_28 policy with
a short pointer to the upstream auditwheel issue:
pypa/auditwheel#613
(3) source/MRMesh/MRMeshConfig.cmake.in: drop the comment explaining
why find_dependency(zlib-ng) is gated on EMSCRIPTEN -- the
surrounding "static builds require to find private dependencies"
comment already covers it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
oitel
approved these changes
Apr 28, 2026
Grantim
approved these changes
Apr 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Route MRZlib's public
zlibCompressStream+zlibDecompressStreamthrough zlib-ng in native mode (zng_prefix,libz-ng), without disturbing any other consumer of stock zlib (libzip, openvdb, etc.).Both directions move together. Per upstream benchmarks, zlib-ng delivers ~1.5–2× faster deflate and ~2–3× faster inflate vs stock zlib (the inflate gain is larger, dominated by a SIMD-vectorised chunk-copy loop). MeshLib's
compressZip/decompressZip, scene loaders, and the round-trip step inCompressManySmallFilesToZipall inherit the speedup without any change to call sites.Scope
Exactly three public functions flip backend:
Public API unchanged (same declarations in
MRZlib.h). Every stock-zlib consumer that runs behind libzip (for example, archive headers and central-directory processing insidecompressZip/decompressZip) keeps its stock-zlib link path unchanged — no graph-wide swap, no DLL-shadowing, no ABI coexistence concerns.Native mode, not compat
zlib-ng is used in its native
-ngvariant (symbolszng_deflate,zng_inflate, …; header<zlib-ng.h>; SONAMElibz-ng). That deliberately avoids overlapping with stock zlib's ABI: stock zlib and zlib-ng live in the same process with zero symbol or SONAME collision. MeshLib code uses one consciously, the other transitively, and the loader resolves both correctly.Per-platform source of the library
zlib-ngport @ v2.3.3 (see below)zlib-ngport @ v2.3.3 (static per triplet shim)zlib-ngformulathirdparty/zlib-ngsubmodule (no apt package)add_subdirectoryprovides the targetthirdparty/zlib-ngsubmodule (forced static)add_subdirectoryprovides the targetCMake wiring (
source/MRMesh/CMakeLists.txt)Net change: +2 lines. Single unconditional dependency:
CONFIGmode is critical on Windows: an earlierfind_library-based approach silently picked the releasezlib-ng2.dllfor both Debug and ReleaseMRMesh.dllbuilds (and skipped the applocal-copy step for the debug variant), producingSTATUS_DLL_NOT_FOUNDon the iterator-debug triplet at MeshViewer.exe launch. The CONFIG package emits a SHARED IMPORTED target with per-configurationIMPORTED_LOCATION_DEBUG/IMPORTED_LOCATION_RELEASE, so multi-config CMake picks the right.liband applocal-deploy ships the matching DLL.thirdparty/CMakeLists.txtadds anIF(NOT APPLE)block aroundadd_subdirectory(./zlib-ng)for the Ubuntu apt + Emscripten paths (Windows and vcpkg-Linux never reach this file; macOS uses brew). Emscripten forcesBUILD_SHARED_LIBS=OFFto avoid a duplicate-rule emission in the wasm toolchain, thenunsets the variable rather than restoring it (jsoncpp downstream breaks on an emptyBUILD_SHARED_LIBS).Linux-vcpkg auditwheel: static-link via triplet shim
libz-ng.so.2exports its symbols with GNU symbol-version tags (ZLIB_NG_2.0.0/ZLIB_NG_2.1.0). auditwheel's manylinux_2_28 policy database has no entry for these tags, so the NuGet wheel-repair step fails with "too-recent versioned symbols" — even though the symbols themselves predate manylinux_2_28's glibc baseline by years.Fix is per-port static linkage in both triplet files (
thirdparty/vcpkg/triplets/{x64,arm64}-linux-meshlib.cmake):Static linkage absorbs zlib-ng's symbols into
libMRMesh.sodirectly — noDT_VERNEEDentry onlibMRMesh.sopoints atlibz-ng.so.2, so auditwheel sees nothing to complain about. zlib-ng isPRIVATE-linked from MRMesh, so no consumer of MRMesh sees the absorption either.Scoped to the
zlib-ngport only — every other vcpkg port keeps the project-widedynamiclinkage. Net change: +6 lines per triplet.Vcpkg overlay port
thirdparty/vcpkg/ports/zlib-ng/overlays the upstream port to pin to v2.3.3 (newer than the 2024.10.21 baseline pinned for msvc-2019). Adapted from upstream's stockzlib-ngportfile — pulls from GitHub at the pinned SHA, configures withZLIB_COMPAT=OFF(native-ngvariant), runsvcpkg_cmake_install, fixes Windows-specific output naming so the.pcfile matches whatever.libgot produced, and runsvcpkg_cmake_config_fixupso the installed config lives atlib/cmake/zlib-ng. +84 lines (portfile.cmake + vcpkg.json).install.bat:--overlay-portsLinux dockerfiles already pass
--overlay-ports "%~dp0vcpkg\ports"tovcpkg install, but Windowsinstall.batwas missing it — so the overlay zlib-ng@2.3.3 port wasn't being picked up on the msvc-2019 leg, and CMakefind_package(zlib-ng CONFIG REQUIRED)failed because the baseline registry pin had no zlib-ng port at all. Added--overlay-portsto the package-install line (the bootstrapvcpkg install vcpkg-cmake vcpkg-cmake-configline above doesn't need it). +1 / −1.cmake/Modules/Findzlib-ng.cmake40-line Find module, used by consumers of installed MRMesh that go through
find_dependency(zlib-ng)inMRMeshConfig.cmakewithout an upstream zlib-ng config available (e.g., a system-installed zlib-ng resolved viapkg-config). The module:pkg-config zlib-ngfor hintsfind_path(... NAMES zlib-ng.h)andfind_library(... NAMES z-ng zlib-ng)zlib-ng.hzlib-ng::zlibUNKNOWN IMPORTED targetConsumer-side only — the in-tree build always goes through
find_package(zlib-ng CONFIG REQUIRED), which suppresses MODULE-mode dispatch.MRMeshConfig.cmake.infind_dependency(zlib-ng)added inside the existingif(EMSCRIPTEN) ... endif()block. zlib-ng isPRIVATEfrom MRMesh on shared-library platforms, so its symbols pre-resolve and consumers don't need to find it again. On Emscripten where MRMesh ships as a static archive, private deps become interface deps because static archives don't pre-resolve symbols.Source layout
One TU,
source/MRMesh/MRZlib.cpp. It includes only<zlib-ng.h>(no<zlib.h>), which sidesteps thein_func/out_functypedef collision between the two headers. Contains both the compress and decompress implementations using the correspondingzng_*primitives. +15 / −15 (line-for-linez_*→zng_*rename, no logic change).What's in the diff (15 files, +180 / −16)
source/MRMesh/MRZlib.cppzng_*primitives.source/MRMesh/CMakeLists.txtfind_package(zlib-ng CONFIG REQUIRED)+target_link_libraries(... PRIVATE zlib-ng::zlib).source/MRMesh/MRMeshConfig.cmake.infind_dependency(zlib-ng)inside the Emscripten block.cmake/Modules/Findzlib-ng.cmakethirdparty/CMakeLists.txtIF(NOT APPLE)add_subdirectory(./zlib-ng)for ubuntu apt + emscripten.thirdparty/zlib-ng(submodule).gitmodulesthirdparty/vcpkg/ports/zlib-ng/portfile.cmake+vcpkg.json-ngvariant).thirdparty/vcpkg/triplets/{x64,arm64}-linux-meshlib.cmakethirdparty/install.bat--overlay-portsto the package-install line.requirements/{windows,vcpkg-linux,macos}.txtzlib-ng.CI safety net
Round-trip correctness is verified by the existing
ZlibCompressTestFixture+ZlibDecompressTestFixtureparametrised suites and theCompressOneBigFileToZip/CompressManySmallFilesToZiplevel-by-level test on master. zlib-ng'szng_inflateconsumes any valid RFC 1950 / RFC 1951 stream, so pre-captured stock-zlib reference blobs continue to decompress correctly.Already-landed prerequisites (now on master, not in this diff)
MRMesh.ZlibCompressStatsengine-agnostic assertions. Was originally part of this branch; landed independently and the merge picked it up cleanly so this PR's diff against master no longer touchesMRZlibTests.cpp.CompressSphereToZip→CompressOneBigFileToZiprename + arch-invariant pseudo-random input. Independent of this PR but pairs nicely:big.zipis now byte-identical across platforms when run on stock zlib, so any drift after the zlib-ng swap is unambiguously the engine's doing.Not in this PR
compressZipvialibdeflate(the alternative "whole-graph, no streaming" path). Closed as MRZip: route compressZip through libdeflate when archive is unencrypted #5952.target_link_libraries(... ZLIB::ZLIB)line fromMRMesh/CMakeLists.txt. It's no longer reached byMRZlib.cppitself but libzip still transitively pulls it in; removing the line is orthogonal cleanup for a separate PR.🤖 Generated with Claude Code