Skip to content

fix(dx12): transition render targets explicitly #259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
besmpl wants to merge 1 commit into from
+206 −51
Closed

fix(dx12): transition render targets explicitly #259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 73 additions & 49 deletions hal/dx12/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ func (e *CommandEncoder) CopyBufferToTexture(src hal.Buffer, dst hal.Texture, re

// Transition source buffer to COPY_SOURCE if needed.
e.transitionBufferIfNeeded(srcBuf, d3d12.D3D12_RESOURCE_STATE_COPY_SOURCE)
e.transitionTextureIfNeeded(dstTex, d3d12.D3D12_RESOURCE_STATE_COPY_DEST)

for _, r := range regions {
// Source location (buffer)
Expand Down Expand Up @@ -337,6 +338,10 @@ func (e *CommandEncoder) CopyTextureToBuffer(src hal.Texture, dst hal.Buffer, re
return
}

// A render target cannot be used as a copy source without an explicit
// transition. Use the tracked state as the barrier's source state here.
e.transitionTextureIfNeeded(srcTex, d3d12.D3D12_RESOURCE_STATE_COPY_SOURCE)

// Transition destination buffer to COPY_DEST if needed.
e.transitionBufferIfNeeded(dstBuf, d3d12.D3D12_RESOURCE_STATE_COPY_DEST)

Expand Down Expand Up @@ -394,6 +399,8 @@ func (e *CommandEncoder) CopyTextureToTexture(src, dst hal.Texture, regions []ha
if !srcOk || !dstOk {
return
}
e.transitionTextureIfNeeded(srcTex, d3d12.D3D12_RESOURCE_STATE_COPY_SOURCE)
e.transitionTextureIfNeeded(dstTex, d3d12.D3D12_RESOURCE_STATE_COPY_DEST)

for _, r := range regions {
// Source location
Expand Down Expand Up @@ -507,22 +514,13 @@ func (e *CommandEncoder) BeginRenderPass(desc *hal.RenderPassDescriptor) hal.Ren
return rpe
}

// Transition surface textures from PRESENT to RENDER_TARGET state.
// DX12 requires explicit barriers (unlike Vulkan which uses render pass layout transitions).
// DX12 render targets require explicit transitions from their tracked state.
for _, ca := range desc.ColorAttachments {
view, ok := ca.View.(*TextureView)
if !ok || view.texture == nil || view.texture.raw == nil {
continue
}
if view.texture.isExternal {
barrier := d3d12.NewTransitionBarrier(
view.texture.raw,
d3d12.D3D12_RESOURCE_STATE_PRESENT,
d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET,
d3d12.D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES,
)
e.cmdList.ResourceBarrier(1, &barrier)
}
e.transitionTextureIfNeeded(view.texture, d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET)
}

// Set render targets
Expand Down Expand Up @@ -662,21 +660,9 @@ func (e *RenderPassEncoder) End() {
resolveRestState = d3d12.D3D12_RESOURCE_STATE_PRESENT
}

// MSAA resolve: render target → resolve source, resolve target → resolve dest.
b1 := d3d12.NewTransitionBarrier(
msaaView.texture.raw,
d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET,
d3d12.D3D12_RESOURCE_STATE_RESOLVE_SOURCE,
d3d12.D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES,
)
b2 := d3d12.NewTransitionBarrier(
resolveView.texture.raw,
resolveRestState,
d3d12.D3D12_RESOURCE_STATE_RESOLVE_DEST,
d3d12.D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES,
)
barriers := [2]d3d12.D3D12_RESOURCE_BARRIER{b1, b2}
e.encoder.cmdList.ResourceBarrier(2, &barriers[0])
// Resolve from the resources' actual tracked states, then commit the
// resting states so later passes and copies use correct before-states.
e.encoder.prepareTextureResolve(msaaView.texture, resolveView.texture)

// Resolve MSAA → single-sample.
format := textureFormatToD3D12(msaaView.texture.format)
Expand All @@ -686,34 +672,13 @@ func (e *RenderPassEncoder) End() {
format,
)

// Transition back: MSAA → render target (for next frame),
// resolve target → resting state.
b3 := d3d12.NewTransitionBarrier(
msaaView.texture.raw,
d3d12.D3D12_RESOURCE_STATE_RESOLVE_SOURCE,
d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET,
d3d12.D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES,
)
b4 := d3d12.NewTransitionBarrier(
resolveView.texture.raw,
d3d12.D3D12_RESOURCE_STATE_RESOLVE_DEST,
resolveRestState,
d3d12.D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES,
)
barriers2 := [2]d3d12.D3D12_RESOURCE_BARRIER{b3, b4}
e.encoder.cmdList.ResourceBarrier(2, &barriers2[0])
e.encoder.finishTextureResolve(msaaView.texture, resolveView.texture, resolveRestState)
continue
}

// No resolve — just transition external surface back to PRESENT.
if msaaView.texture.isExternal {
barrier := d3d12.NewTransitionBarrier(
msaaView.texture.raw,
d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET,
d3d12.D3D12_RESOURCE_STATE_PRESENT,
d3d12.D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES,
)
e.encoder.cmdList.ResourceBarrier(1, &barrier)
e.encoder.transitionTextureIfNeeded(msaaView.texture, d3d12.D3D12_RESOURCE_STATE_PRESENT)
}
}
}
Expand Down Expand Up @@ -1129,6 +1094,49 @@ func (e *CommandEncoder) transitionBufferIfNeeded(buf *Buffer, targetState d3d12
buf.currentState = targetState
}

// transitionTextureIfNeeded inserts a transition barrier for a texture and
// updates its tracked state. Textures created by this backend are not marked
// ALLOW_SIMULTANEOUS_ACCESS, so COMMON -> RENDER_TARGET requires an explicit
// barrier (unlike buffers and simultaneous-access textures).
func (e *CommandEncoder) transitionTextureIfNeeded(tex *Texture, targetState d3d12.D3D12_RESOURCE_STATES) {
if tex == nil || tex.raw == nil || tex.currentState == targetState {
return
}
if !needsExplicitTextureBarrier(tex.currentState, targetState) {
tex.currentState = targetState
return
}

barrier := d3d12.NewTransitionBarrier(tex.raw, tex.currentState, targetState,
d3d12.D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES)
transitionTextureResourceBarrier(e.cmdList, &barrier)
hal.Logger().Debug("dx12: texture state transition",
"label", e.label,
"from", tex.currentState,
"to", targetState)
tex.currentState = targetState
}

// transitionTextureResourceBarrier is a narrow test seam around the native
// call used by transitionTextureIfNeeded.
var transitionTextureResourceBarrier = func(list *d3d12.ID3D12GraphicsCommandList, barrier *d3d12.D3D12_RESOURCE_BARRIER) {
list.ResourceBarrier(1, barrier)
}

func needsExplicitTextureBarrier(current, target d3d12.D3D12_RESOURCE_STATES) bool {
return current != target
}

func (e *CommandEncoder) prepareTextureResolve(source, destination *Texture) {
e.transitionTextureIfNeeded(source, d3d12.D3D12_RESOURCE_STATE_RESOLVE_SOURCE)
e.transitionTextureIfNeeded(destination, d3d12.D3D12_RESOURCE_STATE_RESOLVE_DEST)
}

func (e *CommandEncoder) finishTextureResolve(source, destination *Texture, destinationRestState d3d12.D3D12_RESOURCE_STATES) {
e.transitionTextureIfNeeded(source, d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET)
e.transitionTextureIfNeeded(destination, destinationRestState)
}

