feat(render): the engine's own temporal upscaler — SGSR 2, and the resolution split - #874
Merged
Conversation
Step 4 of #481 begins. SGSR 2 over FSR 3.1 because the engine is short of cost, not of quality: 398 lines of plain GLSL ES in two passes against thousands of lines of HLSL behind the FFX_ macro system across seven, and SGSR is designed for a phone's power budget where FSR is designed for a desktop discrete GPU. BSD 3-Clause. Qualcomm's copyright header stays in the ported file, the full text is in a new NOTICE, and clause 3 is honoured explicitly. Three things did not transliterate, all of which fail silently: - Depth is reversed here and standard upstream, so their min is our max. Converted on read (1 - d) rather than by inverting every comparison, which is the version that looks right and is wrong in one place nobody finds. The identity is EXACT only because the camera uses perspective_infinite_rh_reverse_z. - Their motion is NDC and ours is UV, so their 0.5 factor is dropped. - Upstream reconstructs camera motion from a matrix when no velocity texture is bound; this engine always has real per-pixel vectors that also carry object motion, so that half is not ported. Qualcomm publishes only the shaders, no host code, so cameraFovAngleHor was recovered from a community Unity port and cross-checked against the dimensional analysis of the expression it feeds. scale_ratio's second component is the CUBE of the AREA ratio capped at 20 -- both halves are theirs and neither is obvious; cubing the linear ratio gives 3.4 where the real value is 11.4. Validated by naga in a unit test rather than at pipeline creation, which is what caught textureSampleLevel needing an integer level on a depth texture.
The seam #536 planned, built now with one technique behind it: doing it while there is a single implementation is what makes the second one a day's work, and doing it after is a refactor. Strategy dispatched by enum, not by trait object. The set is closed by construction -- an engine ships the techniques it ships, nothing downstream can define one -- which inverts the usual trade-off: no allocation, no vtable, no pointer chase, the compiler checks every site handles every variant, and the one match per frame is not a hot path. It is also what the project's Rust rules require: identities are values. The inspector dropdown needed no new machinery. #[reflect(choices)] already exists and shading_rate already uses it; upscale is a u32 with the same shape. Sgsr2 is a variant but deliberately NOT a menu entry -- its pass is not built, and an entry that selects nothing is worse than no entry. It goes in with the pass, one line. 🔴 The part that is about DATA, not code: temporal_aa is a serialised field, and replacing it outright would return every project that had turned the resolve ON with it off -- silently, which is the failure the project's rules single out. upscale therefore defaults to a SENTINEL rather than to a technique, because 0 is a real answer and an old file does not say which of the two it meant. The loader resolves it once and writes a real value. Verified the way it has to be: setting the default to 0 makes an_old_file_keeps_its_resolve fail. Also fixes two tests that #868 left behind. The motion-vector pass has been gated on having a consumer since that PR, and motion_vectors.rs never switched the resolve on -- so it asserted on a texture nobody wrote, read zeros, and both of its moving-camera cases have been red since. A gate has to be added to every test that depends on what it gates. And the sgsr2 module no longer warns in downstream builds: its constants are compiled into every project built on this engine, and a warning someone cannot act on is noise.
The half that actually reconstructs. Reprojection and a neighbourhood box are ordinary TAA; what upscales is the weight -- each low-res sample accumulates into the OUTPUT grid with a Lanczos weight taken from how far its jittered position landed from the output pixel. A pixel the jitter landed on gets a confident sample, one it missed leans on history. That weight sum doubles as the blend factor, which is why they are one number and not two heuristics. 🔴 The departure that is not optional: exposure. Upstream is a mobile renderer with display-referred colour and adds a bare 0.075 to its box. Against radiance in the hundreds that is a rounding error and the box would clamp nothing. Same operator as the resolve, applied for the same reason -- compress with exposure, do the arithmetic there, expand on write. Deliberately IDENTICAL to taa.wgsl's: two techniques that disagree about what bright means cannot be compared, and comparing them is the point of having both. 🔴 A trap that is WGSL-specific and silent: GLSL texelFetch out of range is undefined and clamps in practice, WGSL textureLoad is DEFINED to return zero. Transliterated literally it would ring a black border into the accumulation at every screen edge -- a vignette that is a missing clamp. Also drops their NDC 0.5 and the Y flip, since our motion is UV. UpscaleInputs is a struct rather than eight arguments because the next backend takes the same six things; a signature both satisfy is what makes adding one cheap. And it closes the flake that has been in this suite for sessions: contact_shadows is serialised behind the same mutex temporal_motion got. Seven cases against one shared device segfault the process rather than failing a case, intermittently, passing every time under --test-threads=1 -- which is exactly the 'fails sometimes in --workspace, always passes in isolation' that was written off as GPU contention. It was, and this is the fix. Three consecutive parallel runs green; two full-suite runs at 42 binaries green.
The technique is now selectable and runs. Strategy dispatched by value at the one site that draws: a match per frame, no vtable, and the compiler refuses a new variant that no site handles. The GPU scope carries the technique's name rather than a shared 'temporal' label -- a capture has to say WHICH one cost what, or the A/B that decides between them is two numbers under one heading. Both techniques are built unconditionally, the rule the resolve already followed: switching must not be the frame that stalls, and comparing them inside one session is how an upscaler is judged at all. tests/sgsr2_resolve.rs is the oracle #481 said a transliteration would not have. At 1:1 SGSR 2 does not upscale, it resolves, so it runs against the resolve that already ships on the same frames and a wrong port shows as a difference from a known-good image instead of as vague softness. What it does NOT cover is the resolution split, which is step 4 and unbuilt -- separating those two questions is the point. Measured on a still camera, 12 frames settled: brightness sgsr2 92.74 unresolved 92.73 sgsr2 vs plain 0.336 | taa vs plain 0.544 | sgsr2 vs taa 0.504 The matching brightness is the strong result: exposure, the range compressor and the out-of-range textureLoad clamp are all correct, and every loud failure mode of this port moves that number. SGSR 2 resolves about 60 % as hard as ours and lands somewhere different, which is plausible -- it accumulates conservatively to avoid ghosting on mobile where ours goes to 1/64 -- but that is an eye judgement, not a number, and it has not been made yet.
…tor has two #853's symptom is back at a raised point-shadow budget, and the cause is the fix's own arithmetic. The ring's bound was read as 'how many times one cull object is dispatched inside one encoder: MAX_POINT_SHADOWS, 32 today. 64 leaves a factor of two.' It counted ONE view. The stage renders every active view into one encoder and the editor has two, so at 32 casting lamps that is exactly 64 dispatches of the same cull object against 64 slots. The ring laps itself inside the frame, lamps are culled with each other's frusta, and several shadows come out as copies of one. Worse, the cursor is monotonic -- never rewound at the start of a frame -- so a frame beginning mid-ring wraps onto its own earlier slots with FEWER dispatches than there are slots. 🔴 It stayed hidden because the shipped budget was 6. Twelve dispatches into sixty-four cannot collide, so 'the point shadows work' had been measured on the one configuration that could not fail. Raising the budget to what the setting already allowed exposed it immediately. PARAMS_RING is now derived from MAX_POINT_SHADOWS x VIEWS_ASSUMED x 2. the_ring_covers_the_worst_case guards it, and the first version of that test was worthless: it derived its bound from VIEWS_ASSUMED, the same constant the ring is derived from, so both sides moved together and it could never fail. It now measures against an OBSERVED number -- the two viewports the editor renders -- and fails on the old value of 64 with the message that describes the bug. Also removes the legacy temporal_aa control from the inspector, which the dropdown replaced. The FIELD stays: it is what an old file's upscale value is migrated from, and deleting it would silently return every project that had the resolve on with it off.
… one Four hypotheses about the reported point-shadow failure were argued from the code and all four were wrong. This is the measurement instead. The discriminator: an occluder between two lamps on opposite sides throws two shadows in different directions, so if every lamp rendered the same cube, lighting both would cover no more floor than lighting one. Shadow is isolated from illumination by differencing each configuration against itself without an occluder -- two lamps are brighter than one, and comparing lit frames measures that instead. Result: two lamps make two shadows (left 18710, right 18569, both 28409) and they land in different places. The cubes are NOT shared. The sweep, kept as an --ignored tool, walks the count from 2 to 32 and each lamp keeps contributing its own shadow up to 24. It also caught the rig lying: its first run showed contribution collapsing past four lamps, which was DEFAULT_POINT_SHADOWS = 4 doing its job, not a defect. The rig now budgets MAX_POINT_SHADOWS so it measures the question asked. What is confirmed and stays open: moving the View camera changes what the Game panel draws, which the owner observed and which 8111b74 left written down as pending -- caster selection runs per VIEW over state that belongs to the STAGE. That is real. It does not explain the single-view failure on the handheld, which remains unreproduced.
The owner's call, and the reason is that the engine has no users outside this repo yet: two controls for one decision is how they end up disagreeing, and carrying a migration path for files nobody else has written costs more than it protects. BREAKING CHANGE: a project that set temporal_aa and never touched upscale loads with no temporal technique. The FILE still loads -- RON ignores the unknown key, same as when light_samples was removed -- and a_file_naming_the_deleted_toggle_still_loads pins that, because the failure that would actually hurt is the asset refusing to parse and the project falling back to engine defaults for every setting at once. The sentinel and the migration step go with it: with nothing to migrate from, upscale defaults to 0 like any other field.
Step 4 of #481, and the one that makes an upscaler worth having: the scene rasterises smaller than the window and the resolve puts it back. render_scale joins the Temporal group as AMD's preset ladder by the name each ratio is known under -- Quality 67 %, Balanced 59 %, Performance 50 % -- because that is what a player recognises and 67 % is what it means. What shrinks is everything that costs per PIXEL: the visibility buffer, the depth target, the triangle-density accumulator, both Hi-Z pyramids and every target inside the R64 stage up to the resolve. What does not is the resolve's output and the texture the blit presents. At 67 % of the width the shading pass evaluates 44 % of the pixels, and nothing else in the settings file moves the frame by that much. 🔴 Gated at the settings boundary: a technique that cannot reconstruct is refused the scale outright. None and TAA both resolve at render resolution, so handing them a smaller frame returns a smaller frame that the blit stretches -- softer, for a saving the stretch gives back. That is the classic way this setting earns a bad reputation, and it is refused rather than documented as a footgun. 🎯 jitter_phases needed exactly the change predicted when the phase count landed: its second argument. The sequence, the offsets and the matrix were already written against a ratio, so 1.5x now asks for 36 phases without touching the jitter at all.⚠️ The scale takes effect on the next resize_view, which is where a size becomes textures. The editor calls it every frame so it is immediate there; a game calls it when the surface is configured. Reallocating from inside the render would drop bind groups the GPU still has in flight. Asserted on the textures, not the image: a frame that looks right could still be rendering at full resolution and discarding the work. Measured 100x100 rasterised into a 200x200 presentation.
Found by the owner in the editor, from the picture: at 50 % the lighting broke into blocks of wrong colour. The cluster grid is indexed from frag_coord by the shading pass, and that pass runs at render resolution once a technique upscales. Built from the presented size instead, every pixel reads a froxel at twice its address -- half the grid never consulted, the other half read crossed. The two sizes agreeing was an assumption this file never had to state until the split existed. The LOD target had the same defect: it compares a meshlet's projected error against a PIXEL, and the pixels that exist are the rasterised ones. Measured against the window it keeps detail the raster cannot resolve, paying for triangles that land inside one sample. 🔴 The test for it was wrong twice before it was right, which is worth recording because both versions looked fine: 1. Mean brightness. With the bug in place it moved 0.45 % against a 20 % threshold -- three lamps covering the whole scene light it about the same however the froxels are addressed. It would take a hundred localised lights to show up in a mean. 2. The grid's DIMENSIONS. Those come from the aspect ratio and a fixed cluster budget, so 200x200 and 100x100 give the identical 13x13 and the assertion could not fail either. What is actually wrong is tile_factors, the frag_coord-to-cell mapping, which doubles when the width halves. Verified in both directions: 0.065 to 0.13 with the fix, red without it. Also reverts JITTER_BASE_PHASES to AMD's 8. Sixteen was justified by the converged case -- a blend rate of 1/64 integrates sixty frames, so eight points get averaged seven times each -- and paid for by the transient one: confidence resets the moment a pixel moves, so a moving camera blends at 0.1 and never approaches the floor. There the sequence is not integrated, it is SEEN, and sixteen positions swing twice as far as eight. The owner reported it as violent jitter.
…nder size Swept after the owner asked whether the froxel bug had siblings. It did, three of them, and the sweep found a rule worth keeping. - Both Hi-Z pyramids: a pyramid is a mip chain of the DEPTH buffer, which shrinks with the scale. Built at the window's size it describes a target that does not exist and the occlusion cull tests the wrong texels. The lazy construction path had the size twice. - The shading dispatch on both the R64 and the two-pass routes -- the pass the whole scale exists to make cheaper, dispatched over a grid that did not shrink. 🎯 The rule the sweep exposed: everything that DERIVED its size from a texture was already correct, and everything that RECEIVED a size as a parameter was wrong. Contact shadows take theirs from textureDimensions(depth_prepass_texture) and needed nothing; the projected shadows rasterise from the light at their own resolution and never look at the screen at all. Checked and left alone: cascades, cube maps, spot maps, contact shadows, and the screen UBO the shading reads, which already came from the R64 stage's own size.
The owner asked whether TAAU and SGSR 2 are the same thing. They are not: TAAU is the category and SGSR 2 is one implementation of it. This is ours. 🎯 It is the SAME shader as the resolve. At 1:1 the render grid and the output grid coincide, every gather weight collapses to one, and it is byte-for-byte the TAA that shipped -- measured at 0.0002 mean difference, and asserted, because a gather that is not inert at 1:1 would be riding along in every frame the plain resolve has ever produced. Below 1:1 the grids stop lining up and the same code gathers: each low-resolution sample weighted by how near the jitter dropped it to this output pixel, with the accumulated weight feeding the blend so a pixel the jitter missed leans on its history instead of taking a guess at full confidence. Almost all of it was already built. The jitter scaled by ratio, the motion vectors dilated over the 3x3, the disocclusion mask, the YCoCg clip and the exposure-aware range compressor are the parts a TAAU needs, and they have been in this shader for two sessions. The only new piece is the weight kernel -- FastLanczos, which is the shape both FSR and SGSR reach for. What differs from SGSR 2 is every decision inside it: history clipped in YCoCg where theirs clamps in RGB (a neighbourhood varies far more in brightness than in hue, so an RGB box is loose in the direction that matters), disocclusion from a reversed-Z ratio rather than AMD's tuned separation constant, one pass instead of two because we have real motion vectors and do not need their convert, and a range compressor that sees the exposure -- which this engine cannot do without and a mobile renderer never needs. 🔴 The jitter is SUBTRACTED in the weight. The projection was offset by it, so the sample stored at a texel was taken from that much further along; adding it moves every weight the wrong way and produces a soft, faintly swimming image that is plausible enough to ship.
Reported by the owner, who set it under TAA and reasonably expected it to apply. It was already IGNORED there -- forced to 100 unless the technique upscales -- but a control that silently does nothing is worse than an absent one, because it reads as 'I tried the setting and it did not help'. No new machinery: #[reflect(shown_when = ...)] already exists and virtual_camera.rs already uses it for the same shape of problem. The test pins the condition against the enum rather than against anything rendered. Both sets of numbers are serialised into user projects and are append-only, so a variant renumbered without updating the condition would offer the control for the wrong technique -- which is the same failure, arrived at from the other side.
The owner's comparison: SGSR 2 clearly better, and TAAU losing by JAGGED edges rather than by softness. A gather that averages badly produces a soft image; one that does not average produces steps, so the defect was never the amount of blur. Three differences from SGSR 2, found by reading both shaders side by side rather than by guessing again: - The gathered colour went into the blend UNBOUNDED. A gather is a weighted sum of taps that near a silhouette belong to different surfaces, so it can land outside every tap it was built from. That overshoot is a hard rim along each edge, which reads as aliasing the resolve failed to remove rather than as one it introduced. Now clamped to the same neighbourhood the history is clipped to, with upstream's 0.075 margin -- in COMPRESSED space, where that number means something. - The accumulated weight fed the blend raw. It runs past one when several taps land near the pixel, which hands a gathered sample more authority than a native one ever had: the current frame wins outright and brings its aliasing untouched. Upstream's factor of a third, which was simply missing. - The kernel window now scales with the ratio. 🔴 Worked out, this changes NOTHING at Performance -- their formula comes to exactly the fixed window I had -- and everything at Native, where mine was half as wide as it should be. Recorded because the first hypothesis was that the kernel explained the jagged edges at 50 %, and the arithmetic says it cannot. Still byte-identical to the plain resolve at 1:1, measured at 0.0003.
…a sampler Measured on the OneXFly: the nine-tap filtered gather cost 5.177 ms against SGSR 2's 1.868 for the same job, and took the frame from 13.91 to 17.44 ms with the GPU pinned at 100 %. Nine filtered fetches per output pixel where five direct ones do, on a device that measures as bandwidth-bound. Five in a cross and textureLoad, which is exactly what upstream does. The corners are the taps SGSR 2 leaves behind an `if (false)` with the note that they "could generate more realistic output" -- worth having when there is budget, and there is not.⚠️ textureLoad out of range is DEFINED to return zero in WGSL, so the clamp is load-bearing rather than defensive: without it every screen edge rings a black border into the accumulation. 🔴 This is a cost fix and NOT a quality one. TAAU still looked worse than SGSR 2 in the owner's comparison before this measurement existed, and nothing here changes that. What it decides is whether TAAU is worth keeping at all: at 1.868 ms it competes, at 5.177 it does not. Still byte-identical to the plain resolve at 1:1, at 0.0003.
The owner's call, and the numbers back it. Measured on the OneXFly,
same scene, same scale:
SGSR 2 TAAU
resolve 1.868 ms 4.482 ms
frame 13.910 16.290
gpu_busy 76 % 98 %
image better jagged edges
Two attempts to close the gap, and the second was worth doing for what
it taught even though it failed:
- Bounding the gather and applying upstream's weight factor. The
edges stayed jagged.
- Five taps and textureLoad instead of nine with a sampler. Predicted
~2 ms, measured 4.482 -- a 44 % cut in taps bought 13 %. So the taps
were never the cost.
🎯 What the cost actually is, counted per output pixel across both
shaders: TAAU issues 19 fetches where SGSR 2 issues 7. SGSR 2 builds
its variance box from the SAME five taps it already read for the
gather, and reads history with one bilinear tap; ours samples three
independent neighbourhoods -- five for a Catmull-Rom history, nine for
the box, five for the gather. Closing that means restructuring the
resolve, and it would still lose on image.
So the whole thing comes out rather than staying as a worse option in
a menu: the enum variant, the gather, the weight kernel, the uniforms
it needed and its tests. TAA goes back to being what it was, a resolve
that does not scale, and the shader stops carrying a path nothing
takes.
🔴 The question this answered is the one worth keeping: TAAU is the
CATEGORY and SGSR 2 is an implementation of it, and a year of somebody
tuning constants is worth more than starting from a better rejection
test. That is the argument for transliterating rather than inventing,
now with numbers instead of an opinion.
Asset value 3 is not reused: from_asset falls through to None, so a
project that stored it renders without a technique rather than
silently getting a different one.
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.
The engine's own temporal upscaler, built and measured. It closes the handheld budget for the first time.
🎉 The result, on the OneXFly
SGSR 2 at Performance (50 %),
many_lights.scene, TDP pinned at 10 W:shade: compute (half rate)motion vectorsshade: upsampletaa2.883sgsr21.868shadows·cluster grid·blit72 fps stable,
gpu_busy76 %. Before this the device sat at 93–99 % occupancy, power-limited to 1141 MHz of 2900, and still missed the 13.9 ms budget. It now makes it with a quarter of the GPU spare, at a lower clock ceiling and 4 °C cooler.🎯 Everything that costs per pixel fell between 3.7× and 4.5×; everything that does not depend on resolution stayed identical. That is the independent check that the size sweep below is complete — a miss would show up as something that did not fall, or something that fell and should not have.
What is in it
Steps 1–4 of #481. Jitter phases scaled by the render ratio; motion vectors dilated over the full 3×3; history rejected on disocclusion; and the resolution split, where the scene rasterises smaller than the window and the upscaler puts it back.
SGSR 2 transliterated from Qualcomm's GLSL (BSD-3,
NOTICEadded, clause 3 honoured). Both passes, one to convert and one to upscale.A Strategy seam (#536's), dispatched by enum rather than by trait object: the set of techniques is closed by construction, so an enum costs no allocation, no vtable and no pointer chase, and the compiler checks every site handles every variant. The inspector dropdown needed no new machinery —
#[reflect(choices)]already existed.render_scaleas AMD's preset ladder, gated so a technique that cannot reconstruct is refused it outright: handing a smaller frame to a plain resolve returns a smaller frame the blit stretches, which is softer for a saving the stretch gives back.🔴 Three things that were wrong and are now fixed
The froxel grid was sized from the window. Found by the owner from the picture, not by the suite: the shading pass indexes that grid from a fragment coordinate it produces at RENDER resolution, so every pixel read a froxel at twice its address. Sweeping for siblings found three more — the LOD target, both Hi-Z pyramids (they are mip chains of the depth buffer) and the shading dispatch itself.
🎯 The rule the sweep exposed: everything that DERIVED its size from a texture was already correct, and everything that RECEIVED a size as a parameter was wrong. Contact shadows read
textureDimensions(depth)and needed nothing; the projected shadows rasterise from the light and never look at the screen.The cull params ring was sized for one view and the editor renders two — at 32 lamps that is exactly 64 dispatches against 64 slots, and the ring laps inside the frame. It stayed hidden because the shipped budget was 6: twelve dispatches into sixty-four cannot collide, so "point shadows work" had been measured on the one configuration that could not fail.
motion_vectors.rshad been red since #868 and nobody noticed. That PR gated the pass on having a consumer; this file asserts on the pass without switching the consumer on, so it read zeros. A gate has to be added to every test that depends on what it gates.What was built, measured and removed
TAAU — our own upscaler, the same shader as the resolve. It lost on both counts: 4.482 ms against SGSR 2's 1.868, frame 16.29 against 13.91,
gpu_busy98 % against 76 %, and jagged edges.🎯 The cost was never the taps — cutting nine to five bought 13 %. Counted per output pixel, TAAU issued 19 fetches where SGSR 2 issues 7: SGSR 2 builds its variance box from the same five taps it already read for the gather and reads history with one bilinear tap, where ours sampled three independent neighbourhoods. Closing that means restructuring the resolve, and it would still lose on image.
Removed entirely rather than left as a worse menu entry. This is what turns "transliterate rather than invent" from an opinion into a measurement.
Breaking
temporal_aais deleted — the dropdown replaced it, and the owner's call was that the engine has no users outside this repo yet. A file naming it still loads (RON ignores the unknown key, same as whenlight_sampleswent) and a test pins that, because the failure that would actually hurt is the asset refusing to parse and the project falling back to engine defaults for everything at once.!: at 0.x a breaking change is a minor by convention, and 1.0.0 would claim an API stability this engine does not have.Tests
44 test binaries green. New: the phase count against the area rather than the ratio, disocclusion pinned by the 99th percentile (6 with the mask, 44 without), the froxel grid pinned by
tile_factors, two lamps making two shadows, the ring's worst case, and TAAU's identity at 1:1 before it was removed.🔴 Four of those tests could not fail when first written — a mean where the effect is local, a bound derived from the constant it was checking, a sweep measuring
DEFAULT_POINT_SHADOWSinstead of the defect, and an assertion on grid dimensions that come from the aspect ratio and not the resolution. Every one is now verified in both directions: red with the code broken, green with it fixed. The habit that catches this is stating what the test should fail against and then actually breaking it.Still open
read_capturereports the frame-to-frame variation is not explained by GPU work (r = 0.18). With the GPU now at 76 %, whatever moves the slow frames is somewhere else.Closes nothing outright; #481 stays open for steps 5–6.