Repository navigation
test(compositor): share the D3D11 render-test harness - #933
Open
quotentiroler wants to merge 1 commit into
Open
quotentiroler wants to merge 1 commit into
quotentiroler wants to merge 1 commit into
Conversation
Five Windows render tests each carried their own copy of the same setup: an identical `gpu()` probe, and a synthetic NV12 source frame (FakeFrame, MockFrame, GridFrame, Source) whose texture creation, map/unmap and AVFrame wrapping were the same ~40 lines around a different pixel fill. Four tests also carried the same `write_ppm`. tests/common/ now holds them once: - `gpu()`, unchanged. - `Nv12Frame::new(gpu, size, luma, chroma)`: `luma(col, row)` fills the Y plane, `chroma(bx, by)` returns `[Cb, Cr]` for each 2x2 block. Each test keeps its own fill as a small builder next to its existing doc comment (`fake_frame`, `mock_frame`, `grid_frame`, `source`). - `write_ppm`, unchanged, with the doc comment it had in output_geometry_golden. The D3D pieces sit behind `#[cfg(windows)]`, so output_geometry_golden, which also runs on macOS, only pulls in `write_ppm`. The textures are byte-for-byte what the old loops wrote. The old constant-chroma loops wrote every column of the UV plane alternating U/V, which equals one write per 2x2 block because every source size these tests use is even (NV12 requires it, and the aspect-ratio cases already round with `& !1`). No test logic, assertion or threshold changes.
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughCompositor tests now use shared helpers for GPU setup, synthetic NV12 frame creation, and PPM image writing. The render test scenarios and assertions remain unchanged. ChangesCompositor test utilities
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to This test-only refactor consolidates fixture setup and image writing without an identified behavior change. It is mergeable after normal checks; Windows render tests have not been executed locally. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 Clippy (1.98.1)Clippy execution failed Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This branch has not been deployed
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
Five Windows render tests in
crates/compositor/tests/each carried their own copy of the same setup: an identicalgpu()probe, and a synthetic NV12 source frame (FakeFrame,MockFrame,GridFrame,Source). In each frame type, texture creation, map/unmap and theAVFramewrapping were the same ~40 lines around a different pixel fill. Four tests also carried the samewrite_ppm.tests/common/now holds them once:gpu(), unchanged.Nv12Frame::new(gpu, size, luma, chroma):luma(col, row)fills the Y plane andchroma(bx, by)returns[Cb, Cr]for each 2×2 block. Each test keeps its own fill as a small builder next to its existing doc comment (fake_frame,mock_frame,grid_frame,source).write_ppm, unchanged, with the doc comment it had inoutput_geometry_golden.rs.The D3D pieces sit behind
#[cfg(windows)], sooutput_geometry_golden.rs, which also runs on macOS, only pulls inwrite_ppm.Net: 11 files, 479 lines removed and 264 added. No test logic, assertion or threshold changes.
Related issue
None. Found while auditing the crate for duplicated code.
Type of change
Release impact
Desktop impact
Test code only. Nothing under
src/changes.Screenshots / video
Not applicable.
Testing
The render tests were not run locally. The machine I made this on has no MSVC toolchain, and in CI
rust-windows-compositor-checkrunscargo check --all-targets, which compiles these tests but doesn't execute them. What was checked instead:cargo check --all-targets, Linux and macOScargo test): https://github.com/quotentiroler/openscreen/actions/runs/36782635810Nv12Framefill writes byte-for-byte what each old loop wrote. I checked this with a simulation of the old and new loops, for every fill in the five tests, at several sizes and with a padded row pitch. One point worth stating: the old constant-chroma loops wrote every column of the UV plane, alternating U/V, and the new code writes once per 2×2 block. Those are equal because every source size these tests use is even. NV12 requires even dimensions, and the aspect-ratio cases already round with& !1.gpu()andwrite_ppmare moved without changes.If you can run
cargo test -p openscreen-compositor --testson a Windows machine with a GPU, that exercises the moved code for real.Summary by CodeRabbit