// transitionBuffersForCopy inserts batched transition barriers for a source and
// destination buffer pair used in a copy command. Barriers are batched into a
// single ResourceBarrier call when both buffers need transitions (Rust pattern:
Expand Down Expand Up @@ -1228,6 +1236,8 @@ func (e *CommandEncoder) setupDepthStencilAttachment(dsa *hal.RenderPassDepthSte
if !ok || !view.hasDSV {
return nil
}
depthState := depthStencilAttachmentState(view.texture.format, dsa.DepthReadOnly, dsa.StencilReadOnly)
e.transitionTextureIfNeeded(view.texture, depthState)

// Determine clear flags
var clearFlags d3d12.D3D12_CLEAR_FLAGS
Expand All @@ -1252,6 +1262,20 @@ func (e *CommandEncoder) setupDepthStencilAttachment(dsa *hal.RenderPassDepthSte
return &view.dsvHandle
}

func depthStencilAttachmentState(format gputypes.TextureFormat, depthReadOnly, stencilReadOnly bool) d3d12.D3D12_RESOURCE_STATES {
readOnly := depthReadOnly
switch format {
case gputypes.TextureFormatStencil8:
readOnly = stencilReadOnly
case gputypes.TextureFormatDepth24PlusStencil8, gputypes.TextureFormatDepth32FloatStencil8:
readOnly = depthReadOnly && stencilReadOnly
}
if readOnly {
return d3d12.D3D12_RESOURCE_STATE_DEPTH_READ
}
return d3d12.D3D12_RESOURCE_STATE_DEPTH_WRITE
}

// bufferUsageToD3D12State converts buffer usage to D3D12 resource state.
func bufferUsageToD3D12State(usage gputypes.BufferUsage) d3d12.D3D12_RESOURCE_STATES {
var state d3d12.D3D12_RESOURCE_STATES
Expand Down
4 changes: 2 additions & 2 deletions hal/dx12/device.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1287,8 +1287,8 @@ func (d *Device) CreateTexture(desc *hal.TextureDescriptor) (hal.Texture, error)
}

// All DEFAULT heap textures start in COMMON state (DX12 spec requirement).
// Matches Rust wgpu (suballocation.rs:369). Auto-promotion handles the first
// use transition (COMMON → COPY_DEST, COMMON → RENDER_TARGET, etc.).
// Matches Rust wgpu (suballocation.rs:369). The command encoder explicitly
// transitions textures to render, depth, resolve, and copy states as used.
// Previous code used non-COMMON initial states which violates the spec and
// causes incorrect barrier "from" states in PendingWrites (BUG-DX12-009).
initialState := d3d12.D3D12_RESOURCE_STATE_COMMON
Expand Down
131 changes: 131 additions & 0 deletions hal/dx12/texture_state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
//go:build windows && !(js && wasm)

package dx12

import (
"testing"
"unsafe"

"github.com/gogpu/gputypes"
"github.com/gogpu/wgpu/hal/dx12/d3d12"
)

func TestTransitionTextureIfNeededEmitsCopySourceBarrier(t *testing.T) {
texture := &Texture{raw: &d3d12.ID3D12Resource{}, currentState: d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET}
var got d3d12.D3D12_RESOURCE_BARRIER
old := transitionTextureResourceBarrier
transitionTextureResourceBarrier = func(_ *d3d12.ID3D12GraphicsCommandList, barrier *d3d12.D3D12_RESOURCE_BARRIER) { got = *barrier }
defer func() { transitionTextureResourceBarrier = old }()

(&CommandEncoder{}).transitionTextureIfNeeded(texture, d3d12.D3D12_RESOURCE_STATE_COPY_SOURCE)
if got.Type != d3d12.D3D12_RESOURCE_BARRIER_TYPE_TRANSITION {
t.Fatalf("barrier type = %d, want transition", got.Type)
}
transition := (*d3d12.D3D12_RESOURCE_TRANSITION_BARRIER)(unsafe.Pointer(&got.Union[0]))
if transition.StateBefore != d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET || transition.StateAfter != d3d12.D3D12_RESOURCE_STATE_COPY_SOURCE {
t.Fatalf("barrier states = %d -> %d, want RENDER_TARGET -> COPY_SOURCE", transition.StateBefore, transition.StateAfter)
}
if texture.currentState != d3d12.D3D12_RESOURCE_STATE_COPY_SOURCE {
t.Fatalf("tracked texture state = %d, want COPY_SOURCE", texture.currentState)
}
}

func TestTransitionTextureIfNeededEmitsRenderTargetBarrierFromCommon(t *testing.T) {
texture := &Texture{raw: &d3d12.ID3D12Resource{}, currentState: d3d12.D3D12_RESOURCE_STATE_COMMON}
var calls int
var got d3d12.D3D12_RESOURCE_BARRIER
old := transitionTextureResourceBarrier
transitionTextureResourceBarrier = func(_ *d3d12.ID3D12GraphicsCommandList, barrier *d3d12.D3D12_RESOURCE_BARRIER) {
calls++
got = *barrier
}
defer func() { transitionTextureResourceBarrier = old }()

(&CommandEncoder{}).transitionTextureIfNeeded(texture, d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET)
if calls != 1 {
t.Fatalf("ResourceBarrier calls = %d, want 1", calls)
}
transition := (*d3d12.D3D12_RESOURCE_TRANSITION_BARRIER)(unsafe.Pointer(&got.Union[0]))
if transition.StateBefore != d3d12.D3D12_RESOURCE_STATE_COMMON || transition.StateAfter != d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET {
t.Fatalf("barrier states = %d -> %d, want COMMON -> RENDER_TARGET", transition.StateBefore, transition.StateAfter)
}
if texture.currentState != d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET {
t.Fatalf("tracked texture state = %d, want RENDER_TARGET", texture.currentState)
}
}

func TestNeedsExplicitTextureBarrier(t *testing.T) {
tests := []struct {
name string
current, target d3d12.D3D12_RESOURCE_STATES
want bool
}{
{name: "same state", current: d3d12.D3D12_RESOURCE_STATE_COPY_SOURCE, target: d3d12.D3D12_RESOURCE_STATE_COPY_SOURCE},
{name: "common to render target", current: d3d12.D3D12_RESOURCE_STATE_COMMON, target: d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET, want: true},
{name: "render target to copy source", current: d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET, target: d3d12.D3D12_RESOURCE_STATE_COPY_SOURCE, want: true},
{name: "common to copy source", current: d3d12.D3D12_RESOURCE_STATE_COMMON, target: d3d12.D3D12_RESOURCE_STATE_COPY_SOURCE, want: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := needsExplicitTextureBarrier(tt.current, tt.target); got != tt.want {
t.Fatalf("needsExplicitTextureBarrier(%d, %d) = %t, want %t", tt.current, tt.target, got, tt.want)
}
})
}
}

