-
Notifications
You must be signed in to change notification settings - Fork 55
这个PR是用于适配构建工具Cup的更改 #298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
这个PR是用于适配构建工具Cup的更改 #298
Conversation
Walkthrough本次变更涉及构建系统、测试配置、头文件引用、条件编译和编辑器设置的调整。主要内容包括:新增和调整 CMake/CUP 配置文件、条件包含库头文件、标准化测试头文件引用方式、扩展 VSCode C++ 头文件关联、以及 Changes
Sequence Diagram(s)sequenceDiagram
participant 开发者
participant CMake
participant xege
participant test_framework
participant 测试可执行文件
开发者->>CMake: 配置项目(包含 CUP/新 include 路径/子目录)
CMake->>xege: 构建 xege 静态库(按条件包含头文件/链接库)
CMake->>test_framework: 构建测试框架静态库
CMake->>测试可执行文件: 构建各测试用例,链接 xege 和 test_framework
测试可执行文件->>xege: 运行测试时调用
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Possibly related PRs
Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (13)
.gitignore (1)
16-17: 新增 /target 忽略项 OK,但可统一书写风格
现有规则中既有build/(无前导/),也有/out与/target(有前导/)。前导/会将规则限定在仓库根目录;若将来某些子目录中也出现target/,可能无法被忽略。保持同一写法能避免歧义。建议:
-/out -/target +out/ +target/src/compress.cpp (1)
2-6: 硬编码"zlib/zlib.h"路径可读性一般,可考虑统一包含方式
若 Cup 后续将 zlib 目录放到 include path 中,则仍可直接#include <zlib.h>,避免显式写层级;或者在cup.toml中暴露ZLIB_INCLUDE_DIR宏来减少魔法字符串。当前实现没问题,只是可读性稍差。示例改动:
-#ifdef CUP_BUILD -#include "zlib/zlib.h" -#else -#include <zlib.h> -#endif +#if defined(CUP_BUILD) +# include <zlib.h> // Cup 已将 vendor/zlib 目录加入 includePath +#else +# include <zlib.h> +#endif.vscode/settings.json (1)
3-80: 建议:用通配符精简对标准库头文件的语言映射一次性枚举数十个标准/内部头文件不仅维护成本高,也容易遗漏或与未来新标准头脱节。VS Code 支持 glob pattern,例如:
"files.associations": { "*.temp": "plaintext", "**/{algorithm,atomic,bitset,*.h,*stddef*,*.stdint*,*.cxx}": "cpp" }这样既覆盖常见 STL 头,又保持配置轻量。请评估是否可以用更通用的规则替代逐个列举。
tests/putimage_rotate_test.cpp (1)
6-8: 保持头文件次序以减少隐式依赖
performance_timer.h里可能依赖test_framework.h对宏定义或别名的声明,当前顺序为:#include "test_framework.h" #include "performance_timer.h" #include "image_generator.h"若文件间无相互依赖,可忽略;若存在,请保持被依赖文件在前。留意未来修改导致的顺序敏感问题。
src/CMakeLists.txt (1)
7-9: 新增include/私有搜索路径合理,但建议注明目的此处给
xege添加PRIVATE ${PROJECT_SOURCE_DIR}/include:
- 目的应是供 src 内部
.cpp引用拆分出的公共头。最好在注释中说明“此目录仅供库内部使用,外部请使用 export/”。- 若未来
include/中也有被外部项目引用的头文件,应改为PUBLIC而非PRIVATE。请根据实际需求确认。tests/putimage_alphablend_comprehensive_test.cpp (1)
30-34: 局部定义NOMINMAX→ 建议统一在 CMake 层设置当前只有本文件显式
#define NOMINMAX,其余测试依赖 CMake 的全局add_compile_definitions(NOMINMAX)。
为避免双重定义或遗漏,建议删除此处宏定义并完全依赖 CMake 统一设置,或反之。示例 diff:-#define NOMINMAX 1src/image.cpp (1)
25-29: 建议校验 Cup 模式下libpng头文件路径是否可解析加入条件编译后,Cup 构建时改为
#include "libpng/png.h"。请确认:
cup.toml/ CMake 在 Cup 模式下已把libpng的include目录(而不是其父目录)加入include_directories,否则可能出现找不到头文件的编译错误。- 二方包升级或切换到系统编译时,
CUP_BUILD宏状态与 include 路径保持一致,避免混用导致 ODR 或 ABI 冲突。- 其余源文件对
libpng的引用方式保持一致,便于后续维护。如路径需调整,可考虑改为仅
#include <png.h>并在 Cup 模式通过include_directories指向libpng根目录,减少硬编码路径。cup.toml (2)
28-36: VS 编译选项遗漏了/Zc:__cplusplus,与 CMake 配置不一致在
tests/tests/CMakeLists.txt中对 MSVC 目标统一添加了/Zc:__cplusplus以启用正确的__cplusplus值。
建议在 Cup 的 VS 生成器同样加入,保持行为一致:compile_options = ["/utf-8", "/Zc:__cplusplus"]
37-58:link_libs列表出现重复,可通过 table 继承减少维护成本
"MinGW Makefiles"与"Ninja"两个生成器的link_libs完全一致,若后续需要增删库,需要两处同步修改。
可考虑抽取为自定义 profile 或通过generator."*"通配减少重复。tests/tests/CMakeLists.txt (4)
5-8: 全局add_definitions会污染所有目标最好改用
target_compile_definitions,只作用于需要的 target,避免库用户意外继承:target_compile_definitions(test_framework PUBLIC SHOW_CONSOLE=1 EGE_GRAPH_NO_LIB NOMINMAX)
10-19: 系统库列表在每个可执行 target 重复链接,可抽象以降低重复可封装函数或使用
target_link_libraries(<exe> PRIVATE ${COMMON_LINK_LIBS})。
重复代码越多,后续修改(如新增d2d1)就越容易遗漏。
31-35:test_framework静态库本身无需链接系统库该库只暴露工具函数/类,不直接调用 Windows API,把系统库链接到最终可执行程序即可,能减少不必要的依赖传递。
45-114: 多个可执行目标的重复 CMake 逻辑可用宏简化目前
add_executable+target_link_libraries六次复制粘贴,不易维护。
示例重构:function(add_putimage_test name) add_executable(${name} ../${name}.cpp) target_link_libraries(${name} PRIVATE test_framework xege ${SYSTEM_LIBS}) set_target_properties(${name} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) add_test(NAME ${name} COMMAND ${name}) endfunction() foreach(t IN ITEMS performance basic alphablend transparent rotate comparison alphablend_comprehensive) add_putimage_test(putimage_${t}_test) endforeach()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/getimage.jpgis excluded by!**/*.jpgexamples/getimage.pngis excluded by!**/*.png
📒 Files selected for processing (18)
.gitignore(1 hunks).vscode/settings.json(2 hunks)CMakeLists.txt(2 hunks)cup.toml(1 hunks)examples/egelogo.rc(1 hunks)export/ege.h(1 hunks)src/CMakeLists.txt(1 hunks)src/compress.cpp(1 hunks)src/image.cpp(1 hunks)tests/CMakeLists.txt(1 hunks)tests/putimage_alphablend_comprehensive_test.cpp(1 hunks)tests/putimage_alphablend_test.cpp(1 hunks)tests/putimage_basic_test.cpp(1 hunks)tests/putimage_comparison_test.cpp(1 hunks)tests/putimage_performance_test.cpp(1 hunks)tests/putimage_rotate_test.cpp(1 hunks)tests/putimage_transparent_test.cpp(1 hunks)tests/tests/CMakeLists.txt(1 hunks)
🔇 Additional comments (14)
examples/egelogo.rc (1)
1-1: 仅去除尾部空格,变更无风险
资源路径保持不变,编译行为不受影响。export/ege.h (2)
48-52: 条件编译实现良好这段代码很好地实现了基于构建系统的条件库链接。通过
CUP_BUILD宏来控制链接不同的库文件,为 Cup 构建工具提供了支持,同时保持了向后兼容性。宏定义清晰,逻辑简单易懂。
58-60: pragma 指令使用正确正确地将硬编码的库名称替换为
_LINK_NAME宏,实现了条件库链接的目标。这种方式可以根据构建配置自动选择合适的库文件,既支持传统的 graphics.lib,也支持 Cup 构建系统的 xege.lib。tests/putimage_alphablend_test.cpp (1)
11-13: 确认 CMake 是否已为 tests 目录添加include_directories相对路径前缀被去掉后,编译器只能从默认包含路径或 CMake
target_include_directories中找到image_generator.h等文件。请确认tests/tests/CMakeLists.txt中已添加:target_include_directories(test_framework PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})否则在非 IDE 构建环境下可能出现“file not found”错误。
tests/putimage_transparent_test.cpp (1)
7-9: 同上:移除 “../” 后需要保证头文件搜索路径正确如果
test_framework静态库暴露的头文件不在默认搜索路径,请在 CMake 中通过PUBLICinclude 目录转出。否则跨目录引用时仍会失败。tests/putimage_comparison_test.cpp (1)
10-12: 确认包含路径及统一风格多数测试文件均迁移到同级包含路径,但仍有个别文件(如
putimage_basic_test.cpp)可能保留旧相对路径。建议统一,避免 IDE 与命令行行为不一致。tests/putimage_performance_test.cpp (1)
10-12: ✔️ 头文件包含路径调整与新 CMake 布局保持一致由相对路径改为直接文件名,配合
tests/tests/CMakeLists.txt中的
target_include_directories(test_framework …)已经能够解析到正确头文件;本段没有其它隐患。CMakeLists.txt (2)
95-95: 公开头目录改为export/与目录重构一致,👍外部使用者只需包含
export,符合封装意图。无额外问题。
262-262: 同步示例目录名:demo➜examples名称变更已在文件系统中完成,CMake 调用保持一致;务必同时更新 CI 脚本或文档中的运行示例指令。
tests/putimage_basic_test.cpp (1)
7-9: 测试头文件路径统一 OK修改与新的
target_include_directories(test_framework …)相匹配,无功能风险。tests/CMakeLists.txt (1)
1-1: 单行委托构建配置简洁有效
add_subdirectory(tests)将测试构建逻辑下沉,符合目录置换后的层级,且降低了顶层 CMakeLists 的复杂度。👍cup.toml (2)
10-12:jobs = 0的语义可能导致串行编译,请确认在 Cup 的语法中,
jobs = 0有的实现会被解释为 使用 1 个 job,而不是 使用全部 CPU。
请确认 Cup 对 0 的解释是否符合预期,否则 CI/本地构建都可能退化为单线程。
16-24:[tests]中仅列出工具源码,未包含真正的测试用例
tests/tests/*.cpp目录下还有putimage_*_test.cpp等文件,这些才是最终的可执行测试。
Cup 在生成 test target 时只会编译sources数组,当前配置会导致测试目标缺失。
请补充或使用通配符:sources = ["tests/tests/*.cpp"]tests/tests/CMakeLists.txt (1)
37-42:/source-charset:utf-8仅在 VS2019+ 支持,旧版本编译器会报错虽然工程主要面向 VS2022,但既然还保留了 VS2015/2017 的生成器,建议条件判断版本:
if (MSVC_VERSION GREATER_EQUAL 1920) # VS2019+ target_compile_options(test_framework PUBLIC /source-charset:utf-8) endif()
| [dependencies] | ||
| libpng = { url = "@Anglebase/libpng", version = "0.1.0" } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
缺少 zlib 依赖会导致 libpng 连接失败
源代码中 src/compress.cpp / src/image.cpp 明确包含/调用 zlib 接口,但此处仅声明了 libpng。
大多数二进制/源码包的 libpng 并不会自动拉取 zlib,结果是链接阶段找不到 compressBound 等符号。
建议显式声明:
[dependencies]
libpng = { url = "@Anglebase/libpng", version = "0.1.0" }
+zlib = { url = "@Anglebase/zlib", version = "0.1.0" }🤖 Prompt for AI Agents
In cup.toml around lines 13 to 15, the dependency declaration includes libpng
but is missing the required zlib dependency, which causes linking failures due
to unresolved zlib symbols. Add an explicit zlib dependency alongside libpng in
the dependencies section to ensure proper linking and avoid missing symbols like
compressBound.
| include_directories(../include) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
使用相对路径包含 ../include 易在目录结构调整时失效
应改为项目根路径,避免测试子目录被移动/重命名后找不到头文件:
-include_directories(../include)
+include_directories(${PROJECT_SOURCE_DIR}/include)🤖 Prompt for AI Agents
In tests/tests/CMakeLists.txt at lines 2 to 3, the include_directories uses a
relative path '../include' which can break if the directory structure changes.
Replace the relative path with an absolute path based on the project root
directory to ensure the include path remains valid even if the tests
subdirectory is moved or renamed.
|
有冲突呀, @Anglebase 有没有时间解决一下冲突, 然后就可以考虑合入了~ |
|
看了下修改的目录结构, 感觉这种实现方式不太好, 强制要求根目录有一个 export 目录。 总之就是这种集成方式影响项目本身结构了, 侵入性太强了... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adapts the project structure and build configuration to support the Cup build tool, implementing directory restructuring and conditional compilation.
- Restructures directories:
demo→examples, separates headers/sources, and reorganizes test structure - Adds conditional compilation support for Cup build system using
CUP_BUILDpreprocessor flag - Introduces Cup configuration file and updates CMake build scripts to support both build systems
Reviewed Changes
Copilot reviewed 16 out of 101 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/tests/CMakeLists.txt | New CMake configuration for reorganized test structure with complete build setup |
| tests/*.cpp | Updates include paths from relative ../ to direct includes for reorganized structure |
| tests/CMakeLists.txt | Simplified to delegate to subdirectory build configuration |
| src/image.cpp | Adds conditional libpng include path for Cup builds |
| src/compress.cpp | Adds conditional zlib include path for Cup builds |
| src/CMakeLists.txt | Adds include directory for project headers |
| export/ege.h | Implements conditional library linking logic for different build systems |
| cup.toml | New Cup build tool configuration file |
| CMakeLists.txt | Updates include paths and example directory references |
| .vscode/settings.json | Extends file associations for development environment |
| @@ -0,0 +1,137 @@ | |||
| # 包含EGE头文件目录 | |||
| include_directories(../include) | |||
Copilot
AI
Aug 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using include_directories() is deprecated. Use target_include_directories() with specific scope (PUBLIC/PRIVATE/INTERFACE) instead to avoid polluting the global include namespace.
| include_directories(../include) | |
| # Include directories for specific targets |
| add_definitions(-DSHOW_CONSOLE=1) | ||
| add_definitions(-DEGE_GRAPH_NO_LIB) | ||
| add_definitions(-DNOMINMAX) |
Copilot
AI
Aug 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using add_definitions() is deprecated. Use target_compile_definitions() with specific scope instead to avoid polluting the global compile definitions.
| add_definitions(-DSHOW_CONSOLE=1) | |
| add_definitions(-DEGE_GRAPH_NO_LIB) | |
| add_definitions(-DNOMINMAX) | |
| target_compile_definitions(test_framework PUBLIC -DSHOW_CONSOLE=1) | |
| target_compile_definitions(test_framework PUBLIC -DEGE_GRAPH_NO_LIB) | |
| target_compile_definitions(test_framework PUBLIC -DNOMINMAX) |
| add_definitions(-DSHOW_CONSOLE=1) | ||
| add_definitions(-DEGE_GRAPH_NO_LIB) | ||
| add_definitions(-DNOMINMAX) |
Copilot
AI
Aug 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using add_definitions() is deprecated. Use target_compile_definitions() with specific scope instead to avoid polluting the global compile definitions.
| add_definitions(-DSHOW_CONSOLE=1) | |
| add_definitions(-DEGE_GRAPH_NO_LIB) | |
| add_definitions(-DNOMINMAX) | |
| target_compile_definitions(test_framework PUBLIC -DSHOW_CONSOLE=1 -DEGE_GRAPH_NO_LIB -DNOMINMAX) | |
| target_compile_definitions(putimage_performance_test PUBLIC -DSHOW_CONSOLE=1 -DEGE_GRAPH_NO_LIB -DNOMINMAX) |
| add_definitions(-DSHOW_CONSOLE=1) | ||
| add_definitions(-DEGE_GRAPH_NO_LIB) | ||
| add_definitions(-DNOMINMAX) |
Copilot
AI
Aug 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using add_definitions() is deprecated. Use target_compile_definitions() with specific scope instead to avoid polluting the global compile definitions.
| add_definitions(-DSHOW_CONSOLE=1) | |
| add_definitions(-DEGE_GRAPH_NO_LIB) | |
| add_definitions(-DNOMINMAX) | |
| target_compile_definitions(test_framework PUBLIC -DSHOW_CONSOLE=1) | |
| target_compile_definitions(test_framework PUBLIC -DEGE_GRAPH_NO_LIB) | |
| target_compile_definitions(test_framework PUBLIC -DNOMINMAX) |
包含更改
重构(目录结构更改)
demo被重命名为examplestests目录内容与tests/tests目录内容交换include被重命名为exportsrc文件夹下头文件与源文件分离,头文件复制到新建include文件夹下以上修改均已修改对应的CMakeLists.txt文件,且已测试构建
文件更改
.gitignore添加/target忽略Cup的构建目录export/ege.h、src/compress.cpp和src/image.cpp添加条件编译以兼容Cup构建tests/putimage_alphablend_comprehensive_test.cpp添加NOMINMAX宏屏蔽<Windows.h>中的min和max宏定义cup.toml未对脚本内容进行任何更改
Cup 包发行过程说明
Cup 通过检索仓库的发行版标签来获取发行版并安装到本地,发行时建议创意一个专门用于发行的分支(例如
publish-vX.Y.Z),并清理发行非必要的文件(例如文档),然后通过该分支创建一个发行版标签,Cup要求发行版标签必须为vX.Y.Z的形式Bug反馈
在本地构建xege时(未做更改,CMake, Visual Studio 17 2022)原demo目录下的源文件构建不出可执行文件(使用Cup测试构建时可以正常构建出可执行文件),我也并没有发现 CMake 代码有什么Bug
Summary by CodeRabbit
新功能
修复与优化
样式与配置
测试
重构