Skip to content

fix(rendering): support Iris shader vertex formats#76

Merged
Gu-ZT merged 2 commits into
Anvil-Dev:dev/26.1from
ZhuRuoLing:feat/iris_shader_compat
Jul 12, 2026
Merged

fix(rendering): support Iris shader vertex formats#76
Gu-ZT merged 2 commits into
Anvil-Dev:dev/26.1from
ZhuRuoLing:feat/iris_shader_compat

Conversation

@ZhuRuoLing

@ZhuRuoLing ZhuRuoLing commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • preserve and configure Iris immediate rendering state while rebuilding cached block entity meshes
  • upload cached meshes using the vertex format and primitive mode selected by the active BufferBuilder
  • only query GL_MAX_LABEL_LENGTH when GL_KHR_debug is available

Testing

  • .\gradlew.bat test --console=plain
  • git diff --check upstream/dev/26.1...HEAD

@Gugle2308

Copy link
Copy Markdown

现在我已掌握完整的变更情况,以下是审查结果。


代码审查摘要

结论:评论(1 个警告,3 条建议)

PR #76feat/iris_shader_compatdev/26.1
范围: 18 个文件,+171 / -38 行


⚠️ 警告

1. module.test 中新增了未使用的 import

TestCachedRenderer.java 新增了 2 个 import(SheetsRenderTypes),但 diff 中没有对应的方法体变更来使用它们:

+import net.minecraft.client.renderer.Sheets;
+import net.minecraft.client.renderer.rendertype.RenderTypes;

如果已有代码确实引用了这些类,说明旧代码通过其他机制(如 star import 或继承)已经可访问,那么显式 import 加上原本的隐式 import 会导致「ambiguous reference」冲突。如果已有代码并未引用它们,则是死 import。

建议: 确认这些 import 的实际用途。如果不需要,请移除;如果需要,请在对应的代码路径中添加调用。


💡 建议

2. RebuildTaskMixinImmediateState.isRenderingLevel 的直接字段赋值

ImmediateState.isRenderingLevel = true;
ImmediateState.skipExtension.set(false);

handleBegin 方法在 run()@At("HEAD") 注入点直接将 Iris 的内部状态字段设置为硬编码值。这有一个风险:如果在 pushIrisGlobalState() 调用和这两个赋值之间发生异常,会导致 Iris 状态被修改但未压栈,后续 handleEnd 中的 popIrisGlobalState() 恢复的是一个无关的状态。

建议: 考虑将赋值移到 pushIrisGlobalState() 方法内部,或将这两个赋值与 push 合并为原子操作,确保异常安全。例如:

void handleBegin(CallbackInfo ci) {
    IrisSupport.pushIrisGlobalState();  // ← 在这里面就设置好 isRenderingLevel 和 skipExtension
}

或者使用 try { ... } finally { ... } 包装。不过由于 Mixin 注入需要保持签名简洁,更好的方案是将 push + set 合并在 _pushIrisGlobalState() 内。

3. EOF 换行符缺失(8 处)

以下文件缺少末尾换行符:

文件 说明
module.explosion/gradle.properties 新增内容末尾
module.font/gradle.properties 新增内容末尾
module.main/gradle.properties 新增内容末尾
module.multiblock/gradle.properties 新增内容末尾
module.registrum/gradle.properties 新增内容末尾
module.rendering/.../accesstransformer.cfg 新增 AT 条目后(旧文件已有此问题)
module.test/.../accesstransformer.cfg 新文件(仅 1 行内容)

gradle.propertiesaccesstransformer.cfg 虽然是 POSIX 严格性要求较低的场景,但保持一致性总是好的。尤其是 module.test/.../accesstransformer.cfg新文件,理应包含末尾换行符。

4. FullyBufferedBufferSourcerenderTypebufferBuilder 字段访问的 Null 安全

-int compiledVertices = bufferBuilder.vertices * renderType.format().getVertexSize();
+int compiledVertices = bufferBuilder.vertices * bufferBuilder.format.getVertexSize();