func TestTextureResolveUsesAndCommitsTrackedStates(t *testing.T) {
source := &Texture{raw: &d3d12.ID3D12Resource{}, currentState: d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET}
destination := &Texture{raw: &d3d12.ID3D12Resource{}, currentState: d3d12.D3D12_RESOURCE_STATE_COMMON}
var transitions [][2]d3d12.D3D12_RESOURCE_STATES
old := transitionTextureResourceBarrier
transitionTextureResourceBarrier = func(_ *d3d12.ID3D12GraphicsCommandList, barrier *d3d12.D3D12_RESOURCE_BARRIER) {
transition := (*d3d12.D3D12_RESOURCE_TRANSITION_BARRIER)(unsafe.Pointer(&barrier.Union[0]))
transitions = append(transitions, [2]d3d12.D3D12_RESOURCE_STATES{transition.StateBefore, transition.StateAfter})
}
defer func() { transitionTextureResourceBarrier = old }()

encoder := &CommandEncoder{}
encoder.prepareTextureResolve(source, destination)
encoder.finishTextureResolve(source, destination, d3d12.D3D12_RESOURCE_STATE_PRESENT)
want := [][2]d3d12.D3D12_RESOURCE_STATES{
{d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET, d3d12.D3D12_RESOURCE_STATE_RESOLVE_SOURCE},
{d3d12.D3D12_RESOURCE_STATE_COMMON, d3d12.D3D12_RESOURCE_STATE_RESOLVE_DEST},
{d3d12.D3D12_RESOURCE_STATE_RESOLVE_SOURCE, d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET},
{d3d12.D3D12_RESOURCE_STATE_RESOLVE_DEST, d3d12.D3D12_RESOURCE_STATE_PRESENT},
}
if len(transitions) != len(want) {
t.Fatalf("transition count = %d, want %d", len(transitions), len(want))
}
for i := range want {
if transitions[i] != want[i] {
t.Fatalf("transition %d = %v, want %v", i, transitions[i], want[i])
}
}
if source.currentState != d3d12.D3D12_RESOURCE_STATE_RENDER_TARGET || destination.currentState != d3d12.D3D12_RESOURCE_STATE_PRESENT {
t.Fatalf("final states = %d/%d, want RENDER_TARGET/PRESENT", source.currentState, destination.currentState)
}
}

func TestDepthStencilAttachmentState(t *testing.T) {
tests := []struct {
name string
format gputypes.TextureFormat
depthReadOnly, stencilReadOnly bool
want d3d12.D3D12_RESOURCE_STATES
}{
{name: "depth writable", format: gputypes.TextureFormatDepth32Float, want: d3d12.D3D12_RESOURCE_STATE_DEPTH_WRITE},
{name: "depth read only", format: gputypes.TextureFormatDepth32Float, depthReadOnly: true, want: d3d12.D3D12_RESOURCE_STATE_DEPTH_READ},
{name: "stencil read only", format: gputypes.TextureFormatStencil8, stencilReadOnly: true, want: d3d12.D3D12_RESOURCE_STATE_DEPTH_READ},
{name: "combined read only", format: gputypes.TextureFormatDepth24PlusStencil8, depthReadOnly: true, stencilReadOnly: true, want: d3d12.D3D12_RESOURCE_STATE_DEPTH_READ},
{name: "combined depth writable", format: gputypes.TextureFormatDepth24PlusStencil8, stencilReadOnly: true, want: d3d12.D3D12_RESOURCE_STATE_DEPTH_WRITE},
{name: "combined stencil writable", format: gputypes.TextureFormatDepth24PlusStencil8, depthReadOnly: true, want: d3d12.D3D12_RESOURCE_STATE_DEPTH_WRITE},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := depthStencilAttachmentState(tt.format, tt.depthReadOnly, tt.stencilReadOnly); got != tt.want {
t.Fatalf("state = %d, want %d", got, tt.want)
}
})
}
}