Repository navigation
Test framework, stage 3: llrender's tests in Catch2 - #468
Merged
Merged
Conversation
TM22, the fifth wave of stage 3: llrender's 26 TUT files, 392 tests, become three Catch2 binaries, each file still run in a process of its own, cut where the GL context's lifetime differs: - llrender_tests, 7 files that need no GL: the UI transform, the matrix stack and projection helpers, and the font tables and shaping. It links ICU for alfontshaping, which holds HarfBuzz's tables against it; - llrender_gl_fresh_tests, 6 files whose cases each make a context and end the GL classes with it: the rasterizing paths of the font libraries, and llimagegl, which also makes its context again in the middle of a case; - llrender_gl_shared_tests, 13 files whose cases share one context for the process through sharedHeadlessGL() and SharedGLScope. A fresh case ends the classes a shared context needs, so the two kinds never share a binary. The files are tut2catch.py's output, with the names the idiom pass that follows chose. It marked nothing TM-CHECK, and nothing was edited by hand. tut_parity.py, against the TUT executables' recorded results (392 tests: 362 ok and 30 skipped here, on a GL 4.1 context with no compute and no clip control), holds plain for all 26 files, with seven assertion sites reviewed as never running in a passing case on this host: - alfontshaping's two-glyph branch and its fail-only else, where this font composes the combining pair into one glyph; - algpubuffer's clear of a static buffer, only where it clears on the GPU; - three of llglslshader's checks, only where the context runs compute. All 32 entries pass, and so do the three combined runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The idiom pass over TM22's conversion, by seven agents on copies, merged
back onto it. Names, messages and assertion strength only: every case and
every assertion is where the conversion put it.
- Names: 388 of 392 changed. Most were the first sentence of the comment
above the test, cut off mid-phrase, and 85 had commas, which a Catch2
test spec reads as "or".
- Messages: 2,040 to 576, with 66 CAPTUREs. Most of those removed
restated their check ("no GL error" before a check for none, "font
resolves" before a null check); most that remain say which pass,
pixel or driver case a failure is in.
- 12 asserting helpers are named require_*: require_ints, require_near,
require_matrix_close, require_program and the like.
- 105 CHECKs are REQUIREs: a program built, a texture or target made,
or a size read, that the next lines use, where going on would draw
with a failed GL object or end the process in an llassert. 128
REQUIREs that guarded nothing are CHECKs.
- One FAIL's message, built from six numbers, is a CAPTURE of them.
tut_parity.py holds all 26 files under --recounted, every case's count
unchanged, with the conversion's seven sites reviewed as never running
here.
Old test bugs the agents found, left as they were:
- checks that cannot fail: CHECK(true) in llfonttextcache, llfontfreetype
and llfontregistry; checks after an if (!x) SKIP of the same x in
alfontshaping, alfontcolrv1 and llfontfreetype; isValid() on a face
getOrCreateFace returned, which is never invalid; llimagegl's mip
width, which glTexStorage2D gives every level, and a binding read
just after the test bound it; alfontcolrv1's force-mono check, which
tests the test's own ternary;
- reads with no guard, where a failure would crash rather than fail:
getFontFreetype(), getFontFace(), templateFor(), getBitmapCache() and
loaded faces dereferenced in llfontgl, llfontregistry, llfontfreetype,
alfontshaping and alfontcolrv1; getFontFiles()[0] and mPhaseSlots[0]
unsized; llimagegl's texel reader unbounded; llglslshader's uniform
locations, storage binding points and texture units never checked
for -1, so a missing uniform reads zero and every zero expected
passes;
- checks that test something else or less than they say: alfontshaping
compares a codepoint count with a byte count; llfontbitmapcache reads
the generation before init(), whose reset bumps it; llfontfreetype's
address compare cannot tell a cache hit from a reused block; comments
promise checks that are not there (alfontcolrv1, alfontshaping,
llfontregistry, llfonttextcache, llfontfreetype, lluitransform's Y
scale);
- state a failure leaks: alfontcolrv1_gl's sForceMonochromeEmoji when a
REQUIRE fails before it is put back; alinfiniteprojection's queries
and alfontshaping's mm;
- leftovers: alfontshaping's a_addr, set and never read; a case
llfonttextcache has twice; alimageslot's fixture comment.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The overload that reads fonts.xml from a path, which only the tests call, is single-shot: with a registry already made it warns and returns. But it set the DPI, the scale and the application directory first, so a second call with other values left the fonts already made at one DPI and every later layout and measurement reading another. It now returns before setting anything. llfontgl's "a second initClass leaves the fonts already resolved in place" calls it again at 72 DPI and half scale, and left those for whatever ran next. In its own file that was the next case, whose own call at 96 put them back. In one process with llfonttextcache, an ellipsized render 40 pixels wide could follow it instead, and drew no character before the dots, in three of ten random orders. A new llfontgl case holds the DPI and scale to what they were, and fails without the change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…shared GL context ends with the main llrender_test_checks, a listener in each of llrender's three binaries, gives the state guard its checks after every case: - the GL context current on the test's thread: a case that makes one ends it, and one on the shared context leaves that one current; - how deep each of gGL's matrix stacks is, and which one its calls go to: a push left behind stays for every later case, and the stacks are 32 deep. LLRender keeps its stacks private, so it friends ll_test::RenderProbe, as lluiimage.h and llfonttextcache.h friend theirs; - the bound shader, reverse-Z and compute, and the window's viewport; - GL errors left unread, which the next case's own check for none would find. Reading them clears them, so once it has reported, the guard reads what is left, and the case after is not blamed for clearing them; - GL images alive with no context current: in the fresh binary, an image that outlives its case holds a name in a context that is gone. While the shared context lives, the font atlas pages rasterized into it outlive the cases that filled them, by design; - the font manager, LLFontGL's DPI and scale, which a later case's text is laid out and measured at, and its UI origin, depth, scale and pushed transforms. Each binary takes llcommon's checks too. I broke a case each way, and each check named it. The DPI and scale check names llfontgl's second initClass case without the change before this one. In a Catch2 binary, sharedHeadlessGL() and installWhiteTexture() hand their ends to the main's teardown, which runs after the last case while APR and LLError are still up, not to static destruction; the white texture goes first, before the context it was made in. Making the shared context changes the context, the bound program, the pushed matrices and whatever the driver chooses for the process, so the guard takes what it leaves as the state to compare with, rather than blaming the case that first asked. The fixture header is llui's and newview's TUT tests' too, so they keep the function-local statics they had. Each binary, run whole, passes 10 random orders and 20 repeats with no guard report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 of 9 tasks
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 52 minutes. View limit details
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 |
5 of 9 tasks
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
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.
Description
The fifth wave of stage 3 of the test framework's move from TUT to Catch2 (TM22): llrender's tests. 26 TUT files with 392 tests become three Catch2 binaries, cut where the GL context's lifetime differs. Each test file still runs in a process of its own.
llrender_testsllrender_gl_fresh_testsllrender_gl_shared_testssharedHeadlessGL()andSharedGLScopeA fresh case ends the classes the shared context needs, so the two kinds never share a binary.
The conversion:
6e4f183, the conversion: tut2catch.py's output, with the names the idiom pass chose, given through
--names. It marked nothing for review, and nothing was edited by hand.bb3f752, the idiom pass, by seven agents:
CAPTUREs. Most removed ones restated their check.require_*.No case and no assertion was added, removed or moved.
4ca70e1, a production fix:
LLFontGL::initClass's explicit-path overload.5b26ba4, guard checks for this wave, and the shared context's end.
ll_test::RenderProbe, whichLLRenderfriends asLLUIImageandLLFontTextCachefriend theirs;Related Issues
Issue Link: none. This follows #462 (TM21).
Checklist
Additional Notes
ctest -LE manualpass, llui's and newview's TUT GL tests on the changed fixture header among them, and so do all 15 pending combined runs, llrender's three among them;a_addr, as in TUT; that goes in the follow-up.AL_TEST_PER_CASE, the three binaries give 187, 72 and 133 case entries. All pass.tut_parity.pyholds every file to the TUT executables' recorded results: 392 tests, 362 ok and 30 skipped here.--recounted, every case's count unchanged.🤖 Generated with Claude Code