-
Notifications
You must be signed in to change notification settings - Fork 55
使用一些现代C++特性,thread_queue改写为无锁队列 #289
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?
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Walkthrough本次更新主要聚焦于代码风格现代化、宏到类型安全模板/函数的替换、C++标准兼容性提升以及部分内部结构的无锁重构。涉及头文件和源文件的重构、格式整理、宏替换为 Changes
Sequence Diagram(s)sequenceDiagram
participant Producer as Producer
participant Queue as thread_queue<T>
participant Consumer as Consumer
Producer->>Queue: push(data)
Note right of Queue: 原子操作推进_end索引
alt 队列已满
Queue->>Queue: 原子推进_begin丢弃最旧
end
Queue->>Queue: 原子写入元素
Consumer->>Queue: pop(data)
Note right of Queue: 等待_head_locked为false
Queue->>Queue: 原子推进_unoccupied和_begin
Queue-->>Consumer: 返回元素
Poem
✨ 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: 7
🔭 Outside diff range comments (1)
src/egegapi.cpp (1)
93-100: putpixel 的视口边界判断可能错误,存在越界写风险
in_rect()期望的是“宽/高”而非“右下绝对坐标”。
当前把img->m_vpt.right / bottom直接当宽高使用,当视口left/top非零且调用者传入负x/y时,x += left后仍可能位于[0, left)区间,被误判为合法并写到视口外、图像内的区域。- if (in_rect(x, y, img->m_vpt.right, img->m_vpt.bottom)) { + int vptW = img->m_vpt.right - img->m_vpt.left; + int vptH = img->m_vpt.bottom - img->m_vpt.top; + if (in_rect(x - img->m_vpt.left, y - img->m_vpt.top, vptW, vptH)) { img->m_pBuffer[y * img->m_width + x] = color; }这样既保证落点在视口内部,也可避免写越界。
🧹 Nitpick comments (8)
CMakeLists.txt (1)
3-4:CMAKE_CXX_STANDARD设置存在重复/覆盖风险文件顶部强制
set(CMAKE_CXX_STANDARD 11),而稍后若探测到编译器支持 C++17,又会再次set(CMAKE_CXX_STANDARD 17); set(CMAKE_CXX_STANDARD_REQUIRED ON)。
虽然第二次赋值会覆盖第一次,但多处设置同一变量易导致维护困惑,且若用户在外部 toolchain 文件中再次设置,会出现不可预测的版本选择。建议统一在探测结果之后仅设置一次,或显式使用
CACHE并加注释说明覆盖顺序。src/ege_def.h (1)
9-14:EGE_CONSTEXPR的条件判断可再增强可移植性目前基于
__cplusplus >= 201703L判定,但旧版 MSVC 需/Zc:__cplusplus才会给出正确值,且存在部分 Clang/AppleClang 报值为201402L却已支持完整 C++17constexpr的情况。可以追加
__cpp_constexpr特性宏判定,更精确:-#if __cplusplus >= 201703L +#if __cplusplus >= 201703L || (defined(__cpp_constexpr) && __cpp_constexpr >= 201603L)这样对跨编译器更稳健。
src/console.cpp (1)
94-94: 考虑保持与 Windows API 的类型一致性虽然从
BOOL到bool的隐式转换可以工作,但 Windows API 函数(如GetConsoleScreenBufferInfo、FillConsoleOutputCharacter等)返回的是BOOL类型。建议保持BOOL类型以保持与 Windows API 的一致性。src/thread_queue.h (1)
29-30: 修正注释中的拼写错误注释中 "失败直接跳过计科" 应该是 "失败直接跳过即可"。
- // 并发访问时,只要有一个线程清理就可以了,因此只需要CAS一次,失败直接跳过计科 + // 并发访问时,只要有一个线程清理就可以了,因此只需要CAS一次,失败直接跳过即可src/image.cpp (2)
33-34: 避免在命名空间级别引入using std::swap
在头部直接using std::swap;会把不带限定符的swap暴露到整个ege命名空间,增加 ADL/名字冲突的风险,特别是项目中如果后来为自定义类型实现了专门的swap重载时可能导致调用歧义。建议删除该语句,并在实际需要的地方显式写std::swap。
728-731: 微小风格建议:统一使用std::minpng_size_t count = min(cursor.size - cursor.cur, byteCountToRead);项目现在既有自定义
min模板,也包含<algorithm>的std::min。为避免混淆并借助标准库实现,推荐改为std::min(...)并在文件顶部添加#include <algorithm>。src/egegapi.cpp (2)
558-606: switch 分支缺省静默回落,建议显式处理未知枚举
update_pen()中对linecap/linejoin的switch,若未来新增枚举值将自动落到default并默默退化,难以及时发现。建议在
default中加入assert(false)或日志,或使用std::unreachable()(C++23)来显式标记不可达路径,并在落到默认值前加[[fallthrough]]注释。
1426-1428: 硬编码数组hatchmap可改为constexpr std::array并补注释当前
int hatchmap[] = { … };靠索引隐式映射到pattern,可读性较差。
建议:static constexpr std::array<int, 10> kPatternToHatch { HS_HORIZONTAL, HS_BDIAGONAL, /* … */ }; lbr.lbHatch = kPatternToHatch[pattern - 2];并在上方说明
pattern与BS_HATCHED的映射规则,方便维护。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.clangd(1 hunks).gitignore(1 hunks)CMakeLists.txt(1 hunks)include/ege/stdint.h(9 hunks)src/array.h(5 hunks)src/color.cpp(5 hunks)src/color.h(8 hunks)src/compress.cpp(3 hunks)src/console.cpp(5 hunks)src/ege_common.h(0 hunks)src/ege_def.h(1 hunks)src/ege_math.h(3 hunks)src/egegapi.cpp(114 hunks)src/encodeconv.h(1 hunks)src/image.cpp(60 hunks)src/thread_queue.h(1 hunks)src/utils.h(1 hunks)
💤 Files with no reviewable changes (1)
- src/ege_common.h
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/ege_def.h (2)
src/ege_math.h (1)
EGE_CONSTEXPR(42-45)src/color.h (6)
EGE_CONSTEXPR(63-70)EGE_CONSTEXPR(81-102)EGE_CONSTEXPR(117-125)EGE_CONSTEXPR(134-137)EGE_CONSTEXPR(147-151)EGE_CONSTEXPR(165-173)
src/utils.h (2)
src/ege_math.h (1)
T(47-50)src/random.cpp (1)
a(108-112)
src/color.cpp (2)
src/utils.h (4)
min(6-6)min(11-11)max(18-18)max(23-23)src/ege_math.h (1)
DIVIDE_255_FAST(42-42)
src/color.h (1)
src/ege_math.h (2)
EGE_CONSTEXPR(42-45)DIVIDE_255_FAST(42-42)
src/image.cpp (6)
src/image.h (15)
width(62-62)width(63-63)width(106-106)width(107-107)imgDest(146-154)imgDest(156-165)imgDest(167-179)imgDest(181-190)imgDest(192-199)imgDest(201-211)imgDest(213-221)xDest(118-118)xDest(119-119)xSrc(110-110)img(88-88)src/ege_head.h (1)
newbitmap(321-321)include/ege/types.h (40)
width(215-215)width(244-244)width(402-402)width(408-408)width(410-410)width(412-412)width(559-559)width(565-565)width(715-715)width(721-721)width(881-881)width(881-881)height(403-403)height(411-411)height(560-560)height(716-716)height(882-882)height(882-882)size(252-252)size(407-407)size(564-564)size(720-720)size(884-884)size(884-884)size(1372-1372)size(1372-1372)size(2044-2044)size(2044-2044)scale(502-502)scale(648-648)scale(796-796)scale(1172-1175)scale(1172-1172)scale(1177-1181)scale(1177-1177)scale(1183-1189)scale(1183-1183)scale(1581-1584)s(311-311)s(370-370)src/utils.h (2)
min(6-6)min(11-11)src/ege_dllimport.cpp (2)
AlphaBlend(107-113)AlphaBlend(107-107)src/ege_dllimport.h (1)
AlphaBlend(31-31)
src/egegapi.cpp (12)
include/ege/egecontrolbase.h (8)
x(90-96)x(90-90)w(125-129)w(125-125)w(131-135)w(131-131)w(194-203)w(194-194)src/image.h (7)
width(62-62)width(63-63)width(106-106)width(107-107)xDest(118-118)xDest(119-119)xSrc(110-110)include/ege/sys_edit.h (12)
w(145-149)w(145-145)h(109-112)h(109-109)h(114-137)h(114-114)color(164-168)color(164-164)text(151-154)text(151-151)text(156-156)text(156-156)src/color.h (4)
colorblend_inline(63-63)colorblend_inline_fast(81-81)alphablend_inline(134-134)alphablend_inline(147-147)src/ege_math.h (5)
round(13-16)round(13-13)round(18-21)round(18-18)clamp(47-47)src/utils.h (4)
max(18-18)max(23-23)min(6-6)min(11-11)src/graphics.cpp (2)
gdiplusinit(1101-1107)gdiplusinit(1101-1101)src/encodeconv.h (1)
mb2w(17-17)src/encodeconv.cpp (2)
mb2w(35-42)mb2w(35-35)src/font.cpp (1)
origin(735-735)src/ege_time.h (1)
get_highfeq_time_ls(7-7)src/time.cpp (2)
get_highfeq_time_ls(213-251)get_highfeq_time_ls(213-213)
🪛 Gitleaks (8.26.0)
src/image.cpp
3273-3273: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (30)
src/encodeconv.h (1)
19-19: 命名空间闭合注释的添加保持了一致性 👍仅补充了
// namespace ege注释,提升可读性,无任何副作用。.gitignore (1)
2-2: 忽略.cache/目录是合理的补充常见的构建/IDE 缓存目录,纳入忽略列表可以避免污染版本库。
src/ege_def.h (1)
16-21: 宏展开顺序 OK将迭代器/运行时库 mismatch 宏展开提前到单层
#ifndef,逻辑等价且更简洁,无异议。src/compress.cpp (1)
45-46:ege_uncompress路径良好在确认源长度有效后才调用
uncompress,并未出现与上文类似的副作用,逻辑保持正确。src/console.cpp (1)
16-16: 使用标准 C++ 布尔字面量将
FALSE改为false是个好的改进,符合现代 C++ 标准。src/thread_queue.h (1)
18-42: 请确认thread_queue的无锁队列实现并补充相关测试未在项目的测试目录中发现任何针对
thread_queue或原子操作逻辑的单元/并发测试。建议手动验证或补充以下内容:
- 为
push/pop方法编写单元测试,覆盖正常与边界(空队列、满队列)场景- 添加多线程并发测试,验证
_unoccupied、_begin与_end在高并发下的正确性- 审查并确保
compare_exchange_*操作前后添加必要的内存屏障,以防止重排序问题include/ege/stdint.h (1)
29-29: 格式化改进提升了代码可读性typedef 声明和宏定义的对齐格式改进使代码更加整洁易读。
Also applies to: 51-51, 72-72, 84-114, 157-160, 179-182, 249-257
src/ege_math.h (2)
42-45: 将宏转换为类型安全的模板函数将
DIVIDE_255_FAST从宏改为 constexpr 模板函数是很好的现代化改进,提供了类型安全性和编译时求值能力。实现的快速除以 255 算法是正确的。
47-47: 添加 constexpr 支持编译时求值为
clamp、sumIsOverflow和sumIsUnderflow函数添加constexpr是很好的改进,允许这些函数在编译时求值,提高性能。Also applies to: 53-53, 59-59
src/array.h (6)
3-3: 良好的标准合规性改进添加
#include <cstddef>提高了C++标准合规性,这是现代C++的良好实践。
12-12: 现代C++别名声明使用
using别名声明替代typedef是现代C++的推荐做法,提高了代码可读性。
17-19: 优秀的构造函数现代化使用成员初始化列表替代构造函数体内赋值是最佳实践,可以提高性能并避免不必要的默认构造。
44-44: 现代化的默认构造函数使用初始化列表的默认构造函数更加简洁高效。
46-51: 改进的拷贝构造函数使用成员初始化列表优化了拷贝构造函数的实现。
63-63: 一致的现代C++实践正确使用
nullptr替代NULL,符合现代C++标准。src/color.cpp (5)
27-36: 优秀的宏到lambda转换将
IFSWAP宏替换为作用域限定的lambda函数显著提高了类型安全性和代码清晰度,避免了宏污染。
61-63: 一致的颜色格式化函数现代化将颜色格式化相关宏转换为lambda函数保持了局部作用域,提高了类型安全性。这些转换与整体现代化目标完全一致。
Also applies to: 69-71, 87-89
147-149: HSL转换函数的现代化改进颜色饱和度和亮度处理函数的lambda转换实现良好,保持了原有逻辑的同时提升了代码质量。
Also applies to: 155-160, 163-168
212-213: 模板函数替换宏的正确应用使用来自
src/utils.h的min和max模板函数替代宏是类型安全的改进,支持编译时求值且具有更好的错误检查。
383-386: 代码格式的一致性改进
DIVIDE_255_FAST调用周围的间距调整提高了代码可读性,与模板化的实现保持一致。src/utils.h (4)
6-9: 基础模板函数实现单参数
min函数作为递归的基础情况,实现简洁正确。
11-16: 优秀的变参模板设计变参模板
min函数具有以下优点:
static_assert确保所有参数类型一致,提供清晰的错误信息- 递归实现模式正确且高效
EGE_CONSTEXPR支持编译时求值
18-21: 对称的最大值函数基础单参数
max函数与min函数设计一致。
23-28: 类型安全的变参最大值函数
max函数实现与min函数保持对称性,类型检查和递归模式都正确实现。这种设计替代了不安全的宏,是显著的改进。src/color.h (5)
20-23: 宏到constexpr函数的优秀转换将
RGBTOBGR宏转换为constexpr内联函数提高了类型安全性,同时保持了编译时求值能力。
27-30: 一致的颜色转换函数现代化
ARGBTOZBGR的constexpr转换与其他颜色函数保持一致,注释清晰说明了其用途。
63-70: 性能保持的constexpr颜色混合
colorblend_inline函数添加EGE_FORCEINLINE和EGE_CONSTEXPR在保持性能的同时启用了编译时求值。
81-102: 现代C++分支的优秀应用使用
if constexpr替代预处理器条件编译是现代C++17的最佳实践,保持了编译时分支选择的同时提高了代码清晰度。
117-124: 全面的alpha混合函数现代化所有alpha混合相关函数都正确添加了
EGE_CONSTEXPR和EGE_FORCEINLINE,在启用编译时求值的同时保持了性能特征。实现保持了原有的混合算法逻辑。Also applies to: 134-136, 147-150, 165-172
src/egegapi.cpp (1)
3043-3047: 帧率统计用静态变量,全局并发场景存竞态
static_frameCount等变量未加同步;若渲染线程与查询线程并行调用updateFrameRate()/getfps(),会发生数据竞争。
如引擎允许多线程渲染,建议改为:
- 使用
std::atomic<double/int>;或- 将计数器移入单渲染线程上下文。
请确认调用模型是否单线程。
|
@pointertobios 除了关键的内容,其它格式化方面的提交就不要放在这里了,后面很难溯源。 commit 用 rebase 整理一下,比如 "开启 C++17" 在这次 PR 里是无关的。 |
#287
Summary by CodeRabbit
新特性
.clangd配置文件,指定默认使用 C++17 标准进行编译。重构
thread_queue队列由互斥锁实现改为无锁原子操作实现,提高并发性能。std::swap。样式
杂项
.gitignore增加.cache/目录忽略。