epezent / implot

Immediate Mode Plotting
MIT License
4.64k stars 516 forks source link

Improve plot interfaces to be more flexible and convenient #445

Open nuuSolutions opened 1 year ago

nuuSolutions commented 1 year ago

As I understand it now, it it only possible to plot the exact data that is already present in memory

What one would often like to do: Plot something that is calculated from the raw data.

Possible implementation: Pass a lambda (or transform function) to all PlotXYZ() functions The lambda transforms the data to our liking. The lambda can be invocable on an arbitrary data type, but of course produces one result of a plottable type

I might post or push an example when I get it working the CALL_INSTANTIATE_FOR_NUMERIC_TYPES thing seems to be getting in the way though.

bear24rw commented 1 year ago

That is already possible via the ImPlot::PlotLineG function, check out the example in this comment: https://github.com/epezent/implot/blob/d87512353495e7760e7fda7566a05beef7627d8f/implot.h#L838-L848

nuuSolutions commented 1 year ago

You are right, it is a very general solution, but it confused me at first, and I still don't like it for some reasons:

  1. it is not easy/intuitive how to use e.g. PlotLine and PlotLineG together, because the other (scale, offset, ...) parameters are missing.
  2. passing around a void* def a smell. But ok, I can use it with nullptr (which was not intuitive). I guess I would have done it slightly differently. yeah, yeah, the instantiation...

Anyway, thx for pointing out that I should rather use the G versions than mess with your code ;-)

BenBE commented 1 year ago

Would it be possible to allow for the callback to be any std::function<ImPlotPoint(int idx, void* data)>? That way the above example would reduce to:

   if (ImPlot::BeginPlot("MyPlot")) {
       MyData my_data;
       auto my_lambda = [&my_data](int idx, void*) {
           double t = idx / 999.0;
           return ImPlotPoint(t, 0.5+0.5*my_data.at(idx)*std::sin(2*PI*10*t));
       };
       ImPlot::PlotScatterG("scatter", MyDataGetter, &my_data, my_data.Size());
       ImPlot::PlotLineG("line", my_lambda, nullptr, 1000);
       ImPlot::EndPlot();
   }
nuuSolutions commented 1 year ago

the getter type could just be an additional template parameter, but the underlying issue is that the ImPlotXYZ templates need to be instantiated (see code CALL_INSTANTIATE_FOR_NUMERIC_TYPES) I'm still trying to understand that better

As a workaround I'm using static ImPlotPoint GetFoo( int idx, void* ) and not using the void*. I use a (static) data pointer that I set myself. Not ideal, but as you can't pass member functions as the getter, I went with this for now.

BenBE commented 1 year ago

Not ideal, but as you can't pass member functions as the getter, I went with this for now.

You can, but it's a hassle to implement …

struct func_invoker {
  void * data;
  std::function<ImPlotPoint(int, void*)> f;
  static invoke(int index, void* p) {
    func_invoker *obj = reinterpret_cast<func_invoker*>(p);
    return obj->f(index, obj->data);
  }
};

You construct the func_invoker as func_invoker fi{"your actual data pointer", &your_instance->your_method}; and provide fi as the data pointer to ImPlot and func_invoker::invoke as the function to call. Not beautiful and an absolute hassle, including a hazardous invitation for memory leaks, but it works …

nuuSolutions commented 1 year ago

I've prototyped the real deal. With type erasure and a bunch of different getters. Unfortunately this repo does not seem to have much interest

BenBE commented 1 year ago

I'd very much be interested if ImGui itself could provide std::function for its interface directly and avoid the hassle with needing a function invoker class as intermediary in between … FWIW, providing std::function directly also natively supports plan function pointers as before, thus wouldn't even break existing code …

nuuSolutions commented 1 year ago

Yeah, I realized that I basically re-invented std::function - but in a quirky way to support the in-built implot getters. I say quirky because there's always two getters mangled up - plus the 'count'. What one really needs is simple (i.e. individual) value getters. Each of those would be std::function<float(int)> (and similar for other supported types) Unfortunately there seems to be an inherited mistrust of the standard lib here ;-)

epezent commented 1 year ago

While I would love it if PlotG could take a std::function, we're not going to take a hard dependency on STL in implot.h (we try to honor ImGui's stance on this topic).

However, I am not opposed to having an optional implot.hpp with C++ conveniences such as this, for users who want it. I would welcome any PR...

BenBE commented 11 months ago

we try to honor ImGui's stance on this topic

Got some link for further reading for this? TIA.

epezent commented 11 months ago

we try to honor ImGui's stance on this topic

Got some link for further reading for this? TIA.

Not specifically that I know of. It's a very clear decision that has been made by ImGui if you've looked at any of its implementation. @ocornut may be able to provide specific rational.

nuuSolutions commented 11 months ago

Somebody has to honor good old void*

ocornut commented 11 months ago

Not specifically that I know of. It's a very clear decision that has been made by ImGui if you've looked at any of its implementation. @ocornut may be able to provide specific rational.

In the past few years I've been incredibly asked about being able to use capturing lambda, which as I understand requires the callback type to be a std::function<> type and therefore #include <functional>.

I used this approach in the Dear ImGui Test Engine: https://github.com/ocornut/imgui_test_engine/blob/main/imgui_test_engine/imgui_te_utils.h

#if IMGUI_TEST_ENGINE_ENABLE_STD_FUNCTION
#include <functional>
#define ImFuncPtr(FUNC_TYPE)        std::function<FUNC_TYPE>
#else
#define ImFuncPtr(FUNC_TYPE)        FUNC_TYPE*
#endif

Then:

// some typedef
typedef bool (ImGuiScreenCaptureFunc)(ImGuiID viewport_id, int x, int y, int w, int h, unsigned int* pixels, void* user_data);

// in struct or param
ImFuncPtr(ImGuiScreenCaptureFunc) ScreenCaptureFunc = NULL;       // (Optional) To capture graphics output (application _MUST_ call ImGuiTestEngine_PostSwap() function after swapping is framebuffer)

I've been considering that imgui and/or implot could do the same thing. One issue is that it becomes a compile-time requirement, and with increasing amount of people relying on precompiled package it is a annoying. Though you could argue that C++ users are less likely to use precompiled packages and the only one would who care about this?

We could provide a separate header (ala misc/cpp/imgui_stdlib.h) to add extra API entry points, but this wouldn't work for functions stored in structures....

Feels like IMGUI_ENABLE_STD_FUNCTION would be a rather immediate and simpler solution.

ocornut commented 11 months ago

Though there's an additional question. It's extremely likely that calling through a std::function is forty two million times slower than a raw function pointer and this could matter for ImPlot... (for ImGui wouldn't be an issue currently as we don't have callbacks called with such frequency).

So perhaps on your end you should to setup a system where it is possible to use a raw ImPlotGetter without std::function overhead, and have some magic where a std::function can be used as a ImPlotGetter with some intricate indirection.

I don't know C++ enough to dig so much further, but TL;DR i am not against introducing opt-in compile-time support for std::function but I think you need to be exploring those performance avenues a little seriously too.

ocornut commented 11 months ago

Quickly testing in imgui_test_engine.

Raw function pointer

Raw function pointer, debug, vs2022
--------------------------------------------------

                    e->IO.SrcFileOpenFunc(test->SourceFile, test->SourceLine, e->IO.SrcFileOpenUserData);
00007FF7A4BC98F2  mov         rax,qword ptr [e]  
00007FF7A4BC98F9  mov         r8,qword ptr [rax+18h]  
00007FF7A4BC98FD  mov         rax,qword ptr [rbp+1E8h]  
00007FF7A4BC9904  mov         edx,dword ptr [rax+20h]  
00007FF7A4BC9907  mov         rax,qword ptr [rbp+1E8h]  
00007FF7A4BC990E  mov         rcx,qword ptr [rax+18h]  
00007FF7A4BC9912  mov         rax,qword ptr [e]  
00007FF7A4BC9919  call        qword ptr [rax+8]    // lands on my code

Just adding std::function<> to the member, once more C++ STL doesn't disappoint in disappointing:

std::function, debug, vs2022
--------------------------------------------------

                    e->IO.SrcFileOpenFunc(test->SourceFile, test->SourceLine, e->IO.SrcFileOpenUserData);
00007FF65304EAA4  mov         rax,qword ptr [e]  
00007FF65304EAAB  add         rax,8  
00007FF65304EAAF  mov         rcx,qword ptr [e]  
00007FF65304EAB6  mov         r9,qword ptr [rcx+88h]  
00007FF65304EABD  mov         rcx,qword ptr [rbp+1E8h]  
00007FF65304EAC4  mov         r8d,dword ptr [rcx+20h]  
00007FF65304EAC8  mov         rcx,qword ptr [rbp+1E8h]  
00007FF65304EACF  mov         rdx,qword ptr [rcx+18h]  
00007FF65304EAD3  mov         rcx,rax  
00007FF65304EAD6  call        std::_Func_class<void,char const * __ptr64,int,void * __ptr64>::operator() (07FF6526C1D59h)  

...going through some jump table (perhaps for edit&continue or other debug feature)
00007FF6526C1D59  jmp         std::_Func_class<void,char const * __ptr64,int,void * __ptr64>::operator() (07FF65304B7E0h)  

