Cleanup #219
Comments
|
i think the size of imgui.h/cpp is not a concern. i guess right now it is not about to grow in a huge way, so i would say it is not worth splitting it up in multiple files (just my 2c) |
|
A big thing for me personally is making sure it's absolutely trivial to make a part of an existing project/codebase. Preferably, the way it is right now where there is basically no build process at all. I really like being able to just update with: And building by basically just doing a unity build (although I don't do it on the game I'm working on because of poor choices made early on, my personal projects all do unity builds and its great that your library is easy to use in such a manner!). If you decide to split things off into multiple files, not such a big deal as long as I can keep doing unity builds; I think it will keep everybody's lives simpler! Dealing with build systems is literally the worst... |
|
I think at least moving the demo windows into its own file would make sense, that way one can leave them out from the compile process and it's easier to find the demo code for learning the API. |
|
I know it sounds obvious to split the example code out, but I strongly feel like people need NOT to be encouraged to compile-out the demo window. If it's in its own file the temptation is higher to just remove it from your "project". Then it become unavailable to you and everyone else in a team. It's useful as reference and also to just be able to run it any time. There's a least 5 or 6 references to ShowTestWindow scattered in the comments + more in the example application and README so hopefully people know where it is. If anything, the demo code is a strong contender to go in another file obviously but I would like to set it up in a way that it is likely to stay in the build. There is a #define to compile it out btw but that's an explicit act to remove it that rather than requiring an explicit act to add it to your project. |
|
Ah alright, haven't noticed the define yet. It's also not that big of deal to me, the linker and dead code elimination should take care of not linking the code in the final executable if it is not called :) |
|
I know the example makes good use of the API, but supporting better docs eg. through Doxygen would be extra-nice. |
|
Personally I would recommend splitting imgui into several source files for On Tue, May 12, 2015 at 11:01 AM, Andre Weissflog [email protected]
|
|
hypernewbie: Yes. Although my plan for documentation is less of a doxygen thing, but more of an illustrated guide with source code on one side and screenshot or animated gif on the other side. Categorized, showing lots of ideas. jarikomppa: I have no issue for development myself but imgui.cpp could be reorganized, better indexed. In particular all the private stuff that aren't declared in imgui.h are probably less organized than the rest. Having two versions would be additional bureaucracy and confusion (not mentioning the effect on git log/blame). |
|
omar: I refer to the sqlite distribution - you can add it as hundred files On Tue, May 12, 2015 at 11:34 AM, omar [email protected] wrote:
|
|
I believe gtest does this too, they have the impl in many files but when you include it in your own project or compile to a static lib you only include one gtest_all.cc file which effectively is a unity build. |
|
Another option (that I currently employ) is to create a .h that includes all necessary .h and a .cpp that includes all necessary .cpp.
The only difference with the current setup is that you must copy N files instead of 2, but I assume that a person that can code C++ knows how to copy more than 2 files in his source code directory ;-) FYI there is a library that employs this approach for structuring itself in self contained "modules". Each "module" is nothing more than a .cpp that includes all necessary files for a unity build and a .h that includes all headers without the need of header paths or other fancy compiler settings. https://github.com/julianstorer/JUCE/tree/master/modules/juce_core It also has a very nice and simple build generator (visual) tool called Introjucer that can take those modules, together with a super simple .json file that lists all files to be included and generates native ide projects for almost every compiler/platform you like. Example of json file: https://github.com/julianstorer/JUCE/blob/master/modules/juce_core/juce_module_info Shot of introjucer Auto generating build files is probably not worth the effort for just one example on many platforms. |
|
The main question is mostly to ask IF ImGui needs to be split into multiple files, and HOW which I don't have a trivial answer for. How they are actually compiled is a smaller detail. At the moment I was most looking forward to work on simplifying the source code rather than splitting files. I didn't even realize that JUCE what open-source (unfortunately the free version it is GPL). I have bookmarked a few JUCE pages for reference of features I'd like to implement. I mean just look at https://www.juce.com/api/classSlider.html I was actually thinking of auto generated build files for the examples. So strictly speaking not part of core imgui, but we need it to be able to ship example for different high-level libraries that are optional. Right now there's a single .sln for visual studio so we can't easily have optional projects there. That's a separate topic to the cleanup. |
|
Oh, sure, I'm totally with you, simplifying the source code is way more valuable than splitting files! Coming back in topic, unfortunately I've not spent time to study current IMGUI source code to give a specific hint or a pull request for the cleanup but these are the very first steps that I do for cleanup/refactoring:
If you want build files for examples, we could use Introjucer for generating them! my 2 cents! |
|
Honestly, I'm a huge fan if the way you've been writing the code so far. Although it's largely a matter of taste, there's something to be said about the simplicity you've already managed to achieve/maintain in the code. I rather enjoy not having to hunt through various files to see where the implementation of something is, and as a person who edits code in emacs with very little additional assistance, I find this to be a pretty big win (although, emacs dies a little sometimes with the large imgui.cpp in cpp mode). Seems better to me to leave the implementation single file for now. But something I would like some consideration on is splitting the header file (or at least rearrangement). I haven't looked very carefully at the dependencies on definitions, but I feel like you could reorg the header in such a way that somebody could look at the beginning of just one file that held the 90% case (window setup, widgets, input control, etc) and then throw in some of the more advanced feature API into another header (or further down in the header). This would make it easier for those new to the library to just look at one place that will get them the basics done quickly and if they want to branch out into more custom things, they can look elsewhere which has the rest. So more concretely, we have the ImGui namespace starting in the header file at line Line 155 in b6f3c97 Also, the 90% case: Lines 169 to 175 in b6f3c97 Lines 275 to 362 in b6f3c97 Lines 385 to 390 in b6f3c97 Lines 398 to 406 in b6f3c97 I find myself using these the most, by far. The organization you already have I'm a very big fan of (grouping up by category), so splitting them off more and moving them is probably a loss, but I think at least putting the namespace at the beginning of the file would be awesome. |
|
imgui.h is already designed to show the main stuff as high as possible, but..For some reason I was convinced that ImVector<> needed to be at the top for some reason, however it appears that it isn't necessary (maybe it was in 1.0?), so I moved it below the main namespace in da53caf. The main namespace now starts at line 90. Thanks! I'm not sure we can squeeze it more. It's sort of debatable which should are the top most used functions. I think few people uses the input functions (IsKeyDown, etc.) and the other utilities you highlighted such as IsItemHovered() aren't that used in the simplest use case. And we could argue that GetIO/NewFrame/Render etc. even though they aren't useful once you are already setup, it's very valuable to see them at the top the first time. So I wouldn't really know how to re-order that block. Perhaps everything after the blocks of Begin/End/BeginChild/EndChild could go after the widgets? I think it may look less structured this way? |
|
Yeah it's definitely tough, you can't pick to optimize everything for everyone... But all things considered, I think the overall order within the namespace is actually quite good. The only way I can think of to bring the namespace closer to the top is to pull it out into another file but I'm not sure that's a good enough reason for a whole extra file. Or maybe put the section of includes, forward decls and typedefs into their own header and include it into imgui.h (again, a whole extra file) to compress down the text that appears before the namespace. Dale Kim
|
|
I have made a first local (uncommitted) attempt a splitting imgui.cpp into several files just to see the issues I would get into. Not really happy with that. imgui.cpp still too big. splitting up e.g. imgui_textedit.cpp not really worth it considering it requires dragging lots of things out. Feeling with with not having one .h per .cpp. I don't have an obvious natural way of splitting imgui.cpp further, there's no oop style stuff left in imgui.cpp that are natural groups of important size. I could split it but it'd be very either very arbitrary, either it would have to be split in very small files (which to me is undesirable). I think imgui_demo.cpp is the most obvious thing to separate (as mentioned above I am only reluctant to split it because I don't want people to compile it out). My idea right now is that will revert most of that and want to focus on building a cleaner, near exhaustive version of imgui_internal.h. The effects of that would be:
So my intuition is that we will end up with I will intentionally leave some dependencies between imgui.cpp and imgui_demo.cpp to ensure it's not compiled out :) |
|
More musing. Tried the above and not satisfied either. I can't take a imgui_demo.cpp out without introducing a imgui_internal.h because the earlier uses several simple helpers. They are all generic helpers like ImStricmp, etc. It would make some sense to bother rewriting that function without those helpers because ShowTestWindow() being demo code it's preferable to allow it to be copy & pasted. Here's the full list used in ShowTestWindow(). Here's a list of all the generic helpers currently in imgui.cpp // Helpers: String
IMGUI_API int ImStricmp(const char* str1, const char* str2);
IMGUI_API int ImStrnicmp(const char* str1, const char* str2, int count);
IMGUI_API char* ImStrdup(const char* str);
IMGUI_API size_t ImStrlenW(const ImWchar* str);
IMGUI_API const ImWchar* ImStrbolW(const ImWchar* buf_mid_line, const ImWchar* buf_begin); // Find beginning-of-line
IMGUI_API const char* ImStristr(const char* haystack, const char* needle, const char* needle_end);
IMGUI_API size_t ImFormatString(char* buf, size_t buf_size, const char* fmt, ...);
IMGUI_API size_t ImFormatStringV(char* buf, size_t buf_size, const char* fmt, va_list args);
// Helpers: Misc
IMGUI_API ImU32 ImHash(const void* data, size_t data_size, ImU32 seed); // Currently CRC32
IMGUI_API bool ImLoadFileToMemory(const char* filename, const char* file_open_mode, void** out_file_data, size_t* out_file_size, size_t padding_bytes = 0);
static inline int ImUpperPowerOfTwo(int v) { v--; v |= v >> 1; v |= v >> 2; v |= v >> 4; v |= v >> 8; v |= v >> 16; v++; return v; }
static inline bool ImCharIsSpace(int c) { return c == ' ' || c == '\t' || c == 0x3000; }
// Helpers: UTF-8 <> wchar
IMGUI_API int ImTextCharToUtf8(char* buf, size_t buf_size, unsigned int in_char); // return output UTF-8 bytes count
IMGUI_API ptrdiff_t ImTextStrToUtf8(char* buf, size_t buf_size, const ImWchar* in_text, const ImWchar* in_text_end); // return output UTF-8 bytes count
IMGUI_API int ImTextCharFromUtf8(unsigned int* out_char, const char* in_text, const char* in_text_end); // return input UTF-8 bytes count
IMGUI_API ptrdiff_t ImTextStrFromUtf8(ImWchar* buf, size_t buf_size, const char* in_text, const char* in_text_end, const char** in_remaining = NULL); // return input UTF-8 bytes count
IMGUI_API int ImTextCountCharsFromUtf8(const char* in_text, const char* in_text_end); // return number of UTF-8 code-points (NOT bytes count)
IMGUI_API int ImTextCountUtf8BytesFromChar(unsigned int in_char); // return output UTF-8 bytes count
IMGUI_API int ImTextCountUtf8BytesFromStr(const ImWchar* in_text, const ImWchar* in_text_end); // return number of bytes to express string as UTF-8 code-points
// Helpers: Maths
static inline int ImMin(int lhs, int rhs) { return lhs < rhs ? lhs : rhs; }
static inline int ImMax(int lhs, int rhs) { return lhs >= rhs ? lhs : rhs; }
static inline float ImMin(float lhs, float rhs) { return lhs < rhs ? lhs : rhs; }
static inline float ImMax(float lhs, float rhs) { return lhs >= rhs ? lhs : rhs; }
static inline ImVec2 ImMin(const ImVec2& lhs, const ImVec2& rhs) { return ImVec2(ImMin(lhs.x,rhs.x), ImMin(lhs.y,rhs.y)); }
static inline ImVec2 ImMax(const ImVec2& lhs, const ImVec2& rhs) { return ImVec2(ImMax(lhs.x,rhs.x), ImMax(lhs.y,rhs.y)); }
static inline int ImClamp(int v, int mn, int mx) { return (v < mn) ? mn : (v > mx) ? mx : v; }
static inline float ImClamp(float v, float mn, float mx) { return (v < mn) ? mn : (v > mx) ? mx : v; }
static inline ImVec2 ImClamp(const ImVec2& f, const ImVec2& mn, ImVec2 mx) { return ImVec2(ImClamp(f.x,mn.x,mx.x), ImClamp(f.y,mn.y,mx.y)); }
static inline float ImSaturate(float f) { return (f < 0.0f) ? 0.0f : (f > 1.0f) ? 1.0f : f; }
static inline float ImLerp(float a, float b, float t) { return a + (b - a) * t; }
static inline ImVec2 ImLerp(const ImVec2& a, const ImVec2& b, const ImVec2& t) { return ImVec2(a.x + (b.x - a.x) * t.x, a.y + (b.y - a.y) * t.y); }
static inline float ImLengthSqr(const ImVec2& lhs) { return lhs.x*lhs.x + lhs.y*lhs.y; }
static inline float ImLengthSqr(const ImVec4& lhs) { return lhs.x*lhs.x + lhs.y*lhs.y + lhs.z*lhs.z + lhs.w*lhs.w; }
IMGUI_API bool ImIsPointInTriangle(const ImVec2& p, const ImVec2& a, const ImVec2& b, const ImVec2& c);
// Helpers: Vector Maths operators
// Disabled by default so user can include imgui_internal.h and still use the IM_VEC2_CLASS_EXTRA/IM_VEC4_CLASS_EXTRA macros to define conversions from ImGui types to their native types.
#ifdef IMGUI_DEFINE_MATH_OPERATORS
// We are keeping those static in the .cpp file so as not to leak them outside, in the case the user has implicit cast operators between ImVec2 and its own types.
static inline ImVec2 operator*(const ImVec2& lhs, const float rhs) { return ImVec2(lhs.x*rhs, lhs.y*rhs); }
static inline ImVec2 operator/(const ImVec2& lhs, const float rhs) { return ImVec2(lhs.x/rhs, lhs.y/rhs); }
static inline ImVec2 operator+(const ImVec2& lhs, const ImVec2& rhs) { return ImVec2(lhs.x+rhs.x, lhs.y+rhs.y); }
static inline ImVec2 operator-(const ImVec2& lhs, const ImVec2& rhs) { return ImVec2(lhs.x-rhs.x, lhs.y-rhs.y); }
static inline ImVec2 operator*(const ImVec2& lhs, const ImVec2 rhs) { return ImVec2(lhs.x*rhs.x, lhs.y*rhs.y); }
static inline ImVec2 operator/(const ImVec2& lhs, const ImVec2 rhs) { return ImVec2(lhs.x/rhs.x, lhs.y/rhs.y); }
static inline ImVec2& operator+=(ImVec2& lhs, const ImVec2& rhs) { lhs.x += rhs.x; lhs.y += rhs.y; return lhs; }
static inline ImVec2& operator-=(ImVec2& lhs, const ImVec2& rhs) { lhs.x -= rhs.x; lhs.y -= rhs.y; return lhs; }
static inline ImVec2& operator*=(ImVec2& lhs, const float rhs) { lhs.x *= rhs; lhs.y *= rhs; return lhs; }
static inline ImVec2& operator/=(ImVec2& lhs, const float rhs) { lhs.x /= rhs; lhs.y /= rhs; return lhs; }
static inline ImVec4 operator-(const ImVec4& lhs, const ImVec4& rhs) { return ImVec4(lhs.x-rhs.x, lhs.y-rhs.y, lhs.z-rhs.z, lhs.w-lhs.w); }
#endifAnd list of most of the remaining private/static functions of imgui,cpp. // Render
void RenderText(ImVec2 pos, const char* text, const char* text_end = NULL, bool hide_text_after_hash = true);
void RenderTextWrapped(ImVec2 pos, const char* text, const char* text_end, float wrap_width);
void RenderTextClipped(const ImVec2& pos_min, const ImVec2& pos_max, const char* text, const char* text_end, const ImVec2* text_size_if_known, ImGuiAlign align = ImGuiAlign_Default, const ImVec2* clip_min = NULL, const ImVec2* clip_max = NULL);
void RenderFrame(ImVec2 p_min, ImVec2 p_max, ImU32 fill_col, bool border = true, float rounding = 0.0f);
void RenderCollapseTriangle(ImVec2 p_min, bool opened, float scale = 1.0f, bool shadow = false);
void RenderCheckMark(ImVec2 pos, ImU32 col);
// Window
inline ImGuiWindow* GetCurrentWindow();
inline void SetCurrentWindow(ImGuiWindow* window);
inline ImGuiWindow* GetParentWindow();
ImGuiWindow* FindHoveredWindow(ImVec2 pos, bool excluding_childs)
ImGuiWindow* FindWindowByName(const char* name)
ImGuiWindow* CreateNewWindow(const char* name, ImVec2 size, ImGuiWindowFlags flags)
void SetWindowScrollY(ImGuiWindow* window, float new_scroll_y)
void SetWindowPos(ImGuiWindow* window, const ImVec2& pos, ImGuiSetCond cond)
void SetWindowSize(ImGuiWindow* window, const ImVec2& size, ImGuiSetCond cond)
void SetWindowCollapsed(ImGuiWindow* window, bool collapsed, ImGuiSetCond cond)
inline bool IsWindowContentHoverable(ImGuiWindow* window)
void ClearSetNextWindowData()
void FocusWindow(ImGuiWindow* window)
void CheckStacksSize(ImGuiWindow* window, bool write)
void SetActiveId(ImGuiID id, ImGuiWindow* window = NULL);
void RegisterAliveId(ImGuiID id);
ImVector<ImGuiStorage::Pair>::iterator LowerBound(ImVector<ImGuiStorage::Pair>& data, ImU32 key);
void PushClipRect(const ImVec4& clip_rect, bool clipped = true)
void PopClipRect()
ImVec4 GetVisibleRect()
// Settings
ImGuiIniData* FindWindowSettings(const char* name)
ImGuiIniData* AddWindowSettings(const char* name)
void LoadSettings()
void SaveSettings()
void MarkSettingsDirty()
void LogText(const ImVec2& ref_pos, const char* text, const char* text_end)
float CalcWrapWidthForPos(const ImVec2& pos, float wrap_pos_x)
const char* FindTextDisplayEnd(const char* text, const char* text_end = NULL)
bool IsPopupOpen(ImGuiID id)
void CloseInactivePopups()
ImGuiWindow* GetFrontMostModalRootWindow()
void ClosePopupToLevel(int remaining)
void ClosePopup(ImGuiID id)
bool BeginPopupEx(const char* str_id, ImGuiWindowFlags extra_flags)
ImVec2 FindBestPopupWindowPos(const ImVec2& base_pos, const ImVec2& size, ImGuiWindowFlags flags, int* last_dir, const ImRect& r_inner)
void PushMultiItemsWidths(int components, float w_full = 0.0f)
void SetFont(ImFont* font)
float* GetStyleVarFloatAddr(ImGuiStyleVar idx)
ImVec2* GetStyleVarVec2Addr(ImGuiStyleVar idx)
void Scrollbar(ImGuiWindow* window)
bool CloseWindowButton(bool* p_opened)
bool ButtonEx(const char* label, const ImVec2& size_arg = ImVec2(0,0), ImGuiButtonFlags flags = 0)
bool ButtonBehavior(const ImRect& bb, ImGuiID id, bool* out_hovered, bool* out_held, bool allow_key_modifiers, ImGuiButtonFlags flags)
bool DragBehavior(const ImRect& frame_bb, ImGuiID id, float* v, float v_speed, float v_min, float v_max, int decimal_precision, float power)
bool SliderBehavior(const ImRect& frame_bb, ImGuiID id, float* v, float v_min, float v_max, float power, int decimal_precision, bool horizontal)
bool SliderFloatAsInputText(const char* label, float* v, ImGuiID id, int decimal_precision)
inline void ParseFormatPrecision(const char* fmt, int& decimal_precision)
inline float RoundScalar(float value, int decimal_precision)
void Plot(ImGuiPlotType plot_type, const char* label, float (*values_getter)(void* data, int idx), void* data, int values_count, int values_offset, const char* overlay_text, float scale_min, float scale_max, ImVec2 graph_size)
bool SliderFloatN(const char* label, float* v, int components, float v_min, float v_max, const char* display_format, float power)
bool SliderIntN(const char* label, int* v, int components, int v_min, int v_max, const char* display_format)
bool DragFloatN(const char* label, float* v, int components, float v_speed, float v_min, float v_max, const char* display_format, float power)
bool DragIntN(const char* label, int* v, int components, float v_speed, int v_min, int v_max, const char* display_format)
bool InputFloatN(const char* label, float* v, int components, int decimal_precision, ImGuiInputTextFlags extra_flags)
bool InputIntN(const char* label, int* v, int components, ImGuiInputTextFlags extra_flags)
void InputTextApplyArithmeticOp(const char* buf, float *v)
int InputTextCalcTextLenAndLineCount(const char* text_begin, const char** out_text_end)
ImVec2 InputTextCalcTextSizeW(const ImWchar* text_begin, const ImWchar* text_end, const ImWchar** remaining = NULL, ImVec2* out_offset = NULL, bool stop_on_new_line = false)
bool InputTextFilterCharacter(unsigned int* p_char, ImGuiInputTextFlags flags, ImGuiTextEditCallback callback, void* user_data)
bool InputTextEx(const char* label, char* buf, size_t buf_size, const ImVec2& size_arg, ImGuiInputTextFlags flags, ImGuiTextEditCallback callback, void* user_data)
void ItemSize(ImVec2 size, float text_offset_y)
inline void ItemSize(const ImRect& bb, float text_offset_y)
bool ItemAdd(const ImRect& bb, const ImGuiID* id)
bool IsHovered(const ImRect& bb, ImGuiID id, bool flatten_childs = false)
bool IsClippedEx(const ImRect& bb, const ImGuiID* id, bool clip_even_when_logged)
float GetDraggedColumnOffset(int column_index)
void PushColumnClipRect(int column_index)Realistically I think this dozen below covers 98% of the use cases to create new widget with the current style. But arguably the code needs a few more helpers to make the widget code simpler. ImGuiWindow* GetCurrentWindow();
void FocusWindow(ImGuiWindow* window)
void SetActiveId(ImGuiID id, ImGuiWindow* window = NULL);
bool ButtonEx(const char* label, const ImVec2& size_arg = ImVec2(0,0), ImGuiButtonFlags flags = 0)
bool ButtonBehavior(const ImRect& bb, ImGuiID id, bool* out_hovered, bool* out_held, bool allow_key_modifiers, ImGuiButtonFlags flags)
bool DragBehavior(const ImRect& frame_bb, ImGuiID id, float* v, float v_speed, float v_min, float v_max, int decimal_precision, float power)
bool SliderBehavior(const ImRect& frame_bb, ImGuiID id, float* v, float v_min, float v_max, float power, int decimal_precision, bool horizontal)
void ItemSize(ImVec2 size, float text_offset_y)
inline void ItemSize(const ImRect& bb, float text_offset_y)
bool ItemAdd(const ImRect& bb, const ImGuiID* id)
bool IsHovered(const ImRect& bb, ImGuiID id, bool flatten_childs = false)
bool IsClippedEx(const ImRect& bb, const ImGuiID* id, bool clip_even_when_logged)So even thou introducing imgui_internal.h will be of use and I'm not against it I don't think it'll constitute much of a meaningful cleanup. If I want to "index" the content of imgui.cpp I can do that at the top of imgui.cpp Perhaps I could move ImDrawList along with ImFont and ImFontAtlas + the font data into its own file, that would be an extra 2 k. |
|
I agree that it's highly desirable that the demo code not use internal APIs, and it's a bonus if that helps with a splitting. |
|
Maybe you can have something like imgui_utils.cpp/h for your string/vector code? |
|
I can get imgui_demo.cpp out now. Not committed because if I add new files I'd rather do it all once. I thought about including the other files from imgui.cpp which would facilitate development but I can't think of a way to make it "non weird" and it may be more confusing for the user, and I don't want user to be browsing the extra files with their IDE showing them as disabled code because of some ifdef hackery. If they were called .inl it would be more compatible with build systems (more likely to show but not build directly), but I'd rather go for the normal thing, call them .cpp and require the user to add them? Less confusing. That's where I am now (A) We've got an imgui.cpp that is still 7000+ lines here (instead of 13500). (B) I think it'll go with something like either of those. The only things that's making me uncomfortable is the jump to add more .cpp files but once that's done it's done. I just want to settle on a good initial configuration so the files won't change every few months. |
|
From my point of view, the most important change is to separate the demo code to it's own file. The second important change is to separate pure drawing backend (i.e. no API) to it's own file. The rest is solely on your own decision (but as few files as possible might be a good metric for decision making). So the solution (A) would be an option, even though I myself consider |
|
I like having an imgui_internal.h file because then implementation details will not 'leak' into the main imgui.h file (or ar less likely to do so) I vote (A) |
|
Here's where I am presently: imgui.cpp 8850 lines imgui_internal.h is currently only exposing the data structure and a few helper functions but not all the private functions of imgui.cpp (less than 50, but that is the natural followup of events to add them to the .h file). Adding this file is pretty important/useful as it is enable smoother extensions even if they are sitting on unsupported API (from there we can promote bits to the public API). I'm ok with that split but imgui.cpp is still 8850 lines (compared to 13500). |
|
I guess if you want to split more then perhaps consider groups of widgets like |
|
Sliders are 430 lines including some helpers. If I go this way it's either lots of 300-800 lines files either rather arbitrary separation and personally I don't fancy either. I'm forward-declaring all the static functions of imgui.cpp at the top, help sortng things out. |
|
The two other projects I know about where they provide source files to directly add to client projects are SQLite and JUCE. Both work with hundreds of files for development and provide a single-file amalgamation. Maybe that's a good idea for ImGui, too. Jules even has a nice open-sourced amalgamation tool, I used it for my own projects. Edit: I mean, my point is that you can have two different levels of splitting, a max-level which is good for development and another one which is easy to link with. |
|
btw. @ocornut if you release this 'cleanup', could you try to limit this release to the new file structure and not mix it with various API changes ? This would simplify porting a lot :) |
|
i don't see this happening in a branch right now, is that intended ? |
|
Yes it's local until I find a configuration that I like. I'll probably release 1.44 with this and the few minor changes done since 1.43. |
|
ok great. can you please list any API changes explicitly then, to simplify porting ? |
|
Yes. They should always be explicitly listed in the "release note" log. |
|
..and you'll notice that imgui.h won't have changed at all (apart from some comment) |
|
If you are curious to have a look at this branch. imgui_internal.h is exposing
//-----------------------------------------------------------------------------
// Internal API
// No guarantee of forward compatibility here.
//-----------------------------------------------------------------------------
namespace ImGui
{
bool ItemAdd(const ImRect& bb, const ImGuiID* id);
void ItemSize(const ImVec2& size, float text_offset_y = 0.0f);
void ItemSize(const ImRect& bb, float text_offset_y = 0.0f);
bool IsClippedEx(const ImRect& bb, const ImGuiID* id, bool clip_even_when_logged);
bool IsHovered(const ImRect& bb, ImGuiID id, bool flatten_childs = false);
bool FocusableItemRegister(ImGuiWindow* window, bool is_active, bool tab_stop = true); // Return true if focus is requested
void FocusableItemUnregister(ImGuiWindow* window);
ImVec2 CalcItemSize(ImVec2 size, float default_x, float default_y);
float CalcWrapWidthForPos(const ImVec2& pos, float wrap_pos_x);
void SetActiveID(ImGuiID id, ImGuiWindow* window);
void KeepAliveID(ImGuiID id);
ImGuiWindow* GetCurrentWindow();
ImGuiWindow* GetParentWindow();
void FocusWindow(ImGuiWindow* window);
void RenderText(ImVec2 pos, const char* text, const char* text_end = NULL, bool hide_text_after_hash = true);
void RenderTextWrapped(ImVec2 pos, const char* text, const char* text_end, float wrap_width);
void RenderTextClipped(const ImVec2& pos_min, const ImVec2& pos_max, const char* text, const char* text_end, const ImVec2* text_size_if_known, ImGuiAlign align = ImGuiAlign_Default, const ImVec2* clip_min = NULL, const ImVec2* clip_max = NULL);
void RenderFrame(ImVec2 p_min, ImVec2 p_max, ImU32 fill_col, bool border = true, float rounding = 0.0f);
void RenderCollapseTriangle(ImVec2 p_min, bool opened, float scale = 1.0f, bool shadow = false);
void RenderCheckMark(ImVec2 pos, ImU32 col);
bool ButtonBehavior(const ImRect& bb, ImGuiID id, bool* out_hovered, bool* out_held, bool allow_key_modifiers, ImGuiButtonFlags flags = 0);
bool ButtonEx(const char* label, const ImVec2& size_arg = ImVec2(0,0), ImGuiButtonFlags flags = 0);
bool SliderBehavior(const ImRect& frame_bb, ImGuiID id, float* v, float v_min, float v_max, float power, int decimal_precision, bool horizontal);
bool SliderFloatN(const char* label, float* v, int components, float v_min, float v_max, const char* display_format, float power);
bool SliderIntN(const char* label, int* v, int components, int v_min, int v_max, const char* display_format);
bool DragBehavior(const ImRect& frame_bb, ImGuiID id, float* v, float v_speed, float v_min, float v_max, int decimal_precision, float power);
bool DragFloatN(const char* label, float* v, int components, float v_speed, float v_min, float v_max, const char* display_format, float power);
bool DragIntN(const char* label, int* v, int components, float v_speed, int v_min, int v_max, const char* display_format);
bool InputTextEx(const char* label, char* buf, int buf_size, const ImVec2& size_arg, ImGuiInputTextFlags flags, ImGuiTextEditCallback callback = NULL, void* user_data = NULL);
bool InputFloatN(const char* label, float* v, int components, int decimal_precision, ImGuiInputTextFlags extra_flags);
bool InputIntN(const char* label, int* v, int components, ImGuiInputTextFlags extra_flags);
bool InputFloatReplaceWidget(const ImRect& aabb, const char* label, float* v, ImGuiID id, int decimal_precision);
void PlotEx(ImGuiPlotType plot_type, const char* label, float (*values_getter)(void* data, int idx), void* data, int values_count, int values_offset, const char* overlay_text, float scale_min, float scale_max, ImVec2 graph_size);
int ParseFormatPrecision(const char* fmt, int default_value);
float RoundScalar(float value, int decimal_precision);
}The count is: imgui.cpp 8853 lines The presence of data structures and most useful functions in imgui_internal.h I believe makes imgui.cpp more approachable. Any feedback of what could be done next, or are we happy with this? |
|
This is the same as I would have been structuring it, I like it 👍 |
|
Merged in master! |
|
👍 |
|
👍 When you have sane build system, it doesn't matter do you have one file, or many files. I just upgraded ImGui in bgfx and this is what changed: bkaradzic/bgfx@437656b IMHO, only people with crazy build setup (for example, those that manually f* around with .vcproj/.sln files) prefer single header file... |
|
@bkaradzic I'm not happy with relying on the assumption of having a "sane build system". Using Visual Studio project system is still the easiest way to get started. ImGui is meant to be inclusive in the sense that you can add it to your 1-weekend experiment. Anyway this is done and we're not turning back :) I'll close this thread - not to say that there won't be more cleanup, but I think it got already way more sane! |
I'd like to have a pass of cleanup to
No specific requirement but generally anything that makes the more simpler helps.
Some metrics from the WIP menus branch:
imgui.h - 1066 LoC
imgui.cpp - 11865 LoC, including:
395 lines: comments, docs
154 lines: includes, forward declarations
481 lines: helpers
7074 lines: main code
395 lines: ImDrawList
1159 lines : ImFontAtlas, ImFont, text rendering, UTF-8 routines
308 lines : platform dependent helpers
200 lines : ShowUserGuide, ShowStyleEditor
1608 lines : ShowTestWindow + micro sample apps
336 : font data + decompressor
This thread is mainly for me noting ideas but if you have feedback please let me know. Personally I don't have a problem with imgui.cpp being so big. But it is important to factor in other people impression (people who may want to work on improving imgui) and also the "portability" of it.. I would prefer to aim to keep the code a little smaller of course, and will work on that. Splitting into more files isn't out of question either.
The text was updated successfully, but these errors were encountered: