Provide user-defined callbacks for file operations #923

Closed
opened 2016-11-29 16:27:06 +00:00 by ldecarufel · 14 comments
ldecarufel commented 2016-11-29 16:27:06 +00:00 (Migrated from github.com)

ImGui currently uses standard library function to read/write to files. This works most of the time, but it would be useful to be able to provide our own implementation of those functions through callbacks.

In a multithreaded game engine, for example, it would be better to perform the save operations in a background thread.

Also, a game engine can implement it's own file manager, but it is currently not possible to make ImGui use it.

ImGui currently uses standard library function to read/write to files. This works most of the time, but it would be useful to be able to provide our own implementation of those functions through callbacks. In a multithreaded game engine, for example, it would be better to perform the save operations in a background thread. Also, a game engine can implement it's own file manager, but it is currently not possible to make ImGui use it.
ocornut commented 2016-11-29 19:50:48 +00:00 (Migrated from github.com)

The only file operation that may happen past initialisation is saving the .ini file. Do you really need that to go through an asynchronous system?

I don't know of a game hardware that can run ImGui where fopen()/fread() doesn't work given the right filename. Do you have something in mind?

The only file operation that may happen past initialisation is saving the .ini file. Do you really need that to go through an asynchronous system? I don't know of a game hardware that can run ImGui where fopen()/fread() doesn't work given the right filename. Do you have something in mind?
ocornut commented 2016-11-29 19:55:37 +00:00 (Migrated from github.com)

PS: Not trying to dodge the request but we've got a million todos and although I can see the desire for that I need to assert that it is insert a strict blocker to not have those things available.

If periodic blocking write of the .ini file are indeed a problem, we could e.g. provide a function to explicitly trigger .ini file writing which would serve as a workaround that is trivial to implement. For development/tooling my intuition is that an occasional xx ms spike while saving to disk isn't a real issue.

PS: Not trying to dodge the request but we've got a million todos and although I can see the desire for that I need to assert that it is insert a strict blocker to not have those things available. If periodic blocking write of the .ini file are indeed a problem, we could e.g. provide a function to explicitly trigger .ini file writing which would serve as a workaround that is trivial to implement. For development/tooling my intuition is that an occasional xx ms spike while saving to disk isn't a real issue.
ratchetfreak commented 2016-11-29 20:13:14 +00:00 (Migrated from github.com)

IMO making sure that every function that requires reading a file has an alternative that can just take a blob of memory as the file's contents is fine. (I believe that is already the case)

Then maybe a callback for when the file needs to be written out. (the default will just write the .ini file) which would allow user code to copy to a local buffer (if necessary) and then dispatch a async file write.

IMO making sure that every function that requires reading a file has an alternative that can just take a blob of memory as the file's contents is fine. (I believe that is already the case) Then maybe a callback for when the file needs to be written out. (the default will just write the .ini file) which would allow user code to copy to a local buffer (if necessary) and then dispatch a async file write.
ocornut commented 2016-11-29 20:26:00 +00:00 (Migrated from github.com)

I agree about this in theory. In practice I need to manage my time along with the scope/complexity/maintainability of this project and I wonder if asynchronous saving of the .ini file is really a real world problem that needs fixing.

I agree about this in theory. In practice I need to manage my time along with the scope/complexity/maintainability of this project and I wonder if asynchronous saving of the .ini file is really a real world problem that needs fixing.
ldecarufel commented 2016-11-29 20:38:46 +00:00 (Migrated from github.com)

It's mostly the continuous saving that's potentially problematic. I'm on a multi-threaded engine and I'd like to avoid writing to disk inside the ImGui update. I'd prefer to cache the data in a memory buffer and save it asynchronously in another thread.

On Windows the write operation is probably cached in memory, but on PS4 or XboxOne it may be written to disk right away.

Anyway, this is not a blocking issue right now, but I thought it would help make ImGui more portable!

It's mostly the continuous saving that's potentially problematic. I'm on a multi-threaded engine and I'd like to avoid writing to disk inside the ImGui update. I'd prefer to cache the data in a memory buffer and save it asynchronously in another thread. On Windows the write operation is probably cached in memory, but on PS4 or XboxOne it may be written to disk right away. Anyway, this is not a blocking issue right now, but I thought it would help make ImGui more portable!
ocornut commented 2016-11-29 21:01:14 +00:00 (Migrated from github.com)

Some notes:

  • The writing interval can be adjusted by setting io.IniSavingRate and typically it only happens if a window has been moved and the timer is continuously reset while moving. So with default settings, after a series of windows manipulations it would write the positions to disk 5 seconds after the last once, and only once.

  • You can set io.IniFilename to NULL to disable .ini writing. So a mediocre workaround if you still need .ini but want to disable writing during gameplay is to change this variable at rnutime.

  • Having just looked at the code now, I have fixed a glaring bug that would mark settings as dirty when clicking on a window's void but not moving the mouse. As I add functions to dump the .ini file to memory I might as well compare hashes before attempting any disk write.

  • At least on PS4 with file writes hosted on the remote PC I haven't had noticeable issue but I appreciate this may differ with different setup and you are correct it should be improved.

  • FYI The comments next to those functions are dated from the day I hastily wrote them five seconds before releasing the first imgui version publicly, and they say:

// Zero-tolerance, poor-man .ini parsing
// FIXME: Write something less rubbish

Which probably means they need some love :)
Will look at it!

Some notes: - The writing interval can be adjusted by setting `io.IniSavingRate` and typically it only happens if a window has been moved and the timer is continuously reset while moving. So with default settings, after a series of windows manipulations it would write the positions to disk 5 seconds after the last once, and only once. - You can set `io.IniFilename` to NULL to disable .ini writing. So a mediocre workaround if you still need .ini but want to disable writing during gameplay is to change this variable at rnutime. - Having just looked at the code now, I have fixed a glaring bug that would mark settings as dirty when clicking on a window's void but not moving the mouse. As I add functions to dump the .ini file to memory I might as well compare hashes before attempting any disk write. - At least on PS4 with file writes hosted on the remote PC I haven't had noticeable issue but I appreciate this may differ with different setup and you are correct it should be improved. - FYI The comments next to those functions are dated from the day I hastily wrote them five seconds before releasing the first imgui version publicly, and they say: ``` // Zero-tolerance, poor-man .ini parsing // FIXME: Write something less rubbish ``` Which probably means they need some love :) Will look at it!
JWki commented 2017-01-13 12:45:26 +00:00 (Migrated from github.com)

I was about to submit the same issue but also complement it with a possible pull request - I've recently had to add a callback mechanism to the built in serialization system in order to be able to write imgui layout to a cookie when running in an emscripten compiled web app. My implementation allows to register two callbacks in the IO structure: One to control where the .ini is read from, and one to control how to write it.
This works quite well for me and I could imagine it working well for others, if there's interest I can clean up my implementation and prepare a pull request.

I was about to submit the same issue but also complement it with a possible pull request - I've recently had to add a callback mechanism to the built in serialization system in order to be able to write imgui layout to a cookie when running in an emscripten compiled web app. My implementation allows to register two callbacks in the IO structure: One to control where the .ini is read from, and one to control how to write it. This works quite well for me and I could imagine it working well for others, if there's interest I can clean up my implementation and prepare a pull request.
ocornut commented 2017-01-14 12:59:01 +00:00 (Migrated from github.com)

I will implement a solution for it. If you have some code feel free to post the patch/PR for reference/ideas only if that is trivial for you to do. I'm not sure adding new callbacks in the IO structure is the best idea unless you actually need to override fopen() also during initialization for the FONTS, but I yet have to hear a request for that. My suggested solution would be to make Settings related functions public to allow to access raw buffers, and provide optional hints/info to the user about when to save it perhaps.

