mirror of
https://github.com/opencv/opencv.git
synced 2026-07-21 19:33:03 +04:00
Merge pull request #28995 from donotsleepy:fix/png-found-status-guard
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
This commit is contained in:
+5
-1
@@ -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)
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user