Skip to content

Commit 959675e

Browse files
sasvdwclaude
andcommitted
refactor: name the Direct3D12 transition pass after Direct3D12 concepts
Review feedback from @Ethereal77 on #3329. "Attachment" is a Vulkan term. Direct3D12 has a resource, a view, and a barrier that says how the resource is used. IsBoundAsAttachment becomes IsBoundAsRenderTargetOrDepth, and the comment beside it drops the same word. The type test moves from that helper to the loop that calls it. A reader of the loop now sees which resources the pass acts on, and the helper takes a Texture, so the question of what happens to a Buffer no longer arises at the call site. Buffers stay out of scope, which matches the Vulkan pass. That pass skips any descriptor that is not a sampled or storage image. A buffer barrier also replaces the whole access mask, and nothing restores the vertex, index or constant buffer access a buffer may still need, because binding those emits no barrier. Covering buffers means combining every access a buffer is currently bound for, and both backends should gain that together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent ccf564d commit 959675e

3 files changed

Lines changed: 15 additions & 22 deletions

File tree

sources/engine/Stride.Graphics/Direct3D12/CommandList.Direct3D12.cs

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -406,20 +406,20 @@ private void PrepareDraw(bool isDispatch = false)
406406
}
407407

408408
/// <summary>
409-
/// Transitions every Graphics Resource bound in a Descriptor Set to the state its use needs.
409+
/// Transitions every Texture bound in a Descriptor Set to the state its use needs.
410410
/// </summary>
411411
/// <remarks>
412412
/// <para>
413-
/// Explicit transitions at pass boundaries remain the norm, because only a pass knows
414-
/// enough about its own inputs and outputs to batch them. This pass is the floor beneath
415-
/// that, for generic code such as <c>ComputeEffectShader</c> that binds an unordered
416-
/// access view and declares no transition of its own.
417-
/// </para>
418-
/// <para>
419413
/// The pass is idempotent. <see cref="ResourceBarrierTransition"/> returns early when the
420414
/// tracked layout already matches, so a resource an explicit transition already moved
421415
/// costs nothing here.
422416
/// </para>
417+
/// <para>
418+
/// Buffers are out of scope, which matches the Vulkan pass. A buffer barrier replaces the
419+
/// whole access mask, and nothing restores the vertex, index or constant buffer access a
420+
/// buffer may still need, because binding those emits no barrier. Covering buffers means
421+
/// combining every access a buffer is currently bound for, on both backends together.
422+
/// </para>
423423
/// </remarks>
424424
private void TransitionBoundResources()
425425
{
@@ -436,27 +436,23 @@ private void TransitionBoundResources()
436436
int slotCount = descriptorSet.Description.SrvCount;
437437
for (int slot = 0; slot < slotCount; slot++)
438438
{
439-
var resource = tracking.Resources[slot];
440-
if (resource is null)
439+
if (tracking.Resources[slot] is not Texture texture)
441440
continue;
442441

443-
// An attachment the pipeline is about to write keeps its attachment state. Its
444-
// producer already transitioned it, and moving it here would invalidate the draw.
445-
if (IsBoundAsAttachment(resource))
442+
// A render target or depth buffer the pipeline is about to write keeps the state
443+
// its producer set. Moving it here would invalidate the draw.
444+
if (IsBoundAsRenderTargetOrDepth(texture))
446445
continue;
447446

448447
ResourceBarrierTransition(
449-
resource,
448+
texture,
450449
tracking.IsUAV[slot] ? BarrierLayout.UnorderedAccess : BarrierLayout.ShaderResource);
451450
}
452451
}
453452
}
454453

455-
private bool IsBoundAsAttachment(GraphicsResource resource)
454+
private bool IsBoundAsRenderTargetOrDepth(Texture texture)
456455
{
457-
if (resource is not Texture texture)
458-
return false;
459-
460456
var parent = texture.ParentTexture ?? texture;
461457

462458
var depth = DepthStencilBuffer;

sources/engine/Stride.Graphics/Direct3D12/DescriptorPool.Direct3D12.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,8 @@ internal ResourceTracking RentTracking(int srvCount, int handleIncrementSize)
124124

125125
trackingOffset++;
126126

127-
// Clearing also leaves an untouched slot empty, which is what a slot holding a constant
128-
// buffer must look like to the transition pass.
127+
// A slot the Descriptor Set never writes must read as empty, because a constant buffer
128+
// occupies a slot without a resource the transition pass can act on.
129129
tracking.Clear();
130130

131131
return tracking;

sources/engine/Stride.Graphics/Direct3D12/DescriptorSet.Direct3D12.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,6 @@ internal void Set(int bindingOffset, GraphicsResource resource, bool isUav)
248248
IsUAV[index] = isUav;
249249
}
250250

251-
/// <summary>
252-
/// Reports whether this instance has room for a Descriptor Set with the given slot count.
253-
/// </summary>
254251
internal bool CanTrack(int srvCount) => Resources.Length >= srvCount;
255252

256253
/// <summary>

0 commit comments

Comments
 (0)