...operator()
    _Ret operator()(_Types... _Args) const
        {   // call through stored object
00007FF65304B7E0  mov         qword ptr [rsp+20h],r9  
00007FF65304B7E5  mov         dword ptr [rsp+18h],r8d  
00007FF65304B7EA  mov         qword ptr [rsp+10h],rdx  
00007FF65304B7EF  mov         qword ptr [rsp+8],rcx  
00007FF65304B7F4  push        rbp  
00007FF65304B7F5  push        rdi  
00007FF65304B7F6  sub         rsp,108h  
00007FF65304B7FD  lea         rbp,[rsp+20h]  
00007FF65304B802  mov         rdi,rsp  
00007FF65304B805  mov         ecx,42h  
00007FF65304B80A  mov         eax,0CCCCCCCCh  
00007FF65304B80F  rep stos    dword ptr [rdi]  
00007FF65304B811  mov         rcx,qword ptr [rsp+128h]  
        if (_Empty())
00007FF65304B819  mov         rcx,qword ptr [this]  
00007FF65304B820  call        std::_Func_class<void,char const * __ptr64,int,void * __ptr64>::_Empty (07FF6526B8303h)  
.... 00007FF6526B8303  jmp    std::_Func_class<void,char const * __ptr64,int,void * __ptr64>::_Empty (07FF652E00110h)  
....    bool _Empty() const _NOEXCEPT
....        {   // return true if no stored object
.... 00007FF652E00110  mov         qword ptr [rsp+8],rcx  
.... 00007FF652E00115  push        rbp  
.... 00007FF652E00116  push        rdi  
.... 00007FF652E00117  sub         rsp,0F8h  
.... 00007FF652E0011E  lea         rbp,[rsp+20h]  
.... 00007FF652E00123  mov         rdi,rsp  
.... 00007FF652E00126  mov         ecx,3Eh  
.... 00007FF652E0012B  mov         eax,0CCCCCCCCh  
.... 00007FF652E00130  rep stos    dword ptr [rdi]  
.... 00007FF652E00132  mov         rcx,qword ptr [rsp+118h]  
....        return (_Getimpl() == 0);
.... 00007FF652E0013A  mov         rcx,qword ptr [this]  
.... 00007FF652E00141  call        std::_Func_class<void,char const * __ptr64,int,void * __ptr64>::_Getimpl (07FF6526C5ED6h)  
.... 00007FF652E00146  test        rax,rax  
.... 00007FF652E00149  jne         std::_Func_class<void,char const * __ptr64,int,void * __ptr64>::_Empty+47h (07FF652E00157h)  
.... 00007FF652E0014B  mov         dword ptr [rbp+0C0h],1  
.... 00007FF652E00155  jmp         std::_Func_class<void,char const * __ptr64,int,void * __ptr64>::_Empty+51h (07FF652E00161h)  
.... 00007FF652E00157  mov         dword ptr [rbp+0C0h],0  
.... 00007FF652E00161  movzx       eax,byte ptr [rbp+0C0h]  
....        }
.... 00007FF652E00168  lea         rsp,[rbp+0D8h]  
.... 00007FF652E0016F  pop         rdi  
.... 00007FF652E00170  pop         rbp  
.... 00007FF652E00171  ret  
00007FF65304B825  movzx       eax,al  
00007FF65304B828  test        eax,eax  
00007FF65304B82A  je          std::_Func_class<void,char const * __ptr64,int,void * __ptr64>::operator()+52h (07FF65304B832h)  
            _Xbad_function_call();
00007FF65304B82C  call        qword ptr [__imp_std::_Xbad_function_call (07FF6536C03D8h)]  
        return (_Getimpl()->_Do_call(_STD forward<_Types>(_Args)...));
00007FF65304B832  mov         rcx,qword ptr [this]  
00007FF65304B839  call        std::_Func_class<void,char const * __ptr64,int,void * __ptr64>::_Getimpl (07FF6526C5ED6h)  
.... 00007FF6526C5ED6  jmp    std::_Func_class<void,char const * __ptr64,int,void * __ptr64>::_Getimpl (07FF652E00550h)  
..... _Ptrt *_Getimpl() const _NOEXCEPT
....        {   // get pointer to object
.... 00007FF652E00550  mov         qword ptr [rsp+8],rcx  
.... 00007FF652E00555  push        rbp  
.... 00007FF652E00556  push        rdi  
.... 00007FF652E00557  sub         rsp,0C8h  
.... 00007FF652E0055E  mov         rbp,rsp  
.... 00007FF652E00561  mov         rdi,rsp  
.... 00007FF652E00564  mov         ecx,32h  
.... 00007FF652E00569  mov         eax,0CCCCCCCCh  
.... 00007FF652E0056E  rep stos    dword ptr [rdi]  
.... 00007FF652E00570  mov         rcx,qword ptr [rsp+0E8h]  
....        return (_Mystorage._Ptrs[_Num_ptrs - 1]);
.... 00007FF652E00578  mov         eax,8  
.... 00007FF652E0057D  imul        rax,rax,7  
.... 00007FF652E00581  mov         rcx,qword ptr [this]  
.... 00007FF652E00588  mov         rax,qword ptr [rcx+rax]  
....        }
.... 00007FF652E0058C  lea         rsp,[rbp+0C8h]  
.... 00007FF652E00593  pop         rdi  
.... 00007FF652E00594  pop         rbp  
.... 00007FF652E00595  ret  
00007FF65304B83E  mov         qword ptr [rbp+0C0h],rax  
00007FF65304B845  lea         rcx,[<_Args_2>]  
00007FF65304B84C  call        std::forward<void * __ptr64> (07FF6526D8351h)  
.... 00007FF6526D8351  jmp         std::forward<void * __ptr64> (07FF652DFD290h)  
.... template<class _Ty> inline
....    constexpr _Ty&& forward(
....        typename remove_reference<_Ty>::type& _Arg) _NOEXCEPT
....    {   // forward an lvalue as either an lvalue or an rvalue
.... 00007FF652DFD290  mov         qword ptr [rsp+8],rcx  
.... 00007FF652DFD295  push        rbp  
.... 00007FF652DFD296  push        rdi  
.... 00007FF652DFD297  sub         rsp,0C8h  
.... 00007FF652DFD29E  mov         rbp,rsp  
.... 00007FF652DFD2A1  mov         rdi,rsp  
.... 00007FF652DFD2A4  mov         ecx,32h  
.... 00007FF652DFD2A9  mov         eax,0CCCCCCCCh  
.... 00007FF652DFD2AE  rep stos    dword ptr [rdi]  
.... 00007FF652DFD2B0  mov         rcx,qword ptr [rsp+0E8h]  
....    return (static_cast<_Ty&&>(_Arg));
.... 00007FF652DFD2B8  mov         rax,qword ptr [_Arg]  
....    }
.... 00007FF652DFD2BF  lea         rsp,[rbp+0C8h]  
.... 00007FF652DFD2C6  pop         rdi  
.... 00007FF652DFD2C7  pop         rbp  
.... 00007FF652DFD2C8  ret  
00007FF65304B851  mov         qword ptr [rbp+0C8h],rax  
00007FF65304B858  lea         rcx,[<_Args_1>]  
00007FF65304B85F  call        std::forward<int> (07FF6526C53FFh)  
.... 00007FF6526C53FF  jmp         std::forward<int> (07FF652DFD100h)  
.... template<class _Ty> inline
....    constexpr _Ty&& forward(
....        typename remove_reference<_Ty>::type& _Arg) _NOEXCEPT
....    {   // forward an lvalue as either an lvalue or an rvalue
.... 00007FF652DFD100  mov         qword ptr [rsp+8],rcx  
.... 00007FF652DFD105  push        rbp  
.... 00007FF652DFD106  push        rdi  
.... 00007FF652DFD107  sub         rsp,0C8h  
.... 00007FF652DFD10E  mov         rbp,rsp  
.... 00007FF652DFD111  mov         rdi,rsp  
.... 00007FF652DFD114  mov         ecx,32h  
.... 00007FF652DFD119  mov         eax,0CCCCCCCCh  
.... 00007FF652DFD11E  rep stos    dword ptr [rdi]  
.... 00007FF652DFD120  mov         rcx,qword ptr [rsp+0E8h]  
....    return (static_cast<_Ty&&>(_Arg));
.... 00007FF652DFD128  mov         rax,qword ptr [_Arg]  
....    }
.... 00007FF652DFD12F  lea         rsp,[rbp+0C8h]  
.... 00007FF652DFD136  pop         rdi  
.... 00007FF652DFD137  pop         rbp  
.... 00007FF652DFD138  ret  
00007FF65304B864  mov         qword ptr [rbp+0D0h],rax  
00007FF65304B86B  lea         rcx,[<_Args_0>]  
00007FF65304B872  call        std::forward<char const * __ptr64> (07FF6526C3177h)  
.... 00007FF6526C3177  jmp         std::forward<char const * __ptr64> (07FF652DFD2E0h)  
.... template<class _Ty> inline
....    constexpr _Ty&& forward(
....        typename remove_reference<_Ty>::type& _Arg) _NOEXCEPT
....    {   // forward an lvalue as either an lvalue or an rvalue
.... 00007FF652DFD2E0  mov         qword ptr [rsp+8],rcx  
.... 00007FF652DFD2E5  push        rbp  
.... 00007FF652DFD2E6  push        rdi  
.... 00007FF652DFD2E7  sub         rsp,0C8h  
.... 00007FF652DFD2EE  mov         rbp,rsp  
.... 00007FF652DFD2F1  mov         rdi,rsp  
.... 00007FF652DFD2F4  mov         ecx,32h  
.... 00007FF652DFD2F9  mov         eax,0CCCCCCCCh  
.... 00007FF652DFD2FE  rep stos    dword ptr [rdi]  
.... 00007FF652DFD300  mov         rcx,qword ptr [rsp+0E8h]  
....    return (static_cast<_Ty&&>(_Arg));
.... 00007FF652DFD308  mov         rax,qword ptr [_Arg]  
....    }
.... 00007FF652DFD30F  lea         rsp,[rbp+0C8h]  
.... 00007FF652DFD316  pop         rdi  
.... 00007FF652DFD317  pop         rbp  
.... 00007FF652DFD318  ret  
00007FF65304B877  mov         rcx,qword ptr [rbp+0C0h]  
00007FF65304B87E  mov         rcx,qword ptr [rcx]  
00007FF65304B881  mov         qword ptr [rbp+0D8h],rcx  
00007FF65304B888  mov         rdx,qword ptr [rbp+0C8h]  
00007FF65304B88F  mov         r9,rdx  
00007FF65304B892  mov         rdx,qword ptr [rbp+0D0h]  
00007FF65304B899  mov         r8,rdx  
00007FF65304B89C  mov         rdx,rax  
00007FF65304B89F  mov         rcx,qword ptr [rbp+0C0h]  
00007FF65304B8A6  mov         rax,qword ptr [rbp+0D8h]  
00007FF65304B8AD  call        qword ptr [rax+10h]  <---------- actual call
.... 00007FF6526D0502  jmp         std::_Func_impl<void (__cdecl*)(char const * __ptr64,int,void * __ptr64),std::allocator<int>,void,char const * __ptr64,int,void * __ptr64>::_Do_call (07FF652DFFEA0h)  
....    virtual _Rx _Do_call(_Types&&... _Args)
....        {   // call wrapped function
.... 00007FF652DFFEA0  mov         qword ptr [rsp+20h],r9  
.... 00007FF652DFFEA5  mov         qword ptr [rsp+18h],r8  
.... 00007FF652DFFEAA  mov         qword ptr [rsp+10h],rdx  
.... 00007FF652DFFEAF  mov         qword ptr [rsp+8],rcx  
.... 00007FF652DFFEB4  push        rbp  
.... 00007FF652DFFEB5  push        rdi  
.... 00007FF652DFFEB6  sub         rsp,128h  
.... 00007FF652DFFEBD  lea         rbp,[rsp+30h]  
.... 00007FF652DFFEC2  mov         rdi,rsp  
.... 00007FF652DFFEC5  mov         ecx,4Ah  
.... 00007FF652DFFECA  mov         eax,0CCCCCCCCh  
.... 00007FF652DFFECF  rep stos    dword ptr [rdi]  
.... 00007FF652DFFED1  mov         rcx,qword ptr [rsp+148h]  
....        return (_Invoke_ret(_Forced<_Rx>(), _Callee(),
.... 00007FF652DFFED9  mov         rcx,qword ptr [<_Args_2>]  
.... 00007FF652DFFEE0  call        std::forward<void * __ptr64> (07FF6526D8351h)  
........ gave up unwrapping this
.... 00007FF652DFFEE5  mov         qword ptr [rbp+0D8h],rax  
.... 00007FF652DFFEEC  mov         rcx,qword ptr [<_Args_1>]  
.... 00007FF652DFFEF3  call        std::forward<int> (07FF6526C53FFh)  
........ gave up unwrapping this
.... 00007FF652DFFEF8  mov         qword ptr [rbp+0E0h],rax  
.... 00007FF652DFFEFF  mov         rcx,qword ptr [<_Args_0>]  
.... 00007FF652DFFF06  call        std::forward<char const * __ptr64> (07FF6526C3177h)  
........ gave up unwrapping this
.... 00007FF652DFFF0B  mov         qword ptr [rbp+0E8h],rax  
.... 00007FF652DFFF12  mov         rcx,qword ptr [this]  
.... 00007FF652DFFF19  call        std::_Func_impl<void (__cdecl*)(char const * __ptr64,int,void * __ptr64),std::allocator<int>,void,char const * __ptr64,int,void * __ptr64>::_Callee (07FF6526BE37Ah)  
......... 00007FF6526BE37A  jmp         std::_Func_impl<void (__cdecl*)(char const * __ptr64,int,void * __ptr64),std::allocator<int>,void,char const * __ptr64,int,void * __ptr64>::_Callee (07FF652DFFAA0h)  
.........   _Callable& _Callee() _NOEXCEPT
.........       {   // return reference to wrapped function
......... 00007FF652DFFAA0  mov         qword ptr [rsp+8],rcx  
......... 00007FF652DFFAA5  push        rbp  
......... 00007FF652DFFAA6  push        rdi  
......... 00007FF652DFFAA7  sub         rsp,0E8h  
......... 00007FF652DFFAAE  lea         rbp,[rsp+20h]  
......... 00007FF652DFFAB3  mov         rdi,rsp  
......... 00007FF652DFFAB6  mov         ecx,3Ah  
......... 00007FF652DFFABB  mov         eax,0CCCCCCCCh  
......... 00007FF652DFFAC0  rep stos    dword ptr [rdi]  
......... 00007FF652DFFAC2  mov         rcx,qword ptr [rsp+108h]  
.........       return (_Mypair._Get_second());
......... 00007FF652DFFACA  mov         rax,qword ptr [this]  
......... 00007FF652DFFAD1  add         rax,8  
......... 00007FF652DFFAD5  mov         rcx,rax  
......... 00007FF652DFFAD8  call        std::_Compressed_pair<std::allocator<int>,void (__cdecl*)(char const * __ptr64,int,void * __ptr64),1>::_Get_second (07FF6526D3833h)  
.............. 00007FF6526D3833  jmp         std::_Compressed_pair<std::allocator<int>,void (__cdecl*)(char const * __ptr64,int,void * __ptr64),1>::_Get_second (07FF652E00410h)  
..............  _Ty2& _Get_second() _NOEXCEPT
..............      {   // return reference to second
.............. 00007FF652E00410  mov         qword ptr [rsp+8],rcx  
.............. 00007FF652E00415  push        rbp  
.............. 00007FF652E00416  push        rdi  
.............. 00007FF652E00417  sub         rsp,0C8h  
.............. 00007FF652E0041E  mov         rbp,rsp  
.............. 00007FF652E00421  mov         rdi,rsp  
.............. 00007FF652E00424  mov         ecx,32h  
.............. 00007FF652E00429  mov         eax,0CCCCCCCCh  
.............. 00007FF652E0042E  rep stos    dword ptr [rdi]  
.............. 00007FF652E00430  mov         rcx,qword ptr [rsp+0E8h]  
..............      return (_Myval2);
.............. 00007FF652E00438  mov         rax,qword ptr [this]  
..............      }
.............. 00007FF652E0043F  lea         rsp,[rbp+0C8h]  
.............. 00007FF652E00446  pop         rdi  
.............. 00007FF652E00447  pop         rbp  
.............. 00007FF652E00448  ret  
.........       }
......... 00007FF652DFFADD  lea         rsp,[rbp+0C8h]  
......... 00007FF652DFFAE4  pop         rdi  
......... 00007FF652DFFAE5  pop         rbp  
......... 00007FF652DFFAE6  ret  
.... 00007FF652DFFF1E  mov         rcx,qword ptr [rbp+0D8h]  
.... 00007FF652DFFF25  mov         qword ptr [rsp+20h],rcx  
.... 00007FF652DFFF2A  mov         rcx,qword ptr [rbp+0E0h]  
.... 00007FF652DFFF31  mov         r9,rcx  
.... 00007FF652DFFF34  mov         rcx,qword ptr [rbp+0E8h]  
.... 00007FF652DFFF3B  mov         r8,rcx  
.... 00007FF652DFFF3E  mov         rdx,rax  
.... 00007FF652DFFF41  movzx       ecx,byte ptr [rbp+0C4h]  
.... 00007FF652DFFF48  call        std::_Invoke_ret<void,void (__cdecl*& __ptr64)(char const * __ptr64,int,void * __ptr64),char const * __ptr64,int,void * __ptr64> (07FF6526AB03Bh)  
......... 00007FF6526AB03B  jmp         std::_Invoke_ret<void,void (__cdecl*& __ptr64)(char const * __ptr64,int,void * __ptr64),char const * __ptr64,int,void * __ptr64> (07FF652DFB590h)  
......... template<class _Cv_void,
.........   class... _Valtys> inline
.........   void _Invoke_ret(_Forced<_Cv_void, true>, _Valtys&&... _Vals)
.........   {   // INVOKE, "implicitly" converted to void
......... 00007FF652DFB590  mov         qword ptr [rsp+20h],r9  
......... 00007FF652DFB595  mov         qword ptr [rsp+18h],r8  
......... 00007FF652DFB59A  mov         qword ptr [rsp+10h],rdx  
......... 00007FF652DFB59F  mov         byte ptr [rsp+8],cl  
......... 00007FF652DFB5A3  push        rbp  
......... 00007FF652DFB5A4  push        rdi  
......... 00007FF652DFB5A5  sub         rsp,108h  
......... 00007FF652DFB5AC  lea         rbp,[rsp+20h]  
......... 00007FF652DFB5B1  mov         rdi,rsp  
......... 00007FF652DFB5B4  mov         ecx,42h  
......... 00007FF652DFB5B9  mov         eax,0CCCCCCCCh  
......... 00007FF652DFB5BE  rep stos    dword ptr [rdi]  
......... 00007FF652DFB5C0  movzx       ecx,byte ptr [rsp+128h]  
.........   _STD invoke(_STD forward<_Valtys>(_Vals)...);
......... 00007FF652DFB5C8  mov         rcx,qword ptr [<_Vals_3>]  
......... 00007FF652DFB5CF  call        std::forward<void * __ptr64> (07FF6526D8351h)  
.............. gave up unwrapping this
......... 00007FF652DFB5D4  mov         qword ptr [rbp+0C0h],rax  
......... 00007FF652DFB5DB  mov         rcx,qword ptr [<_Vals_2>]  
......... 00007FF652DFB5E2  call        std::forward<int> (07FF6526C53FFh)  
.............. gave up unwrapping this
......... 00007FF652DFB5E7  mov         qword ptr [rbp+0C8h],rax  
......... 00007FF652DFB5EE  mov         rcx,qword ptr [<_Vals_1>]  
......... 00007FF652DFB5F5  call        std::forward<char const * __ptr64> (07FF6526C3177h)  
.............. gave up unwrapping this
......... 00007FF652DFB5FA  mov         qword ptr [rbp+0D0h],rax  
......... 00007FF652DFB601  mov         rcx,qword ptr [<_Vals_0>]  
......... 00007FF652DFB608  call        std::forward<void (__cdecl*& __ptr64)(char const * __ptr64,int,void * __ptr64)> (07FF6526BF996h)  
.............. gave up unwrapping this
......... 00007FF652DFB60D  mov         rcx,qword ptr [rbp+0C0h]  
......... 00007FF652DFB614  mov         r9,rcx  
......... 00007FF652DFB617  mov         rcx,qword ptr [rbp+0C8h]  
......... 00007FF652DFB61E  mov         r8,rcx  
......... 00007FF652DFB621  mov         rcx,qword ptr [rbp+0D0h]  
......... 00007FF652DFB628  mov         rdx,rcx  
......... 00007FF652DFB62B  mov         rcx,rax  
......... 00007FF652DFB62E  call        std::invoke<void (__cdecl*& __ptr64)(char const * __ptr64,int,void * __ptr64),char const * __ptr64,int,void * __ptr64> (07FF6526BED16h)  
.............. gave up unwrapping this
.........   }
......... 00007FF652DFB633  lea         rsp,[rbp+0E8h]  
......... 00007FF652DFB63A  pop         rdi  
......... 00007FF652DFB63B  pop         rbp  
......... 00007FF652DFB63C  ret  

....            _STD forward<_Types>(_Args)...));
....        }
.... 00007FF652DFFF4D  lea         rsp,[rbp+0F8h]  
.... 00007FF652DFFF54  pop         rdi  
.... 00007FF652DFFF55  pop         rbp  
.... 00007FF652DFFF56  ret  

        }
00007FF65304B8B0  lea         rsp,[rbp+0E8h]  
00007FF65304B8B7  pop         rdi  
00007FF65304B8B8  pop         rbp  
00007FF65304B8B9  ret

I actually gave up unwrapping this, there's so much more.

Arguably a bunch of that overhead has to do with Visual Studio eager debug feature. So I wanted to see if my macro to disable some debug feature would work at the call-site:

IM_MSVC_RUNTIME_CHECKS_OFF // before function
...
                if (ImGui::MenuItem(buf.c_str(), NULL, false, open_source_available))
                    e->IO.SrcFileOpenFunc(test->SourceFile, test->SourceLine, e->IO.SrcFileOpenUserData);
IM_MSVC_RUNTIME_CHECKS_RESTORE // after function

Unfortunately it doesn't work and the result is pretty much the same.

So merely adding std::function<> would wreck debug mode performances for anyone even using raw functions. And that's without even any capturing happening. Honestly it is unfortunately so bad I would be hesitant to even add it in dear imgui for our rarely called callback.

I think we need a way to clamp down on this overhead.

Note that Optimized/Release mode VS2022 although still slower is rather acceptable:

                    e->IO.SrcFileOpenFunc(test->SourceFile, test->SourceLine, e->IO.SrcFileOpenUserData);
00007FF7461ED789  mov         rax,qword ptr [r15+88h]  
00007FF7461ED790  mov         qword ptr [rbp-40h],rax  
00007FF7461ED794  mov         eax,dword ptr [r13+20h]  
00007FF7461ED798  mov         dword ptr [rbp-80h],eax  
00007FF7461ED79B  mov         rax,qword ptr [r13+18h]  
00007FF7461ED79F  mov         qword ptr [rbp-38h],rax  
00007FF7461ED7A3  mov         rcx,qword ptr [rbx]  
00007FF7461ED7A6  test        rcx,rcx  
00007FF7461ED7A9  je          ShowTestGroup+17F1h (07FF7461EE2D1h)  
00007FF7461ED7AF  mov         rax,qword ptr [rcx]  
00007FF7461ED7B2  lea         r9,[rbp-40h]  
00007FF7461ED7B6  lea         r8,[rbp-80h]  
00007FF7461ED7BA  lea         rdx,[rbp-38h]  
00007FF7461ED7BE  call        qword ptr [rax+10h]  

    _Rx _Do_call(_Types&&... _Args) override { // call wrapped function
00007FF7461606C0  mov         rax,r8  
00007FF7461606C3  mov         r10,rdx  
        return _Invoker_ret<_Rx>::_Call(_Callee, _STD forward<_Types>(_Args)...);
00007FF7461606C6  mov         r8,qword ptr [r9]  

    _Rx _Do_call(_Types&&... _Args) override { // call wrapped function
00007FF7461606C9  mov         r11,rcx  
        return _Invoker_ret<_Rx>::_Call(_Callee, _STD forward<_Types>(_Args)...);
00007FF7461606CC  mov         edx,dword ptr [rax]  
00007FF7461606CE  mov         rcx,qword ptr [r10]  
00007FF7461606D1  jmp         qword ptr [r11+8]   <---- jumps to my code
ocornut commented 11 months ago

FYI i asked on Twitter https://twitter.com/ocornut/status/1709138967415640118

Is there a portable low-level trick way for callbacks in my C++ codebase to support capturing lambdas with a compile-time flag, but without relying on std::function/, or with less overhead? ( Hoped to add compile-time opt-in support for capturing lambdas, unfortunately merely wrapping a function pointer in std::function<> makes the call jumps through an unacceptable number of hoops (gave up setting after 400+ instructions, and that's without capturing anything). I don't mind those capturing users paying the cost, but I don't want raw function pointers users to be paying nearly that cost. Is there a way? (PS: Yes, that's in Visual Studio Debug build, but it is equally if not more important that Debug builds users are not impacted that much.)

But I'm not equipped to understand all the answers.

Do you expect to take ownership? move_only_function (C++23, but the implementation IIRC doesn't require core language changes) should have less overhead from not having to support copying, but it still needs to type erase the func. There is also function_ref if no ownership.

Yes! function_ref is the thing! https://open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0792r5.html Assuming you don't need to store the function w/bound args. If you need to persist bound args then you're into the std::function world. In my exp. the "function pointer with bound args for call duration" is common case.

https://github.com/TartanLlama/function_ref

https://twitter.com/basit_ayantunde/status/1709142330224046579 https://github.com/lamarrr/STX/blob/bb260d7c80ba7ada763d1d253398184e3cc430ee/include/stx/fn.h#L55

My gut intuition would be to stay away from this, but provide some helper/facilitator/docs if desired.

Eventually this "You can, but it's a hassle to implement …" https://github.com/epezent/implot/issues/445#issuecomment-1423068741 comment is much less of an hassle that we'd have it to deal with to accept std::function without wrecking perfs and compile-time further..

https://twitter.com/foonathan/status/1709172213365313758 "Store a void()() and a void. For function pointers, void is nullptr and the raw function pointer is the reinterpreted function pointer. On call, you reinterpret back to expected type. For lambdas, void stores (heap allocated copy?) lambda, the raw fn pointer is a trampoline" "The cost for non capturing lambdas is an additional branch on void = nullptr." "Alternatively, you could always store a trampoline for function pointers as well, but you can't portable store a function pointer in a void. So you'd need a union of void lambda_ptr and void (fn_ptr)(Args...) and pass the union to the trampoline."

ocornut commented 11 months ago

https://mastodon.gamedev.place/@pervognsen@mastodon.social/111170577205232966 " Alright, it sounds like these are only borrowed for the duration of the call in the usual immediate-mode fashion. You should be able to do something very simple in that case: https://cpp.godbolt.org/z/5fEj967e1 "

#include <stdio.h>

struct Callback {
    void (*func)(void *data, int arg);
    void *data;
};

void call(Callback callback, int arg) {
    callback.func(callback.data, arg);
}

template<typename F>
void unwrap_call(void *user, int arg) {
    F *f = (F *)user;
    (*f)(arg);
}

template<typename F>
Callback as_callback(F* f) {
    return (Callback) {
        .func = unwrap_call<F>,
        .data = (void *)f
    };
}

void test(Callback callback, int times) {
    for (int i = 0; i < times; i++) {
        call(callback, i);
    }
}

int main(int argc, char **argv) {
    auto lambda = [&](int i) { printf("%s\n", argv[i]); };
    test(as_callback(&lambda), argc);
}

This looks like something we could build on.

Make ImPlotGetter the equivalent of that Callback type, with one constructor that takes a raw function.

More Sane references:

nuuSolutions commented 11 months ago

Let's take the 30000ft view. What a user expects from a plot library is an interface that let's him conveniently pass values to a plot.

Certainly worlds collide here technically. That is not the point. This post was not just about "getting data through lambdas" (thanks boss!) but I was asking rather generally for a better interface. I'm sure there is pure C code that also achieves this

nuuSolutions commented 11 months ago

Respect, you changed title BEFORE my remark. Now it's my turn, though.

ocornut commented 11 months ago

Yes I have renamed the topic back, my reframing was hasty and wrong. I agree the key element here is convenience. My intuition is that some users may not be confident with the callback/void* mechanism but I sense it is probably the better mechanism, and what we can do is to improve our docs or provide little helpers. If anything I hope my zealous digging can confirm that mistrust of STL is not misguided.

BenBE commented 11 months ago

From my experience on Linux with GCC, I noticed that in most cases where the compiler is aware of all involved functions (especially with LTO) you often see the compiler more or less inlining the whole std::function<> call; sometimes even without overhead at all. This is especially the case when your code basically looks like this:

void do_something(std::function<double(int)> value_proc, int min, int max) {
    for(int x = min; x < max; x++) {
        do_plot_something(x, value_proc(x));
    }
}

void test_code() {
    double arr[10] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
    std::function<double(int)> f = [&arr](int x) {
        return arr[x];
    };

    do_something(f, 0, 10);
}

This won't usually be fully inlined in -O1, but starting with -O2 -flto you effectively get mostly the following:

void test_code() {
    for(int x = 0; x < 10; x++) {
        do_plot_something(x, arr[x]);
    }
}

From my experience, MSVC is a lot worse on those things, but GCC and Clang quite good on this front. I sometimes don't even recognize the code anymore once the optimizer did is work inlining stuff.

That said: I understand that the issue with MSVC can be a show-blocker here and having the callbacks provided with a small helper type (that transparently handles both std::function<> and plain function pointers) could be a viable way.

P.S.: With GCC there's -Og for debugging, which is basically -O2 -fno-reorder-blocks (and some).