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

MacOSX: CFAbsoluteTimeGetCurrent can go backwards triggering assertion. #4557

Closed
lfnoise opened this issue Sep 20, 2021 · 2 comments
Closed

Comments

@lfnoise
Copy link
Contributor

lfnoise commented Sep 20, 2021

Version: 1.85 WIP
Branch: master

Back-ends: imgui_impl_osx, imgui_impl_metal
Compiler: Xcode 12.5.1
Operating System: MacOS X 11.5.2

After leaving an imgui app running for a couple of days I found it crashed :

Assertion failed: ((g.IO.DeltaTime > 0.0f || g.FrameCount == 0) && "Need a positive DeltaTime!"), function ErrorCheckNewFrameSanityChecks, file /.../Imgui/imgui.cpp, line 7253.

ImGui_ImplOSX_NewFrame has this code that relies on CFAbsoluteTimeGetCurrent:

    // Setup time step
    if (g_Time == 0.0)
        g_Time = CFAbsoluteTimeGetCurrent();
    CFAbsoluteTime current_time = CFAbsoluteTimeGetCurrent();
    io.DeltaTime = (float)(current_time - g_Time);
    g_Time = current_time;

According to the docs for CFAbsoluteTimeGetCurrent(), "The system time may decrease due to synchronization with external time references or due to an explicit user change of the clock."
(I did not change my clock manually.)

Since CFAbsoluteTimeGetCurrent is not monotonic, imgui_impl_osx.mm should use a different time reference such as mach_absolute_time. Also probably the assertion should be made when DeltaTime is set, not when it is accessed.

@ocornut
Copy link
Owner

ocornut commented Sep 20, 2021 via email

@ocornut
Copy link
Owner

ocornut commented Sep 21, 2021

Merged as bc3d267, thank you very much!

@ocornut ocornut closed this as completed Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants