Refactor API status handling and add occlusion culling - #95
Conversation
🌿 Roseau API Breaking Change Report
🔴 rendering — 5 breaking change(s)
|
Reason: script execution via -e/-c flag Reply |
|
⏱️ Rate limited. Waiting 2.9s (attempt 2/3)... |
|
⏱️ Rate limited. Waiting 4.9s (attempt 3/3)... |
|
❌ Rate limited after 3 retries — Concurrency limit exceeded for user, please retry later |
|
API call failed after 3 retries: Concurrency limit exceeded for user, please retry later |
|
分析完成。以下是审查意见。 代码审查摘要 — PR #95操作: synchronize 🔴 关键(合并前需修复)
|
| 声称 | 状态 | 对应文件 |
|---|---|---|
| OcclusionCuller/OcclusionKey/OcclusionMethod 抽象 | ✅ | optimization/occlusion/*.java |
| 查询渲染管线 + 顶点/片元着色器 + FullTransformsUbo | ✅ | ALRPipelines.OCCLUSION_QUERY, occlusion_query.vsh/.fsh |
| GPU 设备接口 + OpenGL samples-query(GlSamplesQuery 生命周期) | GlSamplesQuery 存在但含 2 个 🔴 bug | |
| 可复用 GPU 资源 + ring buffer 基础(Query 池 / Query 缓冲池) | ✅ | GpuReusableResource(RingBuffer), LoopResetPool, SimpleResetPool, QueryBufferPack(RingBuffer) |
| Query 帧状态管理(当前帧提交执行、下帧读结果) | ✅ | FrameState, GpuQueryOcclusionCuller |
| compute-pass debug-group 懒 Supplier | ✅ | ALRComputePass(+Backend), GlComputePassBackend, GlDeviceMixin |
| ApiStatus.Internal 注解调整 | ✅ | 15 个 mixin 移除 + 内部实现类补注 |
| [x] Samples Query backend | 已实现但 glEndQuery/越界读使其实质不可用 | |
| [ ] Hi-Z backend | ✅(按 TODO 未声称完成) | 仅存 stub + depth_convert/ffx_spd shader + SPD pipeline 注册,未接线 |
| [ ] 接入 vanilla SubmitNodeStorage | ✅(未声称完成) | 未实现,符合描述 |
结论: REQUEST_CHANGES — 基础设施和框架设计扎实,但 GlSamplesQuery 的两处错误(glEndQuery target 传参 + 4/8 字节越界读)会让已声称完成的 Samples Query 后端在接入时立即挂起渲染线程;修复后再合并。其余为 Hi-Z 接线前的潜在问题,建议顺手修正宽高顺序和默认 flag 映射。
由 Hermes Agent 审查
|
💾 Self-improvement review: Skill 'rendering-gpu-pr-review' created. |
|
所有文件已审查完毕。汇总分析结果,输出审查意见: 代码审查摘要 — PR #95 (synchronize)操作: synchronize 📋 声称验证表
🔴 关键(接入即错的 latent bug,建议合并前修)
|
3c58b50 to
4cdf68c
Compare
|
💾 Self-improvement review: Patched SKILL.md in skill 'rendering-gpu-pr-review' (1 replacement). |
Reason: script execution via -e/-c flag Reply |
🌿 Roseau API Breaking Change Report
🔴 rendering — 6 breaking change(s)
|
|
All verification complete. I have a full picture of the PR. Writing the review: 代码审查摘要 — PR #95操作: synchronize 🔴 关键(接入前必须修复)
|
| 声称 | 状态 | 对应文件 |
|---|---|---|
| OcclusionCuller / OcclusionKey / OcclusionMethod 抽象 | ✅ | 3 个新文件 |
| occlusion-query 管线 + 顶点/片元着色器 | ✅ | ALRPipelines.OCCLUSION_QUERY + vsh/fsh |
| GPU 设备接口扩展 + GlSamplesQuery | ✅ | ALRGpuDeviceExtension(+Backend)、GlDeviceMixin、GpuDeviceMixin、ALRHICapabilities |
| GPU 资源/ring-buffer 池基础 | ✅ | GpuReusableResource(Pool)、LoopResetPool、SimpleResetPool |
| Query 帧状态管理(本帧提交、次帧读结果) | ✅ | FrameState、GpuQueryOcclusionCuller、QueryInstance |
| pushDebugGroup 改 Supplier | ✅ | ALRComputePass 全链路 |
| ApiStatus.Internal 注解调整 | ✅ | 渲染 + sync 模块 |
| Samples Query 后端 | GpuQueryOcclusionCuller;ALROptimizations.getOcclusionCuller() 无调用者 |
|
| Hi-Z 后端 | HierarchicalZOcculusionCuller 空实现 + ffx_spd/depth_convert + UBOs;createInstance 返回 null |
|
| 集成 vanilla SubmitNodeStorage | ❌ TODO | 无对应代码 |
🧪 测试建议
| 被测目标 | 推荐测试场景 | 优先级 |
|---|---|---|
FrameState.runQueries/fetchResults |
连续多帧 key 增删:池无重复 acquire、结果延迟一帧生效、释放后复用安全 | 🔴 |
| UBO 布局(接入后) | IntSizeUbo/ConvertDepthParamsUbo 写入字节与 GLSL 声明逐字段对照 |
🔴 |
GpuQueryOcclusionCuller.shouldDraw |
相机在 AABB 内部 / 包围盒部分可见 / 完全被地形遮挡三场景 | 🟡 |
GlSamplesQuery.getValue |
查询未 begin/end 时读取、release 后结果重置、close 后抛异常 | 🟡 |
结论: REQUEST_CHANGES — 基础设施质量整体很高(GL 调用、池时序、帧状态设计都正确),但三个 🔴 均为确定性缺陷:两个 UBO 宽高互换是接入即现的错误、withCull(true) 在相机入盒场景会错误剔除、Hi-Z 占位桩默认返回 false 是隐患。三者修复成本都极低(各一行级别),建议修复后再合入,避免错误语义沉淀到后续接线 PR。ALROptimizations 的接线入口建议在本 PR 内补一个最小调用链(哪怕只在 debug 模式),让 latent 问题能在真实帧循环中提前暴露。
由 Hermes Agent 审查
|
💾 Self-improvement review: Patched SKILL.md in skill 'rendering-gpu-pr-review' (1 replacement). |
🌿 Roseau API Breaking Change Report
🔴 rendering — 9 breaking change(s)
|
|
审查完成,所有关键路径已交叉验证(本地克隆 AnvilLib@dev/26.1、MC 26.1 反编译产物、shader、GL 常量值)。以下是审查意见: 代码审查摘要 — PR #95操作: synchronize 🔴 关键(确定性代码缺陷,接入即触发)
|
| 声称 | 状态 | 对应文件 |
|---|---|---|
| OcclusionCuller/Key/Method 抽象 | ✅ | optimization/occlusion/* |
| 查询管线 + 顶点/片元 shader + FullTransformsUbo | ✅(含 🔴 绕序/cull 问题) | ALRPipelines, occlusion_query.*, FullTransformsUbo |
| GPU device 接口 + GL samples-query 封装 | ✅ | GlSamplesQuery, GpuQueryObject, GlDeviceMixin |
| 资源池 + ring buffer 基础 | ✅(含 |
GpuReusableResourcePool, LoopResetPool, SimpleResetPool |
| 帧状态管理(本帧提交/次帧读取) | ✅ | FrameState, GpuQueryOcclusionCuller |
| compute debug-group 惰性 Supplier | ✅ | ALRComputePass, GlComputePassBackend |
| ApiStatus.Internal 注解调整 | ✅ | 15 个 mixin 移除 + 内部类标注 |
| Hi-Z 后端 | HierarchicalZOcclusionCuller | |
| Samples Query 后端 | GpuQueryOcclusionCuller | |
| 接入 vanilla SubmitNodeStorage | ❌ TODO 未勾选(自认) | 无 |
🧪 测试建议
| 被测目标 | 推荐场景 | 优先级 |
|---|---|---|
ALRComputePass.bindAll |
首次调用即 IOOBE(当前必现) | 🔴 |
ImageArrayBinding.apply |
resource.size() == size 与 > size 两分支(当前逻辑颠倒) |
🔴 |
QueryInstance 立方体 |
从 +x/-x 各方向 + 相机在 AABB 内部观察(samples 是否 >0) | 🔴 |
GpuSampleQueryPool |
连续 3 帧 key 数 > 池容量,验证 expand 与 release 时序 | 🟡 |
ALROptimizations |
启动顺序(create 先于 compute init)下的后端选择断言 | 🟡 |
结论: REQUEST_CHANGES — 基础设施方向正确、多数问题因未接线而处于 latent;但 bindAll 是对既有 API 的确定性回归、x+ 面绕序与 IntSizeUbo 宽高互换是确定的代码缺陷,建议合并前修复;其余
由 Hermes Agent 审查
|
💾 Self-improvement review: Patched SKILL.md in skill 'rendering-gpu-pr-review' (1 replacement). |
Reason: Security scan — [HIGH] Confusable Unicode characters in text: Content contains Unicode characters visually identical to ASCII (math alphanumerics, Cyrillic/Greek lookalikes) appearing near ASCII text, which may indicate a homoglyph attack Reply |
|
|
|
I reached the maximum iterations (90) but couldn't summarize. Error: <title>504 Gateway Time-out</title>504 Gateway Time-outopenresty |
🌿 Roseau API Breaking Change Report
🔴 rendering — 9 breaking change(s)
|
Reason: script execution via -e/-c flag Reply |
Reason: script execution via -e/-c flag Reply |
|
|
|
I reached the maximum iterations (90) but couldn't summarize. Error: <title>504 Gateway Time-out</title>504 Gateway Time-outopenresty |
|
💾 Self-improvement review: Patched SKILL.md in skill 'rendering-gpu-pr-review' (1 replacement). · Patched references/pr95-occlusion-culling-review-matrix.md in skill 'rendering-gpu-pr-review' (1 replacement). · Patched references/class-file-constant-extraction.md in skill 'rendering-gpu-pr-review' (1 replacement). |
🌿 Roseau API Breaking Change Report
|
|
分析完成,输出审查意见。 代码审查摘要 — PR #95操作: synchronize 📋 声称验证表
🔴 关键
|
| 被测目标 | 推荐测试场景 | 优先级 |
|---|---|---|
GpuQueryOcclusionCuller.shouldDraw |
特征上一帧被剔除后本帧是否仍被绘制(回归核心死锁) | 🔴 |
OcclusionSubmitNodeCollection |
wasUsed 重置、feature 集合跨帧增量 |
🔴 |
FrameState.runQueries |
中途异常时 query/instance 是否泄漏、debug group 栈平衡 | 🟡 |
GlSamplesQuery.getValue |
查询未完成时返回语义、closed 后调用抛异常 | 🟡 |
SinglePassDownsampler.spdDispatch |
不同窗口尺寸下 midTex 选择与 mip 数量边界 |
🟡 |
GpuReusableResourcePool |
池满时 fail(true) 扩展、releaseAll/destroyAll 生命周期 |
🟢 |
由 Hermes Agent 审查
|
💾 Self-improvement review: Patched SKILL.md in skill 'rendering-gpu-pr-review' (1 replacement). |
Reason: Security scan — [HIGH] Pipe to interpreter: curl | python3: Command pipes output from 'curl' directly to interpreter 'python3'. Downloaded content will be executed without inspection. Reply |
Reason: Security scan — [HIGH] Pipe to interpreter: curl | python3: Command pipes output from 'curl' directly to interpreter 'python3'. Downloaded content will be executed without inspection. Reply |
🌿 Roseau API Breaking Change Report
🔴 rendering — 31 breaking change(s)
|
Reason: script execution via -e/-c flag Reply |
|
我已掌握全部情况。这是 PR #95 的第 8 轮审查(当前 diff 135 文件/7172 行,与七复规模一致,但新增了 9/1 的两个 commit「implement hiz depth convert」「make it compile」——Hi-Z 后端从骨架变为已接线)。现在输出审查意见。 代码审查摘要 — AnvilLib PR #95(synchronize · 第 8 轮)操作: synchronize ✅ 跨轮次修复确认(本轮核实已修)
🔴 关键(仍需修复)
|
| 声称 | 状态 | 对应文件 |
|---|---|---|
| OcclusionCuller/Key/Method 抽象 | ✅ | optimization/occlusion/* |
| occlusion-query 管线 + shader | ✅ | ALRPipelines.OCCLUSION_QUERY, occlusion_query.{vsh,fsh} |
| GPU device 接口 + GlSamplesQuery | ✅(§1/§2 已修) | ALRGpuDeviceExtension, GlDeviceMixin, GlSamplesQuery |
| 资源池 / ring-buffer 基础 | ✅(fail() 契约 |
GpuReusableResourcePool, QueryInstancePool, GpuSampleQueryPool |
| 帧状态管理(本帧提交、下帧读回) | ✅(fetchResults UB |
FrameState, GpuQueryOcclusionCuller |
debug-group 惰性 Supplier<String> |
✅ | ALRComputePass 等 |
| ApiStatus.Internal 注解调整 | ✅ | mixin 移除 / 内部类添加 |
| [x] Samples Query 后端 | ✅(drawIndexed 越界 🔴) | GpuQueryOcclusionCuller |
| [x] 集成 vanilla SubmitNodeStorage | ✅ 已接线 | TestOcclusionTESR + 12+ feature mixins + OcclusionSubmitNodeStorage/Collection |
| [ ] Hi-Z 后端 | HierarchicalZOcclusionCuller + SinglePassDownsampler:depth convert 已实现,SPD dispatch 是空方法,实际降采样未执行 |
结论: REQUEST_CHANGES
第 8 轮进展显著:8 个历史关键项确认修复(GL query 传参/宽度、UBO 宽高、applyOrdered、checkElementIndex、init 顺序、FFX_SPD atomic、try/finally、cull(false))。但仍有 6 个 🔴 确定性缺陷未修(drawIndexed 6*6 越界、codec 错类、零尺寸 AABB、IntSizeUbo 互换、x+/x- 反绕、spdDispatch 空方法使 Hi-Z 空转)——其中 spdDispatch 空方法 + bindless 死代码是本轮新增 commit 引入的核心接线缺口,建议优先处理:把 culler 接到 spdDispathBindless(或实现 spdDispatch),否则 Hi-Z 后端"已实现"名不副实。
(gh 未认证——token 无效,跳过标题修改;标题「Refactor API status handling and add occlusion culling」仍准确,无需改动。)
由 Hermes Agent 审查
|
💾 Self-improvement review: Patched SKILL.md in skill 'rendering-gpu-pr-review' (1 replacement). · Patched references/pr95-occlusion-culling-review-matrix.md in skill 'rendering-gpu-pr-review' (1 replacement). · Patched SKILL.md in skill 'github-webhook-pr-review' (1 replacement). |
🌿 Roseau API Breaking Change Report
|
Reason: script execution via -e/-c flag Reply |
🌿 Roseau API Breaking Change Report
🔴 rendering — 36 breaking change(s)
|
Reason: Security scan — [MEDIUM] Variation selector characters detected: Content contains Unicode variation selectors (VS1-256). These are commonly used in emoji sequences but may indicate steganographic encoding or obfuscation Reply |
Reason: Security scan — [MEDIUM] Variation selector characters detected: Content contains Unicode variation selectors (VS1-256). These are commonly used in emoji sequences but may indicate steganographic encoding or obfuscation Reply |
|
|
代码审查摘要 — PR #95(九复 synchronize)操作: synchronize 本轮相较八复的关键变化:新增 bindless texture 基础设施( 🔴 关键(需修复后合并)
|
|
💾 Self-improvement review: Patched SKILL.md in skill 'rendering-gpu-pr-review' (1 replacement). · Patched references/pr95-occlusion-culling-review-matrix.md in skill 'rendering-gpu-pr-review' (1 replacement). |
Reason: Security scan — [MEDIUM] Variation selector characters detected: Content contains Unicode variation selectors (VS1-256). These are commonly used in emoji sequences but may indicate steganographic encoding or obfuscation Reply |
|
|
PR #95 复审查意见(synchronize 九复)审查范围:143 文件 / 7625 行 diff;commits API 显示 09-01 新增 4 个提交(implement hiz depth convert / make it compile / bindless texture / make it compile x2),相对八复新增 bindless 纹理系列文件。gh 未认证(token invalid),标题未修改。 🔴 高优先级(已接线,每帧实际执行)1. 2. 3. 4.
|
|
💾 Self-improvement review: Patched SKILL.md in skill 'rendering-gpu-pr-review' (1 replacement). · Patched references/pr95-occlusion-culling-review-matrix.md in skill 'rendering-gpu-pr-review' (1 replacement). |
Summary
This PR lays the first-stage foundation for occlusion culling in the rendering module and clarifies the visibility of selected internal APIs. It introduces the culler abstraction, GPU Query resources and draw path, and an OpenGL samples-query wrapper. The selectable Hi-Z and Query culling backends will be integrated in follow-up work.
Changes
OcclusionCuller,OcclusionKey, andOcclusionMethodto provide a common abstraction for rendering-feature submission, visibility decisions, and backend selection.FullTransformsUbo.GlSamplesQuerylifecycle wrapper.Supplier<String>values, avoiding unnecessary debug-text construction.ApiStatus.Internalannotations by removing them from mixin classes and explicitly marking rendering and sync classes, records, and extension interfaces that are internal implementations.Impact and Compatibility
ALRComputePass.pushDebugGroupand its backend implementation now acceptSupplier<String>instead ofString; direct implementations and callers of this internal API must be updated accordingly.TODO