新代码改为直接从 bufferBuilder.format 读取。这是正确的——上传时应使用 BufferBuilder 当前的顶点格式(Iris 可能已改写)。但需要确认 bufferBuilder.format 在任何路径中都不会为 null。BufferBuilder 初始化后 format 字段可能是 null 直到 begin() 被调用。不过从 bufferBuilders.get(renderType) 取出的 builder 应该已经被 begin() 过了——建议确认调用路径保证此不变式。


✅ 看起来不错

  • IrisSupport 状态栈设计清晰Stack<IrisState> + push/pop 模式正确,保存和恢复 isRenderingLevelskipExtension 两个字段,保证了 RebuildTask.run() 对 Iris 状态的修改不会泄漏到后续帧。
  • RebuildTask 改为 public:必要的可见性提升——Mixin 目标类需要可访问。✅
  • 条件式 run config 重构module.gradle 中把硬编码的 run config 提取为 anvillib.needRunConfig.* 属性驱动模式,是合理的架构改进。新增的 clientIrisWorkaroundclientRenderDocIrisWorkaround 跑配置添加了 neoforge.disableGlValidation=true(Iris 兼容的常见需求),方向正确。
  • CachedBlockEntityRenderingPipelineMixin 保持注册:旧的 Iris mixin 未被删除或破坏。
  • AT 条目最小化:只暴露了 BufferBuilder.formatBufferBuilder.mode 两个字段,没有过度暴露 API。
  • Mixin 插件的条件注册正确ALRIntegrationCompatMixinPluginRebuildTaskMixin 被正确标记为 isPresent("iris") 条件,不会在无 Iris 时加载。
  • 测试用的 AT 文件module.test 新增 AT 暴露 BlockModelRenderState.renderType,是正确的测试基础设施。

关于 scope 纯净度

PR 标题为 fix(rendering): support Iris shader vertex formats,但包含了较大的 module.gradle 重构(run config 条件化)和 7 个模块的 gradle.properties 变更。这些是安全的基础设施变更——module.gradle 的重构直接支撑了 clientIrisWorkaroundclientRenderDocIrisWorkaround 两个新 run config 的添加,而 gradle.properties 只是为性质变更提供属性开关。不属于 scope 污染。


总体评价: 结构清晰,改动聚焦,无严重逻辑错误。建议处理 EOF 换行符和 import 废弃问题后合并。

@Gugle2308

Copy link
Copy Markdown

🌿 Roseau API Breaking Change Report

Module Status Breaking Changes
codec ✅ Compatible 0
collision ✅ Compatible 0
config ✅ Compatible 0
integration ✅ Compatible 0
moveable-entity-block ✅ Compatible 0
network ✅ Compatible 0
rendering ✅ Compatible 0
space-select ✅ Compatible 0
font ✅ Compatible 0
util ✅ Compatible 0
explosion ✅ Compatible 0
rpc ✅ Compatible 0
multiblock ✅ Compatible 0
recipe ✅ Compatible 0
registrum ✅ Compatible 0
sync ✅ Compatible 0
wheel ✅ Compatible 0
main ✅ Compatible 0

Full CSVs: see the Artifacts section of this workflow run.

@Gugle2308

Copy link
Copy Markdown

⚠️ Dangerous command requires approval:

echo "=== EOF 换行符缺失统计 ===" && grep -c 'No newline at end of file' /tmp/pr76.diff && echo "" && echo "=== 缺失文件列表 ===" && awk '/^diff --git/{f=$NF} /No newline/{print f}' /tmp/pr76.diff && echo "" && ec...

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 /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown

以下是 PR #76 的代码审查报告。


代码审查摘要

结论:评论(1 个问题,1 条建议,无阻塞性严重问题)

🔴 关键

无。

