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

DirectX9 backend with minimimized viewport can assert #3424

Closed
mq1n opened this issue Aug 21, 2020 · 6 comments
Closed

DirectX9 backend with minimimized viewport can assert #3424

mq1n opened this issue Aug 21, 2020 · 6 comments

Comments

@mq1n
Copy link

mq1n commented Aug 21, 2020

Version/Branch of Dear ImGui:

Version: 1.79
Branch: docking
Full build info: https://gist.github.com/mq1n/5d74b0b889eac7cf9602e09729039028

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_dx9cpp + imgui_impl_win32.cpp
Compiler: Visual studio 2019 Win32
Operating System: Windows 10 (1909)

My Issue/Question:

  • Enable viewport on rendering ('io.ConfigViewportsNoAutoMerge' on demo app)
  • Open ImGui app and keep in foreground
  • Launch a new random process as admin(need uac confirmation screen spawn)
  • Crash/assertion on ImGui app,.

Screenshots/Video

https://streamable.com/e4zdt3

@ocornut ocornut changed the title ImGuiConfigFlags_ViewportsEnable cause to crash DirectX9 backend with minimimized viewport can assert Aug 21, 2020
@ocornut
Copy link
Owner

ocornut commented Aug 21, 2020

Hello @mq1n , thanks for your report.
I have now confirmed this issue and fixed this assert.
Typically Present() will report D3DERR_DEVICELOST and it is up to the main application to reset the device (main.cpp does it).

@mq1n
Copy link
Author

mq1n commented Aug 21, 2020

Hello @ocornut, thanks for your quick fix, I just tried on demo app and works well but my own project still has same issue but hr value is 0x08760878(S_PRESENT_OCCLUDED) so I don't understand what can cause to assertion.

@ocornut
Copy link
Owner

ocornut commented Aug 21, 2020

Try changing it to IM_ASSERT(hr == D3D_OK || hr == D3DERR_DEVICELOST || hr == S_PRESENT_OCCLUDED); and see if it works for you. Ideally we should stop rendering on that situation.

@mq1n
Copy link
Author

mq1n commented Aug 21, 2020

Hello again @ocornut, I tried but after this change assertion triggered with S_PRESENT_MODE_CHANGED(0x08760877). Now I switched assertion to SUCCESSED macro and now looks like working well.

	ImGuiViewportDataDx9* data = (ImGuiViewportDataDx9*)viewport->RendererUserData;
	HRESULT hr = data->SwapChain->Present(NULL, NULL, data->d3dpp.hDeviceWindow, NULL, NULL);
	IM_ASSERT(SUCCEEDED(hr));

Thanks for your help.

ocornut added a commit that referenced this issue Oct 2, 2023
@ocornut
Copy link
Owner

ocornut commented Oct 2, 2023

While investigating #6887 i stumbled on this same issue and applied the fix you suggest with cbc474d 9064dbd. Thank you!

ocornut added a commit that referenced this issue Oct 2, 2023
@sakiodre
Copy link
Contributor

While having a similar issue, I discovered that the proper method for managing HRESULT is to employ the SUCCEEDED or FAILED macro. From Error Handling in COM:

The constants S_OK and S_FALSE are both success codes. Probably 99% of COM methods return S_OK when they succeed; but do not let this fact mislead you. A method might return other success codes, so always test for errors by using the SUCCEEDED or FAILED macro.

Upon examining our code base, I found 112 instances where == S_OK or != S_OK is used, which is not the correct approach.

Initially, I wanted to propose using the appropriate macro. However, the descriptions for other success codes beginning with "S_" are somewhat unclear. I'm uncertain if there have been instances where a function completely failed but still returned an "S_" code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants