jvcleave / ofxImGui

Use ImGui in openFrameworks
300 stars 123 forks source link

ImGui::GetClipboardText() not working for win32 #56

Closed rjx-ray closed 4 years ago

rjx-ray commented 7 years ago

GetClipboardText() returns garbage.

Lines 49 & 50 of EngineGLFW replace the imgui clipboard functions with OF ones

        io.SetClipboardTextFn = &BaseEngine::setClipboardString;
        io.GetClipboardTextFn = &BaseEngine::getClipboardString;

But they don't work. I think it's because they return strings and char * is expected, but I can't quite see it.

These default functions are set up in imgui.cpp:

    GetClipboardTextFn = GetClipboardTextFn_DefaultImpl;   // Platform dependent default implementations
    SetClipboardTextFn = SetClipboardTextFn_DefaultImpl;

If I comment out the two lines from EngineGLFW which override them everything then works well for me.

rjx-ray commented 7 years ago

The fix of using a static variable as described at https://github.com/jvcleave/ofxImGui/issues/43 also works for me. I guess that's better than using the ImGui functions?

rjx-ray commented 7 years ago

Somehow a comment I thought I had made earlier today is missing:

I found that

void ofAppGLFWWindow::setClipboardString(const string& text) {
    static string str = ofToString(text);
    ofGetWindowPtr()->setClipboardString(str.c_str());
}

in ofAppGLFWWindow.cpp is calling itself and crashing when it runs out of stack. I have reverted to using the ImGui copy/paste functions

rjx-ray commented 5 years ago

I've just updated to the current 1.65 master and this issue is still present. Its caused by a bug in BaseEngine::getClipboardString()

The code is

const char* BaseEngine::getClipboardString(void * userData)
{
     return &ofGetWindowPtr()->getClipboardString()[0];
}

However ofGetWindowPtr()->getClipboardString() returns a local string and the reference to element 0 of that is out of context on return to caller.

It can be fixed by recoding as

const char* BaseEngine::getClipboardString(void * userData)
{
    static std::string clip = ofGetWindowPtr()->getClipboardString();
    return clip.c_str();
}

Rather that making a fork for such a simple fix I've fixed it in my own app by setting io.GetClipboardTextFn equal to my own get clipboard string function as above.

rjx-ray commented 5 years ago

The above code only works once, on subsequent calls it always returns the same value regardless of new pastes to the clipboard.

That's because once the static string has been created and allocated it won’t be reallocated.

The solution is to separate the creation from the assignment


const char* ImGui::iglooGetClipboardString(void* userData) {
    static std::string clip;
    clip = ofGetWindowPtr()->getClipboardString();
    return clip.c_str();
}
SeanCodeMedia commented 4 years ago

As Richard said commenting out
//io.SetClipboardTextFn = &BaseEngine::setClipboardString; //io.GetClipboardTextFn = &BaseEngine::getClipboardString; On lines 55 and 56 did the trick. Currently I am able to copy and past to my OF application. At first I thought this was an issue with IMGUI but it turns out to be an OF issue. Don't worry about changing setClipboardString() method in src/BaseEngine.cpp I didn't have to modify the function to get my app to work. Thanks Richard

prisonerjohn commented 4 years ago

This should be fixed in master. Feel free to re-open if you're still having issues.