Skip to content
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.

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

Direct3D12 sample doesn't render properly in release builds #3406

Closed
cx20 opened this issue Dec 28, 2024 · 11 comments · Fixed by #3419
Closed

Direct3D12 sample doesn't render properly in release builds #3406

cx20 opened this issue Dec 28, 2024 · 11 comments · Fixed by #3419
Labels
bug Something isn't working

Comments

@cx20
Copy link

cx20 commented Dec 28, 2024

Summary

Overview

This issue is regarding the DirectX12 triangle sample located at:
crates/samples/windows/direct3d12/src/main.rs

Issue Description

The triangle sample shows different behavior between debug and release builds:

  • Debug build: Triangle renders correctly
  • Release build: Triangle doesn't render when the rendering logic relies on WM_PAINT messages

Reproduction Steps

  1. Clone the repository
  2. Build and run the DirectX12 triangle sample in debug mode
cargo run
  • Triangle renders correctly
  1. Build and run in release mode
cargo run --release
  • The window appears but remains blank (white background) with no triangle visible

Root Cause

The current implementation relies on WM_PAINT messages for rendering:

fn sample_wndproc<S: DXSample>(sample: &mut S, message: u32, wparam: WPARAM) -> bool {
 match message {
     WM_PAINT => {
         sample.update();
         sample.render();
         true
     }
     ...
 }
}

This approach may not trigger frequently enough in release builds due to optimization, leading to inconsistent rendering behavior.

Suggested Fix

Implement continuous rendering in the main message loop instead of relying on WM_PAINT:

loop {
    let mut message = MSG::default();
    if unsafe { PeekMessageA(&mut message, None, 0, 0, PM_REMOVE) }.into() {
        unsafe {
            _ = TranslateMessage(&message);
            DispatchMessageA(&message);
        }
        if message.message == WM_QUIT {
            break;
        }
    } else {
        // Render when no messages are available
        sample.update();
        sample.render();
    }
}

Environment

Rust Version: rustc 1.85.0-nightly (dd84b7d5e 2024-12-27)
Windows Version: Windows 11 Version 24H2 (OS Build 26100.2605)
Graphics Card: NVIDIA GeForce RTX 2060
Driver Version: GeForce Game Ready Driver 555.97

Crate manifest

https://github.com/microsoft/windows-rs/blob/master/crates/samples/windows/direct3d12/Cargo.toml

Crate code

https://github.com/microsoft/windows-rs/blob/master/crates/samples/windows/direct3d12/src/main.rs
@cx20 cx20 added the bug Something isn't working label Dec 28, 2024
@kennykerr
Copy link
Collaborator

Good catch - that sample can definitely be simplified.

@damyanp
Copy link
Member

damyanp commented Jan 6, 2025

The sample follows the same strategy as the C++ version (https://github.com/microsoft/DirectX-Graphics-Samples/blob/master/Samples/Desktop/D3D12HelloWorld/src/HelloTriangle/Win32Application.cpp#L104). I've not heard of any issues with that one. It'd be good to try and understand why there's a difference in behavior here before making any changes.

@kennykerr
Copy link
Collaborator

I tested and sure enough --release builds don't render properly - I haven't looked beyond that.

@kennykerr kennykerr changed the title [DirectX12 Sample] Inconsistent rendering behavior between debug and release builds DirectX12 sample doesn't render properly in release builds Jan 6, 2025
@kennykerr kennykerr changed the title DirectX12 sample doesn't render properly in release builds Direct3D12 sample doesn't render properly in release builds Jan 6, 2025
@riverar
Copy link
Collaborator

riverar commented Jan 6, 2025

The WM_CREATE arm in the window message handler is being optimized away in release mode, which is surprising since the SetWindowLongPtrA call ultimately interacts with an extern function. Could this be a Rust compiler bug? Or are we confusing the compiler with our codegen or cfg attributes? (cc @ChrisDenton)

Note making the pointer read volatile or using the return value of SetWindowLongPtrA resolves the issue, but neither approach feels ideal in this case.

@ChrisDenton
Copy link
Collaborator

I'll look into it when I have a moment but from the description it sounds suspiciously like there's UB lurking somewhere in the unsafe code.

@ChrisDenton
Copy link
Collaborator

ChrisDenton commented Jan 6, 2025

I looked at the code 🎉. Thanks to @riverar for narrowing it down, I think I identified the cause as being the transmute, which creates a pointer without provenance. Hopefully #3419 should fix this, if you'd like to test it.

@riverar
Copy link
Collaborator

riverar commented Jan 6, 2025

I saw that too but it doesn't quite explain why the compiler suddenly optimizes the entire arm out. Something still seems amiss.

@ChrisDenton
Copy link
Collaborator

ChrisDenton commented Jan 6, 2025

Dereferencing a pointer without provenance is UB. Any branch that reaches UB "can't happen" and so LLVM is within its right to optimize out unreachable branches.

@kennykerr
Copy link
Collaborator

It certainly fixes the problem. 😉

@riverar
Copy link
Collaborator

riverar commented Jan 6, 2025

Printing the return value of the SetWindowLongPtrA call also works. 🙃

I'm OK with the change, since transmuting integers to pointers for non-zero sized reads is explicitly documented as undefined behavior, but it feels like there's something else lurking here.

@ChrisDenton
Copy link
Collaborator

I think the UB fully explains it. It's just that println, volatile, etc are very good ways to persuade LLVM not to optimize something out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants