feat(drawlist): add BLIT_RECT opcode with overlap-safe execution#102
Merged
RtlZeroMemory merged 5 commits intomainfrom Feb 26, 2026
Merged
feat(drawlist): add BLIT_RECT opcode with overlap-safe execution#102RtlZeroMemory merged 5 commits intomainfrom
RtlZeroMemory merged 5 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds Drawlist v2 support with a new opcode ZR_DL_OP_BLIT_RECT (14) and payload zr_dl_cmd_blit_rect_t; extends header/version negotiation to accept V1 or V2; implements deserialization, validation, bounds-checking, preflight, and execution for BLIT_RECT; updates docs, headers, config, tooling, and tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Reader as zr_dl_read_cmd_blit_rect
participant Validator as zr_dl_validate_cmd_blit_rect
participant Preflight as zr_dl_preflight_resources
participant Executor as zr_dl_exec_blit_rect
participant FB as Framebuffer
Client->>Reader: Submit drawlist (header v1 or v2) with BLIT_RECT opcode
Reader->>Validator: Parse opcode + payload (src_x,src_y,w,h,dst_x,dst_y)
Validator->>Preflight: Validate payload size and semantics
Preflight->>FB: Check framebuffer bounds & resources
alt Bounds/resources valid
Preflight->>Executor: Approve command
Executor->>FB: Copy rectangle (src -> dst), preserve metadata/styles
FB-->>Client: Blit applied
else Invalid
Preflight-->>Client: Return ZR_ERR_INVALID_ARGUMENT (no partial effects)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
RtlZeroMemory
force-pushed
the
feat/drawlist-blit-rect
branch
from
e7e780d to
1ac705a
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7e780dbd1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/user-guide/rendering-model.md`:
- Around line 39-40: Update the user-facing sentence that mentions
ZR_DL_OP_BLIT_RECT so the phrase "scroll/move style updates" is hyphenated
correctly as "scroll/move-style updates"; locate the paragraph containing the
symbol ZR_DL_OP_BLIT_RECT and replace the unhyphenated phrase with the
hyphenated version.
In `@src/core/zr_drawlist.c`:
- Around line 537-567: The file fails the project's formatter; run the
repository's formatting tool on src/core/zr_drawlist.c (or run the global
formatter flow) and fix style issues in the zr_dl_read_cmd_blit_rect function
and surrounding code so it matches the project's clang-format settings; ensure
whitespace, indentation, and brace placement conform to the codebase formatter
and re-run the formatter check before pushing.
In `@tests/unit/test_drawlist_blit_rect.c`:
- Around line 1-7: The new test file tests/unit/test_drawlist_blit_rect.c fails
CI due to clang-format differences; run the project's formatter on that file
(e.g., clang-format -i or the repo's format script / pre-commit hook) and commit
the updated formatting so the file matches the repository style; ensure the
header comment and any function declarations in test_drawlist_blit_rect.c are
reformatted to the project's clang-format rules before pushing.
In `@tests/unit/test_engine_submit_drawlist_no_partial_effects.c`:
- Around line 299-313: The baseline capture is taken from a blank frame so a
failing drawlist that begins with ZR_DL_OP_CLEAR can hide partial-execution
bugs; instead, make the baseline capture come from a non-blank frame (e.g.,
submit a known-good drawlist that paints pixels or otherwise mutates the frame
using engine_submit_drawlist before calling zr_capture_present_bytes for
a_bytes) or alter the test input so bad_blit does not start with ZR_DL_OP_CLEAR;
update the setup around engine_create/ zr_capture_present_bytes (references:
engine_create, engine_submit_drawlist, zr_capture_present_bytes, bad_blit,
a_bytes) to ensure the baseline is non-blank before asserting no partial
effects.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/abi/c-abi-reference.mddocs/abi/drawlist-format.mddocs/user-guide/rendering-model.mdinclude/zr/zr_drawlist.hsrc/core/zr_drawlist.ctests/CMakeLists.txttests/unit/test_drawlist_blit_rect.ctests/unit/test_engine_submit_drawlist_no_partial_effects.c
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/unit/test_engine_version_negotiation.c (1)
45-53: Avoid hardcoded unsupported version literals in negotiation tests.Using
3u/4u/5uties the test to today’s max version. Please derive cases fromZR_DRAWLIST_VERSION_V2(and add Arrange/Act/Assert markers) so the test stays stable as new versions are introduced.As per coding guidelines, “Replace magic thresholds and protocol constants with named constants in local enums/macros” and “Add section markers … in tests (/* --- Arrange/Act/Assert --- */ …)”.♻️ Suggested refactor
ZR_TEST_UNIT(engine_config_rejects_drawlist_versions_above_v2) { + /* --- Arrange --- */ zr_engine_config_t cfg = zr_engine_config_default(); - cfg.requested_drawlist_version = 3u; - ZR_ASSERT_EQ_U32(zr_engine_config_validate(&cfg), ZR_ERR_UNSUPPORTED); - cfg.requested_drawlist_version = 4u; - ZR_ASSERT_EQ_U32(zr_engine_config_validate(&cfg), ZR_ERR_UNSUPPORTED); - cfg.requested_drawlist_version = 5u; - ZR_ASSERT_EQ_U32(zr_engine_config_validate(&cfg), ZR_ERR_UNSUPPORTED); + const uint32_t unsupported_versions[] = { + ZR_DRAWLIST_VERSION_V2 + 1u, + ZR_DRAWLIST_VERSION_V2 + 2u, + ZR_DRAWLIST_VERSION_V2 + 3u, + }; + + /* --- Act/Assert --- */ + for (size_t i = 0u; i < (sizeof(unsupported_versions) / sizeof(unsupported_versions[0])); ++i) { + cfg.requested_drawlist_version = unsupported_versions[i]; + ZR_ASSERT_EQ_U32(zr_engine_config_validate(&cfg), ZR_ERR_UNSUPPORTED); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_engine_version_negotiation.c` around lines 45 - 53, The test ZR_TEST_UNIT(engine_config_rejects_drawlist_versions_above_v2) uses hardcoded literals 3u/4u/5u; replace those with values computed from ZR_DRAWLIST_VERSION_V2 (e.g. ZR_DRAWLIST_VERSION_V2 + 1, +2, +3) when assigning cfg.requested_drawlist_version and keep the same assertions calling zr_engine_config_validate(&cfg) expecting ZR_ERR_UNSUPPORTED; also add clear /* --- Arrange --- */, /* --- Act --- */, /* --- Assert --- */ section comments around setup, the version assignments/validation calls, and the ZR_ASSERT_EQ_U32 checks so the test follows the Arrange/Act/Assert guideline.docs/VERSION_PINS.md (1)
26-27: Clarify that v1 remains the baseline protocol.This reads correctly, but adding one explicit clause like “v1 is the baseline; v2 is additive for
BLIT_RECT” would prevent version-line confusion in wrapper docs.Based on learnings, “the current drawlist protocol is designated as v1 (baseline), even if it internally implements ZRDL v6 features.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/VERSION_PINS.md` around lines 26 - 27, Clarify the baseline protocol by adding a short explicit sentence stating that drawlist version v1 is the baseline protocol and v2 is additive only for the BLIT_RECT opcode; reference ZR_DL_OP_BLIT_RECT and note that v1 remains the baseline even though it may internally implement ZRDL v6 features so readers won't confuse v2 as a replacement—i.e., add something like “v1 is the baseline; v2 only adds support for ZR_DL_OP_BLIT_RECT (BLIT_RECT).”
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/VERSION_PINS.md`:
- Around line 26-27: Clarify the baseline protocol by adding a short explicit
sentence stating that drawlist version v1 is the baseline protocol and v2 is
additive only for the BLIT_RECT opcode; reference ZR_DL_OP_BLIT_RECT and note
that v1 remains the baseline even though it may internally implement ZRDL v6
features so readers won't confuse v2 as a replacement—i.e., add something like
“v1 is the baseline; v2 only adds support for ZR_DL_OP_BLIT_RECT (BLIT_RECT).”
In `@tests/unit/test_engine_version_negotiation.c`:
- Around line 45-53: The test
ZR_TEST_UNIT(engine_config_rejects_drawlist_versions_above_v2) uses hardcoded
literals 3u/4u/5u; replace those with values computed from
ZR_DRAWLIST_VERSION_V2 (e.g. ZR_DRAWLIST_VERSION_V2 + 1, +2, +3) when assigning
cfg.requested_drawlist_version and keep the same assertions calling
zr_engine_config_validate(&cfg) expecting ZR_ERR_UNSUPPORTED; also add clear /*
--- Arrange --- */, /* --- Act --- */, /* --- Assert --- */ section comments
around setup, the version assignments/validation calls, and the ZR_ASSERT_EQ_U32
checks so the test follows the Arrange/Act/Assert guideline.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
docs/ABI_REFERENCE.mddocs/VERSION_PINS.mddocs/abi/c-abi-reference.mddocs/abi/drawlist-format.mddocs/abi/versioning.mddocs/user-guide/rendering-model.mdinclude/zr/zr_drawlist.hinclude/zr/zr_version.hscripts/check_version_pins.pysrc/core/zr_config.csrc/core/zr_drawlist.ctests/unit/test_drawlist_blit_rect.ctests/unit/test_engine_submit_drawlist_no_partial_effects.ctests/unit/test_engine_version_negotiation.ctests/unit/test_public_abi_headers.c
✅ Files skipped from review due to trivial changes (1)
- docs/ABI_REFERENCE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/user-guide/rendering-model.md
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/test_drawlist_canvas.c (1)
322-327: Replace hard-coded drawlist versions with named constants.Line 322, Line 324, and Line 326 use bare protocol version literals (
2u,3u,4u). Please use named constants/local enum aliases so version-boundary intent stays explicit and easier to maintain.♻️ Proposed refactor
ZR_TEST_UNIT(drawlist_canvas_versions_above_v2_rejected_as_unsupported) { + enum { + ZR_TEST_SUPPORTED_VERSION = ZR_DRAWLIST_VERSION_V2, + ZR_TEST_UNSUPPORTED_VERSION_1 = ZR_DRAWLIST_VERSION_V2 + 1u, + ZR_TEST_UNSUPPORTED_VERSION_2 = ZR_DRAWLIST_VERSION_V2 + 2u, + }; uint8_t blob[4] = {1u, 2u, 3u, 255u}; uint8_t bytes[160]; zr_limits_t lim = zr_limits_default(); zr_dl_view_t v; zr_dl_cmd_draw_canvas_t cmd = {0, 0, 1, 1, 1, 1, 1u, 0u, (uint8_t)ZR_BLIT_ASCII, 0u, 0u}; - size_t len = zr_make_canvas_drawlist(bytes, 2u, &cmd, blob, 4u, 0u); + size_t len = zr_make_canvas_drawlist(bytes, ZR_TEST_SUPPORTED_VERSION, &cmd, blob, 4u, 0u); ZR_ASSERT_EQ_U32(zr_dl_validate(bytes, len, &lim, &v), ZR_OK); - len = zr_make_canvas_drawlist(bytes, 3u, &cmd, blob, 4u, 0u); + len = zr_make_canvas_drawlist(bytes, ZR_TEST_UNSUPPORTED_VERSION_1, &cmd, blob, 4u, 0u); ZR_ASSERT_EQ_U32(zr_dl_validate(bytes, len, &lim, &v), ZR_ERR_UNSUPPORTED); - len = zr_make_canvas_drawlist(bytes, 4u, &cmd, blob, 4u, 0u); + len = zr_make_canvas_drawlist(bytes, ZR_TEST_UNSUPPORTED_VERSION_2, &cmd, blob, 4u, 0u); ZR_ASSERT_EQ_U32(zr_dl_validate(bytes, len, &lim, &v), ZR_ERR_UNSUPPORTED); }As per coding guidelines: "Replace magic thresholds and protocol constants with named constants in local enums/macros."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_drawlist_canvas.c` around lines 322 - 327, Replace the magic version literals passed to zr_make_canvas_drawlist (currently 2u, 3u, 4u) with descriptive named constants or a small local enum (e.g., DRAWLIST_VER_SUPPORTED, DRAWLIST_VER_UNSUPPORTED_LOW, DRAWLIST_VER_UNSUPPORTED_HIGH) so the intent of testing version boundaries is explicit; update the three calls that pass the version argument and any related assertions to use these constants (refer to zr_make_canvas_drawlist and zr_dl_validate in the failing test) and add the constants near the top of the test file to keep them local and maintainable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/test_drawlist_canvas.c`:
- Around line 322-327: Replace the magic version literals passed to
zr_make_canvas_drawlist (currently 2u, 3u, 4u) with descriptive named constants
or a small local enum (e.g., DRAWLIST_VER_SUPPORTED,
DRAWLIST_VER_UNSUPPORTED_LOW, DRAWLIST_VER_UNSUPPORTED_HIGH) so the intent of
testing version boundaries is explicit; update the three calls that pass the
version argument and any related assertions to use these constants (refer to
zr_make_canvas_drawlist and zr_dl_validate in the failing test) and add the
constants near the top of the test file to keep them local and maintainable.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/VERSION_PINS.mdtests/unit/test_drawlist_canvas.ctests/unit/test_engine_version_negotiation.c
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/test_engine_version_negotiation.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ZR_DL_OP_BLIT_RECTand wire payloadzr_dl_cmd_blit_rect_tin the public ABI headerBLIT_RECTw > 0,h > 0, src/dst fully in-bounds)zr_fb_blit_rect(...)to preserve overlap-safe memmove semantics and per-cell metadataBLIT_RECTTesting
Local checks (Linux CI-equivalent)
bash scripts/guardrails.sh✅python3 scripts/check_version_pins.py✅bash scripts/clang_format_check.sh --diff <merge-base> HEAD✅cmake --preset posix-clang-debug -DZIREAEL_WARNINGS_AS_ERRORS=ON -DZIREAEL_BUILD_EXAMPLES=OFF✅python3 scripts/run_clang_tidy.py --build-dir out/build/posix-clang-debug✅Targeted unit coverage for this change
./out/build/posix-clang-debug/tests/zireael_unit_tests --prefix unit.drawlist_blit_rect✅ (6/6)./out/build/posix-clang-debug/tests/zireael_unit_tests --prefix unit.engine_submit_drawlist_invalid_blit_rect_has_no_partial_effects✅Note on unrelated baseline failures
Running full
cteston pristineorigin/mainreproduces unrelated existing failures:unit.drawlist_canvas_missing_blob_rejectedunit.drawlist_canvas_free_blob_invalidates_future_refsThese are independent of this PR (confirmed in a clean worktree at
origin/main).Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests