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

Examples: Metal: Restore state after UserCallback #2452

Closed
wants to merge 1 commit into from

Conversation

bear24rw
Copy link
Contributor

In other backends it is possible for the UserCallback to read the current state, do whatever state changes it needs, and then restore the imgui state before returning. In Metal there doesn't seem to be a way for a UserCallback to save the existing state before changing it so we should restore our state after the UserCallback is complete.

In other backends it is possible for the UserCallback to read the current state, do whatever state changes it needs, and then restore the imgui state. In Metal there doesn't seem to be a way for a UserCallback to save the existing state before changing it so we should restore our state after the UserCallback is complete.
@ocornut
Copy link
Owner

ocornut commented Mar 27, 2019

Hello,

This is not desirable. See #2037 to see why.
We ought to add some special defines/dummy callbacks to explicitely instruct the renderer to do this sort of thing.

It won't be merged as-is, we can keep this PR open as a reminder that a solution is needed for that problem.

@bear24rw
Copy link
Contributor Author

Okay, after reading those threads it makes sense not to blindly reset the state. I really like the idea of being able to AddCallback(ImDrawCallback_ResetState).

ocornut added a commit that referenced this pull request Mar 29, 2019
…upport. Not tested much. Not covering all renderers. (#2037, #1639, #2452)
@ocornut
Copy link
Owner

ocornut commented Mar 29, 2019

Hello @bear24rw,

See the work done in the branch mentioned here:
#2037 (comment)

Would you be able to provide a commit to do the same in the Metal back-end, following the overall structure, comments and naming convention used by the other back-ends?
Apart from the overall structure, note that your PR only setup three things (setRenderPipelineState, setVertexBuffer, setVertexBufferOffset) whereas the intent of ImDrawCallback_ResetRenderState is to re-setup everything, so I presume everything from setCullMode would be moved to the new function?

(You can push the commit in a fork of that new branch or an isolated patch and I'll cherry-pick it from there)

Thank you!

@bear24rw
Copy link
Contributor Author

@ocornut , I have pushed a branch here:

https://github.com/bear24rw/imgui/tree/features/drawcallback_reset_render_state_metal

Specific commit:

bear24rw@e74e501

@ocornut
Copy link
Owner

ocornut commented Mar 29, 2019

Thank you Max! I have merged this commit in the feature branch.

I'll wait for some extra feedback about this feedback and want to see if the other discussion (override the texture binding) would have any effect that is best bundled with the same update, but overall this is looking good to me.

@ocornut
Copy link
Owner

ocornut commented Apr 1, 2019

Closing this as the discussion is moved to #2037, thanks again!

@ocornut
Copy link
Owner

ocornut commented Apr 30, 2019

Branch has been merged in now.
Using ImDrawList::AddCallback(ImDrawCallback_ResetRenderState) it is possible to request the renderer to reset render state. This is now supported by all example renderers.

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

Successfully merging this pull request may close these issues.

2 participants