Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/upload-mobile-dlls/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ runs:
-D CMAKE_OSX_ARCHITECTURES="arm64"
-D CMAKE_OSX_DEPLOYMENT_TARGET=13.0
-D CMAKE_BUILD_TYPE=Release
-D CMAKE_C_FLAGS="-DPNG_ARM_NEON_OPT=0"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

CMAKE_C_FLAGSを直接上書きしないでください
cmake -D CMAKE_C_FLAGS="..." でキャッシュ値を丸ごと置き換えると、Apple向けツールチェーンが自動で設定していた必須フラグ(-fembed-bitcode-isysroot など)が消失し、最悪ビルド失敗や提出要件を満たせなくなる恐れがあります。PNGのNEON最適化だけを抑止したいのであれば、target_compile_definitions() 等で該当ターゲットに PNG_ARM_NEON_OPT=0 を付与するか、CMake側に専用オプションを生やして切り替えるようにしてください。

🤖 Prompt for AI Agents
.github/actions/upload-mobile-dlls/action.yml around line 91: this change
overwrites the entire cached CMAKE_C_FLAGS via -D CMAKE_C_FLAGS="...", which
removes Apple toolchain required flags (e.g. -fembed-bitcode, -isysroot) and can
break builds; instead, stop setting CMAKE_C_FLAGS directly and apply
PNG_ARM_NEON_OPT=0 to the specific build target(s) (use
target_compile_definitions() in the CMakeLists for the iOS-related targets) or
add a dedicated CMake option (e.g. -DPNG_ARM_NEON_OPT=0 used within CMake to set
a definition) so only the PNG_NEON macro is disabled without replacing global
compiler flags.

shell: ${{ inputs.shell_type }}