⚠️ 警告

  • IrisSupport.java:215,243,252IrisState 静态 Stack 在多线程环境下存在竞态条件
    • RebuildTaskForkJoinPool 工作线程上执行,多个构建任务可能同时运行
    • 静态 Stack<IrisState> 是所有线程共享的,而非 ThreadLocal
    • 当线程 A push → 线程 B push → 线程 A pop 时,A 拿到的不是自己压入的状态,导致恢复时写错 ImmediateState.isRenderingLevel
    • ImmediateState.isRenderingLevel 本身也是 static boolean(非常量语义),多线程同时写入存在 data race
    • 建议修复:将 irisStateStack 改为 ThreadLocal<Stack<IrisState>>,同时确认 ImmediateState.isRenderingLevel 的读取是否在渲染线程还是 worker 线程上——若是前者,worker 线程上的写入本身可能没有实际效果

💡 建议

  • 多个 .properties 文件和 AT 文件缺失末尾换行符(共 8 处):

    • module.explosion/gradle.properties
    • module.font/gradle.properties
    • module.main/gradle.properties
    • module.multiblock/gradle.properties
    • module.registrum/gradle.properties
    • module.wheel/gradle.properties
    • module.rendering/src/main/resources/META-INF/accesstransformer.cfg
    • module.test/src/main/resources/META-INF/accesstransformer.cfg
    • 虽然 .properties 和 AT 文件通常不受末行换行缺失影响(不同于 POSIX 文本文件),但统一补充有助于避免 diff --check 报错,且保持仓库整洁
  • IrisSupport.java:252irisStateStack.peek() 在空栈上抛出 EmptyStackException,其后 if (peek != null) 检查不会生效(peek() 不会返回 null):

    IrisState peek = irisStateStack.peek();
    if (peek != null) {  // ← 多余的 null 检查,Stack.peek() 在空栈时抛异常而非返回 null
        irisStateStack.pop();
        ...
    }

    建议要么在 push/pop 数量绝对匹配的前提下直接 pop(信任 mixin 配对),要么加 if (!irisStateStack.isEmpty()) 做防火检查

✅ 看起来不错

  • FullyBufferedBufferSource.uploadNow() 核心改动正确:从 BufferBuilder.format / BufferBuilder.mode 而非 RenderType.format() / RenderType.mode() 读取顶点格式和绘制模式——这正是 Iris 兼容性的关键。Iris 在内部包装 RenderType 并拦截其 format() 返回扩展顶点格式(含法线、中纹理坐标等),直接读取 BufferBuilder 的字段能获得 Iris 实际配置的格式,而非虚报的原始 format
  • RebuildTaskMixin HEAD/RETURN 成对注入:push/pop 保证异常路径下也会恢复状态
  • Mixin 条件加载正确:通过 ALRIntegrationCompatMixinPlugin.isPresent("iris") 控制,无 Iris 时完全不加载
  • AT 文件添加完整BufferBuilder formatBufferBuilder mode 字段暴露正确,与代码改动对应
  • 构建配置重构(module.gradle):将硬编码的运行配置改为条件启用的模式合理——needRunConfig.* 属性在各模块的 gradle.properties 中声明,减少了无渲染需求的模块加载过多运行配置;新增 clientIrisWorkaroundclientRenderDocIrisWorkaround 两种 Iris 专用运行配置,便于兼容性测试
  • 模块 scope 纯净:仅 rendering 模块有实质代码变更,test 模块仅更新依赖版本和添加测试所需的 AT/import,其余模块仅新增构建属性——PR 范围集中,无无关变更

核心逻辑总结:PR 通过在 RebuildTask.run() 执行期间注入 Iris 的 ImmediateState 配置,并在 FullyBufferedBufferSource 中改用 BufferBuilder 的实际格式/模式数据,解决了 Iris 着色器启用时缓存 BE 网格使用错误顶点格式导致渲染异常的问题。主要遗留问题在线程安全性——当前 IrisState stack 和 ImmediateState.isRenderingLevel 在多 RebuildTask 并发时不可靠。

@Gugle2308

Copy link
Copy Markdown

现在我已经全面了解了改动范围,以下是审查意见。


代码审查摘要:PR #76 — Iris shader vertex format 兼容性

结论:评论(无阻塞性问题,以下为可选的改进建议)


📋 文件概览

类型 计数
总文件 19
新增文件 2(RebuildTaskMixin.java, test AT cfg
Java 源文件 7
Gradle/构建配置 7
Mixin/AT 配置 3
Gradle properties 7(全部新增 anvillib.needRunConfig

🟢 主要改动

  1. IrisSupport.java (+41/-0) — 新增基于 Stack<IrisState> 的 Iris 即时渲染状态压栈/出栈机制。在 RebuildTask.run() 执行期间保存并恢复 ImmediateState.isRenderingLevelImmediateState.skipExtension

  2. RebuildTaskMixin.java(新文件,+31)RebuildTask.run()@HEAD 处压入 Iris 状态并设置 isRenderingLevel=true + skipExtension=false@RETURN 处恢复。

  3. FullyBufferedBufferSource.java — 核心修复:上传网格时使用 bufferBuilder.format.getVertexSize()bufferBuilder.mode.indexCount(...) 替代 renderType.format() / renderType.mode()。Iris 可能会覆盖这些值,从 BufferBuilder 直接读取保证使用 Iris 的活跃顶点格式和基元模式。

  4. CachedBlockEntityRenderingPipeline.create() — 新增 GL.getCapabilities().GL_KHR_debug 守卫,防止 GL_KHR_debug 扩展不可用时崩溃(Iris 下常见)。

  5. module.gradle — 将运行配置改为按 anvillib.needRunConfig.* 属性条件启用,新增 clientIrisWorkaroundclientRenderDocIrisWorkaround 运行配置(带 neoforge.disableGlValidation=true)。

  6. AT 条目BufferBuilder.formatBufferBuilder.mode(渲染模块)和 BlockModelRenderState.renderType(测试模块)。

  7. RebuildTaskpublic class — Mixin 需要类可访问。


⚠️ 警告

  • 8 个文件缺少末尾换行符(No newline at end of file — 包括 accesstransformer.cfg(渲染模块+测试模块)和 6 个 gradle.properties。这违反 POSIX 标准,可能导致某些工具(diffpatchwc -l 等)行为异常。建议修复所有此类文件。

    # 定位具体文件
    awk '/^diff --git/{f=$NF} /No newline/{print f}' /tmp/pr76.diff

    受影响文件:

    • module.rendering/src/main/resources/META-INF/accesstransformer.cfg
    • module.test/src/main/resources/META-INF/accesstransformer.cfg(新文件)
    • 以及 6 个 gradle.propertiesexplosionfontmainmultiblockregistrumwheel

💡 建议

  1. IrisSupport:Stack → ArrayDequejava.util.Stack 是遗留类(同步开销,基于 Vector)。渲染线程是单线程的,使用 ArrayDeque 更合适:

    private static final Deque<IrisState> irisStateStack = new ArrayDeque<>();
    // push → irisStateStack.push()
    // pop → irisStateStack.pop() / isEmpty() guard
  2. _popIrisGlobalState()peek() 空检查无效Stack.peek() 对于空栈会抛出 EmptyStackException,不会返回 null。应改为显式的 isEmpty() 守卫:

    private static void _popIrisGlobalState() {
        if (!irisStateStack.isEmpty()) {
            IrisState state = irisStateStack.pop();
            ImmediateState.isRenderingLevel = state.isRenderingLevel;
            ImmediateState.skipExtension.set(state.skipExtension);
        }
    }

    当前代码中的 peek != null 检查是多余的(不可能为 null),而 EmptyStackException 仍可能因并发或不平衡的 push/pop 抛出。

  3. RebuildTaskMixin@At("RETURN") 不捕获异常 — 如果 RebuildTask.run() 抛出未检查异常(如 NullPointerException),handleEnd 不会执行,导致 Iris 状态栈永远不平衡(push 而无 pop)。考虑使用 @At("TAIL") 替代 @At("RETURN") 来捕获正常返回路径,或添加 try/finally 包围 run() 主体:

    // 或者更健壮的方式:Inject 整个方法体
    @Inject(method = "run", at = @At("HEAD"))
    void handleBegin(CallbackInfo ci) { ... }
    
    @Inject(method = "run", at = @At("RETURN"))
    void handleEnd(CallbackInfo ci) { ... }
    
    // 添加上面的 @At("RETURN") 已经足够,考虑增加 throws 保护

    实际上,更健壮的做法是将 RebuildTask.run() 包装为 try/finally,但当前 mixin 模式在 stable 场景下可接受——仅建议。

  4. 渲染模块未启用 anvillib.needRunConfigmodule.rendering/gradle.properties 未获得 anvillib.needRunConfig=true。这可能是设计意图(渲染是库模块,无需直接运行配置),但若未来需要 Iris workaround 的调试运行配置,需补充。


✅ 看起来不错

  • 架构设计合理:Stack 式的状态保存/恢复与 RebuildTask 的生命周期绑定(run 开始时保存、结束时恢复),模式清晰。
  • Iris 存在性守卫:两层守卫——ALRIntegrationCompatMixinPlugin 的条件 mixin 注册 + IrisSupport 中的 IRIS_PRESENT 运行时检查,防止 ClassNotFoundException。
  • FullyBufferedBufferSource 的改动精准:直接读取 BufferBuilder 字段而非 RenderType 属性,这正是 Iris 兼容所需——Iris 在 BufferBuilder 上设置自定义格式和模式,从源头读取保证正确。
  • 条件性运行配置设计良好:通过 anvillib.needRunConfig.* 属性控制,避免所有模块都生成不必要的 IDE 运行配置。
  • Iris Workaround 运行配置disableGlValidation=true 正确应对 Iris 自定义 GL 操作导致的标准验证失败。
  • GL_KHR_debug 守卫:防止缺少 GL 调试扩展时崩溃,是一种务实的容错。

由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown

Raw diff analysis is complete. Here is my review:


代码审查摘要 — PR #76

结论:请求更改(2 个需关注项,2 条建议)

🔴 关键

  • IrisSupport.java:50Stack.peek() 空值检查为死代码,异常时不恢复状态
    _popIrisGlobalState()irisStateStack.peek()if (peek != null) 分支永远不会触发——java.util.Stack.peek() 在空栈时抛出 EmptyStackException,永远不会返回 null。更严重的问题是:RebuildTaskMixin 使用 @At("RETURN") 注入 handleEndRebuildTask.run() 中途抛出异常,popIrisGlobalState() 不会被调用,导致 Iris 全局状态永久污染——后续渲染使用错误的 isRenderingLevel/skipExtension 值。

    • 建议:将 Stack.peek() 改为 !irisStateStack.isEmpty() 检查;或在 RebuildTask 原方法内用 try/catch 包裹(见建议 2 的替代方案)。
  • IrisSupport.java:16 — 静态 java.util.Stack<> 线程不安全
    IrisSupport.irisStateStackjava.util.Stack(非线程安全)。如果 CachedBlockEntityRenderingPipelineForkJoinPool 或工作线程池上并行执行 RebuildTask,多个线程同时 push/pop 静态栈会导致竞态——状态栈损毁。即使当前实现串行处理,随着未来多区块并行重建优化,这个静态共享状态会成为隐患。

    • 建议:改为 Deque + synchronized 包装,或使用 ThreadLocal<Deque<IrisState>> 实现线程隔离。
  • RebuildTaskMixin.java:23@At("RETURN") 无法处理异常路径保护
    Mixin 的 @At("RETURN") 仅在方法正常返回时触发,不覆盖异常传播路径。若 RebuildTask.run() 执行过程中抛出任何异常,IrisSupport.popIrisGlobalState() 不被调用。替代方案:

    • RebuildTask.run() 主体包装 try/finally(需手动重构原方法,或使用 @Inject + @ModifyVariable 等更复杂的 Mixin 模式)
    • 或在 IrisSupport 中添加当前帧 ID 校验,异常时自动重置

⚠️ 警告

  • RebuildTaskMixin.java handleEnd 定义:缺少 CallbackInfo ci 参数前的空格

    void handleEnd(CallbackInfo ci){

    Mixin 编码规范通常保留 ci{ 之间的空格(CallbackInfo ci) {)。当前写法在功能上无问题,但与项目中其他 Mixin 风格不一致。

  • module.test/src/main/resources/META-INF/accesstransformer.cfg — 缺少尾随换行符
    新文件末尾 \ No newline at end of file。虽不影响 AT 解析器功能,但与 POSIX 文本文件规范不符,且在 diff 中产生噪声。

  • TestCachedRenderer.java 新增 import 可能未使用
    SheetsRenderTypes 两个 import 被添加,但 diff 中无对应使用这些类型的新代码。如果确实未被引用,建议删除以避免编译期/检查工具警告。

💡 建议

  1. module.gradle 嵌套条件层次深if (anvillib.needRunConfig)if (client)if (clientRenderDoc)if (clientRenderDocIrisWorkaround) 嵌套 4 层。建议将各 run config 提取为独立 if 块或 Groovy 方法,提高可维护性。

  2. FullyBufferedSource 读 BufferBuilder 字段的正确性 — ✅ 核心修复逻辑正确。当 Iris 激活时,RenderType.format() 返回的可能是默认格式,而 BufferBuilder 中实际使用的格式已被 Iris 修改。直接读取 bufferBuilder.formatbufferBuilder.mode(通过 AT 暴露)确保上传的缓存网格使用 Iris 当前选中的顶点格式和渲染模式。

  3. GL_KHR_debug 守卫 — ✅ CachedBlockEntityRenderingPipeline.create() 中对 glGetInteger(GL_MAX_LABEL_LENGTH) 的守卫正确修复。Iris 某些驱动/配置下不暴露该扩展,原来的无条件调用会导致 GL_INVALID_ENUM 错误。

✅ 看起来不错

  • Iris 状态栈设计合理pushIrisGlobalState() 保存 isRenderingLevel + skipExtensionpopIrisGlobalState() 干净恢复。保存/恢复模式正确。
  • RebuildTask 改为 public — 必要的可见性提升,使 RebuildTaskMixin 可以 target 该类,且不会影响封装性(类本身无敏感状态)。
  • ALRIntegrationCompatMixinPlugin 条件注册RebuildTaskMixin 仅在 Iris/Oculus 存在时注册,避免非 Iris 环境下的无意义加载。
  • AT 条目精准 — 仅暴露 BufferBuilder.formatBufferBuilder.mode 两个必要字段,最小权限原则执行正确。
  • Gradle run config 改为按需启用anvillib.needRunConfig.* 标志系统减少了非测试模块的配置污染,新增 clientIrisWorkaroundclientRenderDocIrisWorkaround 为开发者提供 Iris 兼容调试路径。

整体评估: 核心修复方向正确——通过 Mixin 在缓存网格重建时保存/恢复 Iris ImmediateState、直接从 BufferBuilder 读取实际顶点格式替代 RenderType 的默认格式。主要风险点在异常路径下 Iris 状态不恢复(关键)和静态栈线程安全性(潜在),建议在合并前修复。

@Gugle2308

Copy link
Copy Markdown

🌿 Roseau API Breaking Change Report

Module Status Breaking Changes
codec ✅ Compatible 0
collision ✅ Compatible 0
config ✅ Compatible 0
integration ✅ Compatible 0
moveable-entity-block ✅ Compatible 0
network ✅ Compatible 0
rendering ✅ Compatible 0
space-select ✅ Compatible 0
font ✅ Compatible 0
util ✅ Compatible 0
explosion ✅ Compatible 0
rpc ✅ Compatible 0
multiblock ✅ Compatible 0
recipe ✅ Compatible 0
registrum ✅ Compatible 0
sync ✅ Compatible 0
wheel ✅ Compatible 0
main ✅ Compatible 0

Full CSVs: see the Artifacts section of this workflow run.

@Gu-ZT
Gu-ZT merged commit fba1027 into Anvil-Dev:dev/26.1 Jul 12, 2026
38 checks passed
@Gugle2308

Copy link
Copy Markdown

停止审查

PR #76closed(已关闭)。根据审查流程的步骤 0:

"closed" / "labeled"Stop, don't post a comment

因此我不会执行审查分析,也不会在 PR 上发布任何评论。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants