From 44c51a38349a58313d22897d8366c0f710cb4509 Mon Sep 17 00:00:00 2001 From: Stephen Saunders Date: Sat, 6 Jan 2024 20:04:25 -0500 Subject: [PATCH 1/4] Win32: Improve win_glimp for better error recovery, remove redundant code, update comments --- neo/sys/DeviceManager_DX12.cpp | 15 ++-- neo/sys/win32/win_glimp.cpp | 135 +++++++-------------------------- 2 files changed, 31 insertions(+), 119 deletions(-) diff --git a/neo/sys/DeviceManager_DX12.cpp b/neo/sys/DeviceManager_DX12.cpp index 29d9142a2..0d2e6b5fb 100644 --- a/neo/sys/DeviceManager_DX12.cpp +++ b/neo/sys/DeviceManager_DX12.cpp @@ -165,7 +165,7 @@ static RefCountPtr FindAdapter( const std::wstring& targetName ) return targetAdapter; } - +/* SRS - No longer needed, window centering now done in CreateWindowDeviceAndSwapChain() within win_glimp.cpp // Adjust window rect so that it is centred on the given adapter. Clamps to fit if it's too big. static bool MoveWindowOntoAdapter( IDXGIAdapter* targetAdapter, RECT& rect ) { @@ -203,7 +203,7 @@ static bool MoveWindowOntoAdapter( IDXGIAdapter* targetAdapter, RECT& rect ) return false; } - +*/ void DeviceManager_DX12::ReportLiveObjects() { nvrhi::RefCountPtr pDebug; @@ -297,16 +297,11 @@ bool DeviceManager_DX12::CreateDeviceAndSwapChain() */ HRESULT hr = E_FAIL; - RECT clientRect; - GetClientRect( ( HWND )windowHandle, &clientRect ); - UINT width = clientRect.right - clientRect.left; - UINT height = clientRect.bottom - clientRect.top; - ZeroMemory( &m_SwapChainDesc, sizeof( m_SwapChainDesc ) ); - m_SwapChainDesc.Width = width; - m_SwapChainDesc.Height = height; + m_SwapChainDesc.Width = m_DeviceParams.backBufferWidth; + m_SwapChainDesc.Height = m_DeviceParams.backBufferHeight; m_SwapChainDesc.SampleDesc.Count = m_DeviceParams.swapChainSampleCount; - m_SwapChainDesc.SampleDesc.Quality = 0; + m_SwapChainDesc.SampleDesc.Quality = m_DeviceParams.swapChainSampleQuality; m_SwapChainDesc.BufferUsage = m_DeviceParams.swapChainUsage; m_SwapChainDesc.BufferCount = m_DeviceParams.swapChainBufferCount; m_SwapChainDesc.SwapEffect = DXGI_SWAP_EFFECT_FLIP_DISCARD; diff --git a/neo/sys/win32/win_glimp.cpp b/neo/sys/win32/win_glimp.cpp index e5c7ce6d4..1ba044d8b 100755 --- a/neo/sys/win32/win_glimp.cpp +++ b/neo/sys/win32/win_glimp.cpp @@ -156,7 +156,7 @@ static void GLW_CreateWindowClasses() if( !RegisterClass( &wc ) ) { - common->FatalError( "GLW_CreateWindow: could not register window class" ); + common->FatalError( "GLW_CreateWindowClasses: could not register window class" ); } common->Printf( "...registered window class\n" ); @@ -219,12 +219,6 @@ static bool GetDisplayCoordinates( const int deviceNum, int& x, int& y, int& wid { bool verbose = false; - idStr deviceName = GetDeviceName( deviceNum ); - if( deviceName.Length() == 0 ) - { - return false; - } - DISPLAY_DEVICE device = {}; device.cb = sizeof( device ); if( !EnumDisplayDevices( @@ -239,7 +233,7 @@ static bool GetDisplayCoordinates( const int deviceNum, int& x, int& y, int& wid DISPLAY_DEVICE monitor; monitor.cb = sizeof( monitor ); if( !EnumDisplayDevices( - deviceName.c_str(), + device.DeviceName, 0, &monitor, 0 /* dwFlags */ ) ) @@ -249,7 +243,7 @@ static bool GetDisplayCoordinates( const int deviceNum, int& x, int& y, int& wid DEVMODE devmode; devmode.dmSize = sizeof( devmode ); - if( !EnumDisplaySettings( deviceName.c_str(), ENUM_CURRENT_SETTINGS, &devmode ) ) + if( !EnumDisplaySettings( device.DeviceName, ENUM_CURRENT_SETTINGS, &devmode ) ) { return false; } @@ -457,7 +451,10 @@ bool R_GetModeListForDisplay( const int requestedDisplayNum, idList& // get the monitor for this display if( !( device.StateFlags & ( DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE ) ) ) { - continue; + // SRS - If requested display number is not attached to desktop, no monitor is present. In this case we + // return true with an empty mode list for this display number. This can result in non-contiguous + // display numbers on Windows, but is better than undefined behaviour for "filled-in" false displays. + return true; } DISPLAY_DEVICE monitor; @@ -554,17 +551,12 @@ int DisplayMax() dd.cb = sizeof( DISPLAY_DEVICE ); int deviceNum = 0; - int deviceMax = 0; + int deviceMax = -1; while( EnumDisplayDevices( NULL, deviceNum, &dd, 0 ) ) { if( dd.StateFlags & ( DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE ) ) { - DISPLAY_DEVICE monitor = { 0 }; - monitor.cb = sizeof( DISPLAY_DEVICE ); - if( EnumDisplayDevices( dd.DeviceName, 0, &monitor, 0 ) ) - { - deviceMax = deviceNum; - } + deviceMax = deviceNum; } deviceNum++; } @@ -581,16 +573,11 @@ int DisplayPrimary() { if( dd.StateFlags & DISPLAY_DEVICE_PRIMARY_DEVICE ) { - DISPLAY_DEVICE monitor = { 0 }; - monitor.cb = sizeof( DISPLAY_DEVICE ); - if( EnumDisplayDevices( dd.DeviceName, 0, &monitor, 0 ) ) - { - return deviceNum; - } + return deviceNum; } deviceNum++; } - return 0; + return -1; } /* @@ -784,7 +771,7 @@ bool DeviceManager::CreateWindowDeviceAndSwapChain( const glimpParms_t& parms, c if( !win32.hWnd ) { - common->Printf( "^3GLW_CreateWindow() - Couldn't create window^0\n" ); + common->Printf( "^3CreateWindowDeviceAndSwapChain() - Couldn't create window^0\n" ); return false; } @@ -798,7 +785,7 @@ bool DeviceManager::CreateWindowDeviceAndSwapChain( const glimpParms_t& parms, c win32.hDC = GetDC( win32.hWnd ); if( !win32.hDC ) { - common->Printf( "^3GLW_CreateWindow() - GetDC()failed^0\n" ); + common->Printf( "^3CreateWindowDeviceAndSwapChain() - GetDC() failed^0\n" ); return false; } @@ -806,7 +793,7 @@ bool DeviceManager::CreateWindowDeviceAndSwapChain( const glimpParms_t& parms, c RECT rect; if( !GetClientRect( win32.hWnd, &rect ) ) { - common->Printf( "^3GLW_CreateWindow() - GetClientRect() failed^0\n" ); + common->Printf( "^3CreateWindowDeviceAndSwapChain() - GetClientRect() failed^0\n" ); return false; } @@ -861,77 +848,6 @@ void DeviceManager::UpdateWindowSize( const glimpParms_t& parms ) } } -/* -======================= -GLW_CreateWindow - -Responsible for creating the Win32 window. -If fullscreen, it won't have a border -======================= -*/ -static bool GLW_CreateWindow( glimpParms_t parms ) -{ - int x, y, w, h; - if( !GLW_GetWindowDimensions( parms, x, y, w, h ) ) - { - return false; - } - - int stylebits; - int exstyle; - if( parms.fullScreen != 0 ) - { - exstyle = WS_EX_TOPMOST; - stylebits = WS_POPUP | WS_VISIBLE | WS_SYSMENU; - } - else - { - exstyle = 0; - stylebits = WINDOW_STYLE | WS_SYSMENU; - } - - win32.hWnd = CreateWindowEx( - exstyle, - WIN32_WINDOW_CLASS_NAME, - GAME_NAME, - stylebits, - x, y, w, h, - NULL, - NULL, - win32.hInstance, - NULL ); - - if( !win32.hWnd ) - { - common->Printf( "^3GLW_CreateWindow() - Couldn't create window^0\n" ); - return false; - } - - ::SetTimer( win32.hWnd, 0, 100, NULL ); - - ShowWindow( win32.hWnd, SW_SHOW ); - UpdateWindow( win32.hWnd ); - common->Printf( "...created window @ %d,%d (%dx%d)\n", x, y, w, h ); - - // makeCurrent NULL frees the DC, so get another - win32.hDC = GetDC( win32.hWnd ); - if( !win32.hDC ) - { - common->Printf( "^3GLW_CreateWindow() - GetDC()failed^0\n" ); - return false; - } - - // TODO - glConfig.stereoPixelFormatAvailable = false; - - SetForegroundWindow( win32.hWnd ); - SetFocus( win32.hWnd ); - - glConfig.isFullscreen = parms.fullScreen; - - return true; -} - /* =================== PrintCDSError @@ -1138,6 +1054,7 @@ bool GLimp_Init( glimpParms_t parms ) if( !GetClientRect( win32.hWnd, &rect ) ) { common->Printf( "^3GLimp_Init() - GetClientRect() failed^0\n" ); + GLimp_Shutdown(); return false; } @@ -1263,17 +1180,6 @@ void DeviceManager::Shutdown() windowHandle = nullptr; win32.hWnd = NULL; } - - // reset display settings - if( win32.cdsFullscreen ) - { - common->Printf( "...resetting display\n" ); - ChangeDisplaySettings( 0, 0 ); - win32.cdsFullscreen = 0; - } - - // restore gamma - GLimp_RestoreGamma(); } /* @@ -1290,6 +1196,17 @@ void GLimp_Shutdown() { deviceManager->Shutdown(); } + + // reset display settings + if( win32.cdsFullscreen ) + { + common->Printf( "...resetting display\n" ); + ChangeDisplaySettings( 0, 0 ); + win32.cdsFullscreen = 0; + } + + // restore gamma + GLimp_RestoreGamma(); } From f9c387a5b5aa727c36db73292928892f66159b4b Mon Sep 17 00:00:00 2001 From: Stephen Saunders Date: Sat, 6 Jan 2024 20:10:36 -0500 Subject: [PATCH 2/4] Improve safeMode for startup recovery, fix shutdown for slow renderers, update old OpenGL log entries --- neo/renderer/NVRHI/RenderBackend_NVRHI.cpp | 12 ++++---- neo/renderer/RenderSystem_init.cpp | 34 +++++++++++++++------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/neo/renderer/NVRHI/RenderBackend_NVRHI.cpp b/neo/renderer/NVRHI/RenderBackend_NVRHI.cpp index baf70f908..687cc7c08 100644 --- a/neo/renderer/NVRHI/RenderBackend_NVRHI.cpp +++ b/neo/renderer/NVRHI/RenderBackend_NVRHI.cpp @@ -138,11 +138,11 @@ static NvrhiContext prevContext; /* ================== -R_InitOpenGL +idRenderBackend::Init -This function is responsible for initializing a valid OpenGL subsystem -for rendering. This is done by calling the system specific GLimp_Init, -which gives us a working OGL subsystem, then setting all necessary openGL +This function is responsible for initializing a valid DX12/Vulkan subsystem +for rendering. This is done by calling the system specific GLimp_Init/VKimp_Init, +which gives us a working subsystem, then setting all necessary renderer state, including images, vertex programs, and display lists. Changes to the vertex cache size or smp state require a vid_restart. @@ -154,11 +154,11 @@ and model information functions. */ void idRenderBackend::Init() { - common->Printf( "----- R_InitOpenGL -----\n" ); + common->Printf( "----- idRenderBackend::Init -----\n" ); if( tr.IsInitialized() ) { - common->FatalError( "R_InitOpenGL called while active" ); + common->FatalError( "idRenderBackend::Init called while active" ); } // SRS - create deviceManager here to prevent allocation loop via R_SetNewMode( true ) diff --git a/neo/renderer/RenderSystem_init.cpp b/neo/renderer/RenderSystem_init.cpp index 0c2d7d9fc..b42c40388 100644 --- a/neo/renderer/RenderSystem_init.cpp +++ b/neo/renderer/RenderSystem_init.cpp @@ -402,9 +402,8 @@ void R_SetNewMode( const bool fullInit ) idList modeList; if( !R_GetModeListForDisplay( r_fullscreen.GetInteger() - 1, modeList ) ) { - idLib::Printf( "r_fullscreen reset from %i to 1 because mode list failed.\n", r_fullscreen.GetInteger() ); - r_fullscreen.SetInteger( 1 ); - R_GetModeListForDisplay( r_fullscreen.GetInteger() - 1, modeList ); + idLib::Printf( "Going to safe mode because display not found.\n" ); + goto safeMode; } if( modeList.Num() < 1 ) @@ -502,7 +501,7 @@ void R_SetNewMode( const bool fullInit ) if( i == 2 ) { - common->FatalError( "Unable to initialize OpenGL" ); + common->FatalError( "Unable to initialize renderer" ); } if( i == 0 ) @@ -514,8 +513,25 @@ void R_SetNewMode( const bool fullInit ) safeMode: // if we failed, set everything back to "safe mode" // and try again + + // SRS - get the first display with a non-zero mode list, or fail if not found + int safeDisplay = 0; + idList safeList; + for( ; ; safeDisplay++ ) + { + if( !R_GetModeListForDisplay( safeDisplay, safeList ) ) + { + common->FatalError( "Unable to find a valid display for renderer" ); + } + else if( safeList.Num() > 0 ) + { + break; + } + } + // SRS end + r_vidMode.SetInteger( 0 ); - r_fullscreen.SetInteger( 1 ); + r_fullscreen.SetInteger( safeDisplay + 1 ); r_displayRefresh.SetInteger( 0 ); r_antiAliasing.SetInteger( 0 ); } @@ -2201,12 +2217,8 @@ void idRenderSystemLocal::Shutdown() UnbindBufferObjects(); - // SRS - wait for fence to hit before freeing any resources the GPU may be using, otherwise get Vulkan validation layer errors on shutdown - // SRS - skip this step if we are in a Doom Classic game - if( common->GetCurrentGame() == DOOM3_BFG ) - { - backend.GL_BlockingSwapBuffers(); - } + // SRS - wait for device idle before freeing any resources the GPU may be using, otherwise get errors on shutdown + deviceManager->GetDevice()->waitForIdle(); // free the vertex cache, which should have nothing allocated now vertexCache.Shutdown(); From e95f2fffe97816f9364e88489ca1024fa360a8fd Mon Sep 17 00:00:00 2001 From: Stephen Saunders Date: Sun, 7 Jan 2024 11:44:38 -0500 Subject: [PATCH 3/4] Handle all no-monitor cases, add primary display number to log --- neo/sys/win32/win_glimp.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/neo/sys/win32/win_glimp.cpp b/neo/sys/win32/win_glimp.cpp index 1ba044d8b..37180c87e 100755 --- a/neo/sys/win32/win_glimp.cpp +++ b/neo/sys/win32/win_glimp.cpp @@ -451,7 +451,7 @@ bool R_GetModeListForDisplay( const int requestedDisplayNum, idList& // get the monitor for this display if( !( device.StateFlags & ( DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE ) ) ) { - // SRS - If requested display number is not attached to desktop, no monitor is present. In this case we + // SRS - If requested display number is not attached to desktop, skip this device. In this case we // return true with an empty mode list for this display number. This can result in non-contiguous // display numbers on Windows, but is better than undefined behaviour for "filled-in" false displays. return true; @@ -465,7 +465,10 @@ bool R_GetModeListForDisplay( const int requestedDisplayNum, idList& &monitor, 0 /* dwFlags */ ) ) { - continue; + // SRS - If EnumDisplayDevices() returns false for device, no monitor is detected. In this case we + // return true with an empty mode list for this display number. This can result in non-contiguous + // display numbers on Windows, but is better than undefined behaviour for "filled-in" false displays. + return true; } DEVMODE devmode; @@ -646,7 +649,7 @@ static bool GLW_GetWindowDimensions( const glimpParms_t parms, int& x, int& y, i { displayNotFound = true; displayNum = DisplayPrimary(); - common->Warning( "Window position out of bounds, falling back to primary display" ); + common->Warning( "Window position out of bounds, falling back to primary display %d", displayNum + 1 ); } // get the current monitor position and size on the desktop, assuming From 7b57b1de4e681744a146a74647f2e28738690309 Mon Sep 17 00:00:00 2001 From: Stephen Saunders Date: Sun, 7 Jan 2024 14:41:16 -0500 Subject: [PATCH 4/4] Use first attached monitor as fallback for primary if not found --- neo/sys/win32/win_glimp.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/neo/sys/win32/win_glimp.cpp b/neo/sys/win32/win_glimp.cpp index 37180c87e..1a3c6b027 100755 --- a/neo/sys/win32/win_glimp.cpp +++ b/neo/sys/win32/win_glimp.cpp @@ -572,15 +572,22 @@ int DisplayPrimary() dd.cb = sizeof( DISPLAY_DEVICE ); int deviceNum = 0; + int deviceMin = -1; while( EnumDisplayDevices( NULL, deviceNum, &dd, 0 ) ) { if( dd.StateFlags & DISPLAY_DEVICE_PRIMARY_DEVICE ) { return deviceNum; } + + // SRS - save first device attached to desktop as fallback if primary device not found + if( deviceMin < 0 && ( dd.StateFlags & DISPLAY_DEVICE_ATTACHED_TO_DESKTOP ) ) + { + deviceMin = deviceNum; + } deviceNum++; } - return -1; + return deviceMin; } /*