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

Metal assertion when opening modal window while running in debug mode - "indexCount(0) must be non-zero" #4857

Closed
irieger opened this issue Jan 3, 2022 · 7 comments

Comments

@irieger
Copy link

irieger commented Jan 3, 2022

Hello,

I've run into an issue with the Metal backend that I found while trying to debug something completely unrelated in a hobby project of mine. Hope the following information is of enough detail to reproduce, otherwise I'm happy to dig deeper but I so far have no actual experience with Metal UI development experience so have no idea where to start searching the problem myself.

Version/Branch of Dear ImGui:

Version: 1.87 WIP (Git Hash 9c8f288)
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_metal.mm + imgui_impl_osx.mm (Also occurs with imgui_impl_metal.mm + imgui_impl_glfw.cpp)
Compiler: Xcode 13.2.1 / Apple clang version 13.0.0 (clang-1300.0.29.30)
Operating System: macOS Monterey 12.1 (M1 / arm64)

My Issue:

When opening a modal window while running an imgui app in Debug mode (through Xcode), an assertion in the Metal framework stops the app. This can be simply provoked/reproduced by just opening the example project examples/example_apple_metal/example_apple_metal.xcodeproj and running it from Xcode (Product/Run). Then click in the "Dear ImGui Demo" window the following sequence: Popups & Modal windows / Modals / [Delete..] or Stacked Modals... After less than a second the program stops with the following debug output.

-[MTLDebugRenderCommandEncoder validateDrawIndexedPrimitives:indexCount:indexType:indexBuffer:indexBufferOffset:instanceCount:function:]:5605: failed assertion `Draw Indexed Primitives Validation
indexCount(0) must be non-zero.
'
-[MTLDebugRenderCommandEncoder validateDrawIndexedPrimitives:indexCount:indexType:indexBuffer:indexBufferOffset:instanceCount:function:]:5605: failed assertion `Draw Indexed Primitives Validation
indexCount(0) must be non-zero.

This comes from imgui_impl_metal.mm line 573 where drawIndexedPrimitives is called.

I get the same problem in an App of mine that uses the glfw and metal backends. Everything else works good. And if I change imgui_impl_metal.mm to skip the loop if pcmd->ElemCount == 0 the app runs as expected and the modal shows. Updated imgui just the past days (before I was also on docking branch with Git commit 1ad1429, there I was not running into this issue).

Can't reproduce this in the master branch. Also no issue on OpenGL3 + glfw which is my main development platform for this hobby project (on Linux).

Standalone, minimal, complete and verifiable example:

As described above, no code change needed after checking out the docking branch:
This can be simply provoked/reproduced by just opening the example project examples/example_apple_metal/example_apple_metal.xcodeproj and running it from Xcode (Product/Run). Then click in the "Dear ImGui Demo" window the following sequence: Popups & Modal windows / Modals / [Delete..] or Stacked Modals... After less than a second the program stops with the assertion output listed above.

Cheers and thank you for all the hard work on ImGui!

@ocornut
Copy link
Owner

ocornut commented Jan 3, 2022

Thank you for the detailed report.

I don't have access to OSX/Metal but I could easily "reproduce" that issue by adding an assert IM_ASSERT(pcmd->ElemCount > 0) in another backend (DX11 but all should work). So it just happens that Metal complains about the zero-count and other backend don't.

I'm going to investigate this because we shouldn't be emitting zero-elem draw commands...

@irieger
Copy link
Author

irieger commented Jan 3, 2022

I don't have access to OSX/Metal but I could easily "reproduce" that issue by adding an assert IM_ASSERT(pcmd->ElemCount > 0) in another backend (DX11 but all should work). So it just happens that Metal complains about the zero-count and other backend don't.

Of course you are right. Adding some test code I can confirm the same happening in the OpenGL3 backend adding the assertion.

One interesting point to note I forgot in the initial post: When for debugging I commented out every call inside the Modal, I did not run into this but of course was then stuck in the Modal. Maybe this is also an interesting information.

ocornut added a commit that referenced this issue Jan 3, 2022
…e creation of ImDrawCmd with zero triangles, (#4857)
@ocornut
Copy link
Owner

ocornut commented Jan 3, 2022

Pushed a fix 9ad9bd7 on master.
Not going to merge into docking right away AS we have another merge coming soon, but you can easily add the same code in your copy temporarily (until we update docking) or keep the ->ElemCount > 0 in your backend temporarly.

ocornut added a commit that referenced this issue Jan 3, 2022
…e creation of ImDrawCmd with zero triangles, (#4857)

(amended)
@ocornut
Copy link
Owner

ocornut commented Jan 3, 2022

Reworked as 9aa764c which is simpler and more general and will also merge without conflict.

@ocornut ocornut closed this as completed Jan 3, 2022
@irieger
Copy link
Author

irieger commented Jan 3, 2022

So just adding this one line in RenderDimmedBackgrounds from 9aa764c did not fix the problem on the Docking branch for me. Still getting the error/running into the assert.

@ocornut ocornut reopened this Jan 3, 2022
ocornut added a commit that referenced this issue Jan 3, 2022
…e creation of ImDrawCmd with zero triangles, (#4857)

(2nd amend)
@irieger
Copy link
Author

irieger commented Jan 3, 2022

Ok, thank you. This second call now fixed it! Closing and looking forward to the next merge into docking, but for now it is fine with the local change.

Thank you very much for this quick reaction!

@ocornut
Copy link
Owner

ocornut commented Jan 3, 2022

Right, thanks! I was writing fuller tests and found this at the same time as you did (docking has other paths).
Should be good now b0a6cd6 + merged to docking.

It's quite annoying that RenderDimmedBackgrounds() run that late... making it so that we have to do this book-keeping manually. In theory we could do it as another pass on draw commands in Render() but better to avoid excessive iterations.

I run our full test suite with the check and it works now, but there is the possibility that user manipulation of ImDrawList:: operations can lead to it, so I'm also going to add a test at the backend level too.

ocornut added a commit that referenced this issue Jan 27, 2022
…so RenderDimmedBackgrounds()/RenderMouseCursor() don't need to deal with the side-effects (#4857, #4317)
sergeyn pushed a commit to sergeyn/imgui that referenced this issue Mar 19, 2022
…so RenderDimmedBackgrounds()/RenderMouseCursor() don't need to deal with the side-effects (ocornut#4857, ocornut#4317)
ocornut added a commit that referenced this issue Dec 1, 2022
kjblanchard pushed a commit to kjblanchard/imgui that referenced this issue May 5, 2023
kjblanchard pushed a commit to kjblanchard/imgui that referenced this issue May 5, 2023
ocornut added a commit that referenced this issue Jul 10, 2023
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

2 participants