I will implement a solution for it. If you have some code feel free to post the patch/PR for reference/ideas only if that is trivial for you to do. I'm not sure adding new callbacks in the IO structure is the best idea unless you actually need to override fopen() also during initialization for the _FONTS_, but I yet have to hear a request for that. My suggested solution would be to make Settings related functions public to allow to access raw buffers, and provide optional hints/info to the user about when to save it perhaps.
thedmd commented 2017-01-22 18:39:27 +00:00 (Migrated from github.com)

I will share my solution with you. I added callback for saving and loading settings from user context.

struct ImGuiIO
{
    ...

    // Optional: save or load configuration.
    void        (*SaveIniCb)(const char* buffer, size_t size);
    size_t      (*LoadIniCb)(char* buffer, size_t size);

I imported changes from my local copy to settings-callbacks branch.

Example usage:

    static std::string mySettings;

    ImGuiIO& io = ImGui::GetIO();
    io.SaveIniCb = [](const char* buffer, size_t size)
    {
        mySettings.assign(buffer, size);
    };

    io.LoadIniCb = [](char* buffer, size_t size) -> size_t
    {
        if (buffer)
            memcpy(buffer, mySettings.data(), size);
        return mySettings.size();
    };

Save callback is straightforward, it receive a text buffer and size to save.
Load callback is called twice, first time with buffer set to NULL to determine size, second time with a buffer to read content.

Edit:
If there is an interest in this solution I will create PR compatible with upstream master branch. Please let me know.

I will share my solution with you. I added callback for saving and loading settings from user context. ```cpp struct ImGuiIO { ... // Optional: save or load configuration. void (*SaveIniCb)(const char* buffer, size_t size); size_t (*LoadIniCb)(char* buffer, size_t size); ``` I imported changes from my local copy to [settings-callbacks](https://github.com/thedmd/imgui/commit/38c20c57327a3fa9a4ca74becf108138b606ea77) branch. Example usage: ```cpp static std::string mySettings; ImGuiIO& io = ImGui::GetIO(); io.SaveIniCb = [](const char* buffer, size_t size) { mySettings.assign(buffer, size); }; io.LoadIniCb = [](char* buffer, size_t size) -> size_t { if (buffer) memcpy(buffer, mySettings.data(), size); return mySettings.size(); }; ``` Save callback is straightforward, it receive a text buffer and size to save. Load callback is called twice, first time with buffer set to `NULL` to determine size, second time with a buffer to read content. Edit: If there is an interest in this solution I will create PR compatible with upstream master branch. Please let me know.
ocornut commented 2017-01-22 19:31:35 +00:00 (Migrated from github.com)

Thanks @thedmd!
I can see the branch no need to make a PR right now! As discussed above the only different I'm thinking of is to avoid adding callbacks and just make those explicit somehow? (User can call a func load settings / save settings + somehow figure out the dirty state).

Thanks @thedmd! I can see the branch no need to make a PR right now! As discussed above the only different I'm thinking of is to avoid adding callbacks and just make those explicit somehow? (User can call a func load settings / save settings + somehow figure out the dirty state).
ocornut commented 2017-01-22 19:32:40 +00:00 (Migrated from github.com)

(It might be useful for others interested in this to make a PR actually, as I likely won't tackle this, however simple it is, until I finish shipping my game).

(It might be useful for others interested in this to make a PR actually, as I likely won't tackle this, however simple it is, until I finish shipping my game).
thedmd commented 2017-01-22 20:31:18 +00:00 (Migrated from github.com)

You're welcome. I will prepare a PR link it there shortly.

I want to minimize number of modifications I make in ImGui code. Callback seems to be simpler than introducing some kind of string management and got the job done for me.

I thought about exposing Load/Save function in public API. Settings loading is a part of initialization and I didn't wanted to change internals. Callback avoid problems like calling Load in a middle of the frame, resolving conflicts or duplicates. A function to queue settings save/load at the end of the frame however can be convenient.

You're welcome. I will prepare a PR link it there shortly. I want to minimize number of modifications I make in ImGui code. Callback seems to be simpler than introducing some kind of string management and got the job done for me. I thought about exposing Load/Save function in public API. Settings loading is a part of initialization and I didn't wanted to change internals. Callback avoid problems like calling Load in a middle of the frame, resolving conflicts or duplicates. A function to queue settings save/load at the end of the frame however can be convenient.
thedmd commented 2017-01-22 21:11:31 +00:00 (Migrated from github.com)

#993 there is a PR

I had to remove 'ToDisk' suffix from function name and filename parameter because they didn't make sense in context of changes.

#993 there is a PR I had to remove 'ToDisk' suffix from function name and filename parameter because they didn't make sense in context of changes.
ocornut commented 2018-05-07 20:39:02 +00:00 (Migrated from github.com)

New API

// Settings/.Ini Utilities
// The disk functions are automatically called if io.IniFilename != NULL (default is "imgui.ini"). 
// Set io.IniFilename to NULL to load/save manually. Read io.WantSaveIniSettings description about handling .ini saving manually.
void          LoadIniSettingsFromDisk(const char* ini_filename);                  // call after CreateContext() and before the first call to NewFrame(). NewFrame() automatically calls LoadIniSettingsFromDisk(io.IniFilename).
void          LoadIniSettingsFromMemory(const char* ini_data, size_t ini_size=0); // call after CreateContext() and before the first call to NewFrame() to provide .ini data from your own data source.
void          SaveIniSettingsToDisk(const char* ini_filename);
const char*   SaveIniSettingsToMemory(size_t* out_ini_size = NULL);               // return a zero-terminated string with the .ini data which you can save by your own mean. call when io.WantSaveIniSettings is set, then save data by your own mean and clear io.WantSaveIniSettings. 

The disk function are not really necessary to expose, but I think it makes the API more consistent and obvious to have them (they already existed in the private space). The Disk functions are automatically called when io.IniFilename != NULL.

If you want to save manually, you can poll the io.WantSaveIniSettings flag:

bool   WantSaveIniSettings;  // When manual .ini load/save is active (io.IniFilename == NULL), this will be set to notify your application that you can call SaveIniSettingsToMemory() and save yourself. IMPORTANT: You need to clear io.WantSaveIniSettings yourself.

This should hopefully solve all the discussed use cases (and without callbacks :)
@ldecarufel @thedmd @JWki @ratchetfreak If you have time to check this out let me know if it works for you. Thanks!

New API ```cpp // Settings/.Ini Utilities // The disk functions are automatically called if io.IniFilename != NULL (default is "imgui.ini"). // Set io.IniFilename to NULL to load/save manually. Read io.WantSaveIniSettings description about handling .ini saving manually. void LoadIniSettingsFromDisk(const char* ini_filename); // call after CreateContext() and before the first call to NewFrame(). NewFrame() automatically calls LoadIniSettingsFromDisk(io.IniFilename). void LoadIniSettingsFromMemory(const char* ini_data, size_t ini_size=0); // call after CreateContext() and before the first call to NewFrame() to provide .ini data from your own data source. void SaveIniSettingsToDisk(const char* ini_filename); const char* SaveIniSettingsToMemory(size_t* out_ini_size = NULL); // return a zero-terminated string with the .ini data which you can save by your own mean. call when io.WantSaveIniSettings is set, then save data by your own mean and clear io.WantSaveIniSettings. ``` The disk function are not _really_ necessary to expose, but I think it makes the API more consistent and obvious to have them (they already existed in the private space). The Disk functions are automatically called when `io.IniFilename != NULL`. If you want to save manually, you can poll the `io.WantSaveIniSettings` flag: ```cpp bool WantSaveIniSettings; // When manual .ini load/save is active (io.IniFilename == NULL), this will be set to notify your application that you can call SaveIniSettingsToMemory() and save yourself. IMPORTANT: You need to clear io.WantSaveIniSettings yourself. ``` This should hopefully solve all the discussed use cases (and without callbacks :) @ldecarufel @thedmd @JWki @ratchetfreak If you have time to check this out let me know if it works for you. Thanks!
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ocornut/imgui#923
No description provided.