- name: Build by CMake
Expand Down
2 changes: 1 addition & 1 deletion 3rdparty/libpng
2 changes: 1 addition & 1 deletion 3rdparty/zlib
Submodule zlib updated 110 files
34 changes: 34 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,41 @@ set(PNG_ARM_NEON "off"
set(PNG_ARM_NEON_OPT "0"
CACHE STRING "Enable ARM NEON optimizations")
set(PNG_STATIC "defined" CACHE STRING "" FORCE)

Comment on lines 290 to +291
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

PNG_STATIC の再代入(型不整合)を削除してください

PNG_STATIC ON の直後に set(PNG_STATIC "defined" ...) を文字列で再代入しており、オプションの意味を壊します。重複かつ誤用の可能性が高いです。後者の行を削除してください。Windows の消費側に PNG_STATIC マクロが必要なら、ターゲット側へ target_compile_definitions(... PUBLIC PNG_STATIC) を付けるのが正攻法です(別コメントで提案)。

-set(PNG_STATIC "defined" CACHE STRING "" FORCE)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
set(PNG_STATIC "defined" CACHE STRING "" FORCE)
🤖 Prompt for AI Agents
In CMakeLists.txt around lines 290-291, remove the erroneous re-assignment
set(PNG_STATIC "defined" CACHE STRING "" FORCE) because it overwrites the
PNG_STATIC option with a string (type mismatch) and breaks the option; simply
delete that line and, if consumers on Windows actually need a PNG_STATIC
preprocessor macro, add target_compile_definitions(<target> PUBLIC PNG_STATIC)
on the appropriate targets instead of re-setting the cached variable.

# macOS 15 (Sequoia) 以降でfp.hが無いことによるエラーの対応
# libpng 1.6.37ではfp.hをincludeしているが、
# macOS 15以降では fp.h がシステムから削除されているためビルドエラーになる。math.hに統合されている
# libpngはUnrealEngineで利用している物とバージョンを揃える必要があるためアップデートが出来ないため、fp.hという名前のヘッダーを作成し、
# その中でmath.hをincludeするというアプローチでコンパイルを通す

if(APPLE)
set(FP_H_COMPAT_DIR "${CMAKE_CURRENT_BINARY_DIR}/include_compat")
file(MAKE_DIRECTORY "${FP_H_COMPAT_DIR}")
file(WRITE "${FP_H_COMPAT_DIR}/fp.h"
"/* macOS 15以降で削除された fp.h の互換ヘッダー
* fp.h の機能は math.h に統合されているため、math.h をインクルードします。
*/
#ifndef _FP_H_COMPAT_
#define _FP_H_COMPAT_
#include <math.h>
#endif
")
include_directories(BEFORE SYSTEM "${FP_H_COMPAT_DIR}")

# libpngのビルド時のみCFLAGSを一時的に変更するため、現在の値を保存
set(CMAKE_C_FLAGS_SAVE "${CMAKE_C_FLAGS}")
set(CMAKE_CXX_FLAGS_SAVE "${CMAKE_CXX_FLAGS}")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -isystem ${FP_H_COMPAT_DIR}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -isystem ${FP_H_COMPAT_DIR}")
endif()

add_subdirectory("3rdparty/libpng")

# libpngのビルド後、CFLAGSを元に戻す(他のサブプロジェクトに影響を与えないため)
if(APPLE)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS_SAVE}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS_SAVE}")
endif()
set(PNG_INCLUDE_DIRS
"${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libpng"
"${CMAKE_CURRENT_BINARY_DIR}/3rdparty/libpng")
Expand Down
22 changes: 10 additions & 12 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,22 @@ if( NOT(IOS OR ANDROID) )
# 注意点:
# libpngのバージョンは、Unreal EngineのPngImageWrapper.hに記載のバージョンに合わせる必要があります。
# そうでないと、UEでパッケージ化したあとexeを起動したときにassertion failedのダイアログが出て強制終了になります。
# Unreal Engineの対応バージョンを上げるときには併せてPngImageWrapperで利用のlibpngバージョンを確認し、
# libpngのバージョンが上がる場合はlibplateau submoduleのlibpngを新しいバージョンにしたうえで下のpng15_staticの15を{major_ver}{minor_ver}に置き換える必要がありそうです。
add_dependencies(plateau libjpeg-turbo png15_static tiff hmm)
target_link_libraries(plateau PRIVATE ${JPEG_TURBO_LIBRARIES} png15_static tiff hmm)
add_dependencies(plateau libjpeg-turbo png_static tiff hmm)
target_link_libraries(plateau PRIVATE ${JPEG_TURBO_LIBRARIES} png_static tiff hmm)
target_include_directories(plateau PRIVATE
Comment on lines +34 to 36
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

❓ Verification inconclusive

png15_static → png_static 置換は妥当。Windows 向けに PNG_STATIC を公開定義してください

差し替えは正しいです。Windows で静的 libpng を使う場合、消費側が PNG_STATIC を定義していないと __declspec(dllimport) との不整合でリンク警告/失敗の恐れがあります。plateau に公開定義を追加してください。

 add_dependencies(plateau libjpeg-turbo png_static tiff hmm)
 target_link_libraries(plateau PRIVATE ${JPEG_TURBO_LIBRARIES} png_static tiff hmm)
+if(WIN32)
+  target_compile_definitions(plateau PUBLIC PNG_STATIC)
+endif()

🏁 Script executed:

#!/bin/bash
# リポジトリ内に png15_static の残存参照がないか確認
rg -nP 'png15_static'

Length of output: 0


Windowsビルド向けにPNG_STATIC定義を追加
静的リンクした libpng と __declspec(dllimport) の不整合によるリンク警告/失敗を防ぐため、CMakeLists.txt に以下を追加してください。

 add_dependencies(plateau libjpeg-turbo png_static tiff hmm)
 target_link_libraries(plateau PRIVATE ${JPEG_TURBO_LIBRARIES} png_static tiff hmm)
+if(WIN32)
+  target_compile_definitions(plateau PUBLIC PNG_STATIC)
+endif()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
add_dependencies(plateau libjpeg-turbo png_static tiff hmm)
target_link_libraries(plateau PRIVATE ${JPEG_TURBO_LIBRARIES} png_static tiff hmm)
target_include_directories(plateau PRIVATE
add_dependencies(plateau libjpeg-turbo png_static tiff hmm)
target_link_libraries(plateau PRIVATE ${JPEG_TURBO_LIBRARIES} png_static tiff hmm)
if(WIN32)
target_compile_definitions(plateau PUBLIC PNG_STATIC)
endif()
target_include_directories(plateau PRIVATE
🤖 Prompt for AI Agents
In src/CMakeLists.txt around lines 34 to 36, the build can get link
warnings/failures on Windows due to using a statically linked libpng while
symbols expect __declspec(dllimport); add the PNG_STATIC preprocessor definition
for the plateau target when building on Windows. Modify the CMake file to set
the definition only for Windows (e.g., check WIN32 or MSVC) and apply it to the
plateau target via target_compile_definitions(... PRIVATE PNG_STATIC) so the
static libpng export/import mismatch is avoided.

"${JPEG_TURBO_INCLUDE_DIRS}"
"${PNG_INCLUDE_DIRS}"
"${TIFF_INCLUDE}"
"${HMM_INCLUDE}"
)
else()
target_link_libraries(plateau PUBLIC
"citygml"
"GLTFSDK"
"crypto"
"ssl"
target_link_libraries(plateau PUBLIC
"citygml"
"GLTFSDK"
"crypto"
"ssl"
"c_wrapper"
"png15_static"
"png_static"
"${LibOpenMeshCore}"
"${LibOpenMeshTools}"
"hmm"
Expand Down Expand Up @@ -97,7 +95,7 @@ if(BUILD_LIB_TYPE STREQUAL "static")
$<TARGET_FILE:LibXml2>
$<TARGET_FILE:httplib>
$<TARGET_FILE:zlibstatic>
$<TARGET_FILE:png15_static>
$<TARGET_FILE:png_static>
$<TARGET_FILE:tiff>
$<TARGET_FILE:hmm>
${JPEG_TURBO_LIBRARIES}
Expand All @@ -116,7 +114,7 @@ if(BUILD_LIB_TYPE STREQUAL "static")
endif()

if(BUILD_LIB_TYPE STREQUAL "static")
set(COMBINE_LIB_DEPENDS plateau citygml xerces-c GLTFSDK crypto ssl LibXml2 httplib zlibstatic libjpeg-turbo png15_static tiff ${LibOpenMeshCore} ${LibOpenMeshTools} hmm)
set(COMBINE_LIB_DEPENDS plateau citygml xerces-c GLTFSDK crypto ssl LibXml2 httplib zlibstatic libjpeg-turbo png_static tiff ${LibOpenMeshCore} ${LibOpenMeshTools} hmm)

# windows で libファイル結合します。
# 参考 : https://stackoverflow.com/questions/60190374/how-to-bundle-multiple-static-libraries-into-single-library-in-cmake-for-windows
Expand Down