From e640a9bfb8649df98903f0165991683a14f605a4 Mon Sep 17 00:00:00 2001 From: donotsleepy <12411806@mail.sustech.edu.cn> Date: Tue, 12 May 2026 17:40:57 +0800 Subject: [PATCH] Merge pull request #28995 from donotsleepy:fix/png-found-status-guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmake: fix PNG status display when BUILD_PNG=ON #28995 ### Description Fixes #28657. On macOS (and other Apple platforms), `BUILD_PNG=ON` is the default. The build system correctly clears `PNG_FOUND` in `OpenCVFindLibsGrfmt.cmake` and builds libpng from the bundled source. However, during the subsequent module processing phase, a downstream `find_package(PNG)` call — triggered transitively via `include()` in the **same scope** — overwrites not only `PNG_FOUND`, but also `PNG_INCLUDE_DIR`, `PNG_LIBRARIES`, and `PNG_VERSION_STRING`. #### Root Cause: FindPNG.cmake Has Asymmetric Guards Reading CMake 4.3's `FindPNG.cmake` (`/opt/homebrew/share/cmake/Modules/FindPNG.cmake`) reveals: | Variable | Guard? | Fate | |---|---|---| | `PNG_LIBRARY` | `if(NOT PNG_LIBRARY)` at line 138 — **guarded** | Preserved | | `PNG_PNG_INCLUDE_DIR` | `find_path(...)` at line 115 — **no guard** | **Overwritten** | | `PNG_INCLUDE_DIR` | Derived from `PNG_PNG_INCLUDE_DIR` | **Chain-overwritten** | | `PNG_LIBRARIES` | Derived from `PNG_LIBRARY` + `ZLIB_LIBRARY` | **Partly overwritten** | | `PNG_VERSION_STRING` | Parsed from `PNG_PNG_INCLUDE_DIR/png.h` | **Overwritten** | `PNG_LIBRARY` survives because FindPNG itself checks `if(NOT PNG_LIBRARY)`. But `find_path(PNG_PNG_INCLUDE_DIR)` runs unconditionally — re-searching and overwriting the bundled path with the system one. #### Fix: Three Coordinated Changes **Part A — Status guard** (`CMakeLists.txt`): When `BUILD_PNG=ON`, pass `FALSE` as the condition to always show `"build"` regardless of `PNG_FOUND`. **Part B — Variable lock** (`cmake/OpenCVFindLibsGrfmt.cmake`): After building from bundled source, lock `PNG_PNG_INCLUDE_DIR` as `CACHE INTERNAL`. CMake's `find_path()` checks the cache first — if the variable exists with a valid path, it skips re-searching. The system path is never found. **Part C — Cache cleanup** (`cmake/OpenCVFindLibsGrfmt.cmake`): Add `PNG_PNG_INCLUDE_DIR` to the `ocv_clear_internal_cache_vars()` call in the `else` branch. When a user switches `BUILD_PNG=OFF`, the cached variable is cleared, allowing `find_path` to search for the system libpng normally. Prevents stale cache leakage across configuration changes. #### Verification (macOS 26, CMake 4.3, Homebrew libpng 1.6.58) ``` BEFORE downstream find_package(PNG): PNG_LIBRARY = libpng PNG_INCLUDE_DIR = .../3rdparty/libpng PNG_VERSION_STRING = 1.6.37 AFTER downstream find_package(PNG) [with fix]: PNG_LIBRARY = libpng ← preserved (FindPNG guard) PNG_INCLUDE_DIR starts with .../3rdparty/libpng ← locked (Part B) PNG_VERSION_STRING = 1.6.53 ← from bundled png.h Status display: build (ver 1.6.53) ← correct (Part A) ``` The full reproduction and analysis are in the attached `bugReview/` directory. ### Checklist - [x] There is a reference to the original bug report and related work - [x] The test case is in `bugReview/` directory - [x] Tested on macOS 26 (arm64) with CMake 4.3 + Homebrew libpng 1.6.58 - [x] Cache hygiene verified for BUILD_PNG ON→OFF→ON transitions --- CMakeLists.txt | 6 +++++- cmake/OpenCVFindLibsGrfmt.cmake | 14 +++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1edd5f53cd..1d49243093 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1540,7 +1540,11 @@ if(WITH_SPNG) status(" Metadata Support:" "EXIF XMP ICC") # SPNG does not support cICP chunk. elseif(WITH_PNG OR HAVE_PNG) - status(" PNG:" PNG_FOUND THEN "${PNG_LIBRARY} (ver ${PNG_VERSION_STRING})" ELSE "build (ver ${PNG_VERSION_STRING})") + if(BUILD_PNG) + status(" PNG:" FALSE THEN "" ELSE "build (ver ${PNG_VERSION_STRING})") + else() + status(" PNG:" PNG_FOUND THEN "${PNG_LIBRARY} (ver ${PNG_VERSION_STRING})" ELSE "build (ver ${PNG_VERSION_STRING})") + endif() if(BUILD_PNG AND PNG_HARDWARE_OPTIMIZATIONS) status(" SIMD Support Request:" "YES") if(PNG_INTEL_SSE) diff --git a/cmake/OpenCVFindLibsGrfmt.cmake b/cmake/OpenCVFindLibsGrfmt.cmake index c3c358f6fc..15e8a133cd 100644 --- a/cmake/OpenCVFindLibsGrfmt.cmake +++ b/cmake/OpenCVFindLibsGrfmt.cmake @@ -333,7 +333,7 @@ if(NOT HAVE_SPNG AND WITH_PNG) if(BUILD_PNG) ocv_clear_vars(PNG_FOUND) else() - ocv_clear_internal_cache_vars(PNG_LIBRARY PNG_INCLUDE_DIR) + ocv_clear_internal_cache_vars(PNG_LIBRARY PNG_INCLUDE_DIR PNG_PNG_INCLUDE_DIR) find_package(PNG QUIET) endif() @@ -348,6 +348,18 @@ if(NOT HAVE_SPNG AND WITH_PNG) ocv_parse_header_version(PNG "${PNG_INCLUDE_DIR}/png.h" PNG_LIBPNG_VER_STRING) endif() + if(BUILD_PNG) + # Downstream find_package(PNG) calls from transitive dependencies + # (included via include() in the same scope) may overwrite PNG_FOUND + # and related variables. PNG_LIBRARY is naturally protected by + # FindPNG's "if(NOT PNG_LIBRARY)" guard, but PNG_PNG_INCLUDE_DIR + # (searched via find_path without a guard) and its derived variables + # (PNG_INCLUDE_DIR, PNG_LIBRARIES, PNG_VERSION_STRING) are not. + # Lock PNG_PNG_INCLUDE_DIR so that find_path() respects the cached + # bundled path and skips the system search. + set(PNG_PNG_INCLUDE_DIR "${PNG_INCLUDE_DIR}" CACHE INTERNAL "PNG include dir (bundled)") + endif() + set(HAVE_PNG YES) endif()