LoadImages.hpp file

[R-Abbasi]

• The header containing two functions is included in mian.cpp but not used there.
• One of the functions is used in SideBarMenu.cpp. It sounds the other hasn't any usage. Why not removing that? (ofc, I've not loaded images with Dear ImGui, hence can't be quite sure)
• What if the header is included in other TUs too? Then you'll get a symbol redefinition linker error most supposedly.

[AKMaily]

1. The header is incorporated into the main.cpp to prevent a redefinition linker error.
2. The first function is designed for loading images while the program is rendering, whereas the second function is intended for loading images before rendering. Currently, the first function is not in use, but its necessity for future use is uncertain.

[R-Abbasi]

1- It doesn't work that way, I suppose. There should certainly be only one definition for any symbol, here your function, otherwise the linker confuses which one to choose. #include is also merely copy-paste, nothing that more. So including that header in main.cpp and using its function(s) in another translation unit then including that translation unit in main.cpp doesn't prevent any symbol redefinition error, but simply creates a couple of redundant complexities, to me. When you create a header, you should consider it being potentially included in more than one TUs, therefore the symbols there ought to normally either have the static or inline specifier to prevent the redefection error. I prefer the latter since only one of the repetitious definitions will be picked up making the executable smaller in size.

Likely something like declaring a .hpp file with that header included and SetSideBarMenu function declared in it (defined in a .cpp file) would be better. That way you'd remove worries about the error and wouldn't include a .cpp file either.

2- The required function could be moved into where it's needed leaving out the excessive one.

[AKMaily]

Do you mean if i include the LoadImages.hpp in the main.cpp and use it in two different files like SideBarMenu.cpp and Devices.cpp this will conclude in an linker error ?

[R-Abbasi]

If you want to use stuff in that header in a TU, the header should be included. So the code will be copied into it. If used in more than one TU, more than one copy-paste will be done. Then you should get a linker error because more than one definition exist. To solve this, the static (or more preferably inline) specifier is normally required for the function(s).

https://godbolt.org/z/rvjdKW1nr _[100%] Linking CXX executable main /opt/compiler-explorer/gcc-13.2.0/bin/../lib/gcc/x86_64-linux-gnu/13.2.0/../../../../x8664-linux-gnu/bin/ld: CMakeFiles/main.dir/main.cpp.o: in function foo()': main.cpp:4: multiple definition offoo()'; CMakeFiles/main.dir/source.cpp.o:/app/source.cpp:4: first defined here

[R-Abbasi]

• As well as, in Dear ImGui Wiki: Image Loading and Displaying Examples (https://github.com/ocornut/imgui/wiki/Image-Loading-and-Displaying-Examples#example-for-opengl-users) only stb_image.h is used seemingly. Where is stb_image_write.h applied in the code, please? Commenting out these two lines makes no sensible difference on my machine:

  //#define STB_IMAGE_WRITE_IMPLEMENTATION
  //#include "../stb_image/stb_image_write.h"

• LoadTextureFromFile is the function to load image files and mentioned throughout that whole page as far as I see. Why did you go for LoadTextureFromHeader, and, where did you get this from, please (although their codes resemble each other)?

[AKMaily]

• As well as, in Dear ImGui Wiki: Image Loading and Displaying Examples (https://github.com/ocornut/imgui/wiki/Image-Loading-and-Displaying-Examples#example-for-opengl-users) only stb_image.h is used seemingly. Where is stb_image_write.h applied in the code, please? Commenting out these two lines makes no sensible difference on my machine:

//#define STB_IMAGE_WRITE_IMPLEMENTATION
//#include "../stb_image/stb_image_write.h"

• LoadTextureFromFile is the function to load image files and mentioned throughout that whole page as far as I see. Why did you go for LoadTextureFromHeader, and, where did you get this from, please (although their codes resemble each other)?

I included stb_image_write.h because at the point where i used the LoadTextureFromFile method it was needed. If it does not cause an error when deleted it can be deleted i suppose.

We used LoadTextureFromHeader because two reasons:

1. the images only have to be loaded once and not every frame
2. the images do not depend on their path. Using LoadTexturefromFile the images where not displayed in the CI version because they where not found in the right path.

[AKMaily]

If you want to use stuff in that header in a TU, the header should be included. So the code will be copied into it. If used in more than one TU, more than one copy-paste will be done. Then you should get a linker error because more than one definition exist. To solve this, the static (or more preferably inline) specifier is normally required for the function(s).

https://godbolt.org/z/rvjdKW1nr _[100%] Linking CXX executable main /opt/compiler-explorer/gcc-13.2.0/bin/../lib/gcc/x86_64-linux-gnu/13.2.0/../../../../x8664-linux-gnu/bin/ld: CMakeFiles/main.dir/main.cpp.o: in function foo()': main.cpp:4: multiple definition offoo()'; CMakeFiles/main.dir/source.cpp.o:/app/source.cpp:4: first defined here

Why would you include the LoadImage.hpp twice in two different cpp files ? If it is included in the main.cpp it works in the SideBarMenu.cpp and every other file that is included afterwards. Do you mean changing the LoadImagefromHeader function to static?

[R-Abbasi]

Why would you include the LoadImage.hpp twice in two different cpp files

As mentioned earlier, a header is expected to be included in TUs. If you want to use it only in one single TU why did you create the header in the first place?

Do you mean changing the LoadImagefromHeader function to static?

Read above posts again, please.

I included stb_image_write.h because at the point where i used the LoadTextureFromFile method it was needed.

It's not required.

the images only have to be loaded once and not every frame

You're calling LoadTextureFromHeader on every frame.

Using LoadTexturefromFile the images where not displayed in the CI version because they where not found in the right path.

The CI version is not that important, supposedly. I don't know of any program sent out as a standalone executable file. Programs are sent out for users (or market) commonly as a pack comprising all dependencies needed for the executable to run correctly, such as images, sounds, .gif files, or even videos.

The stb_image.h library looks pretty much like our other external library imfilebrowser.h that we have a link to it in our repo. Loading images using that library is not that complicated but quite easy and straightforward. It's not needed to load the file(s) on each frame either. Actually only the Dear ImGui widget ImGui::Image() is in the main loop, just like all other widgets.

Here is a resizable image in our project:

[AKMaily]

Why would you include the LoadImage.hpp twice in two different cpp files

As mentioned earlier, a header is expected to be included in TUs. If you want to use it only in one single TU why did you create the header in the first place?

If the header is included in the main.cpp it works. If this is not c++ standard we can change the specifier to static or inline specifier and load the .hpp in the files where it is needed.

the images only have to be loaded once and not every frame

You're calling LoadTextureFromHeader on every frame.

In the code there is a boolean "loaded_png", if the picture is loaded once this boolean is set to true, so the LoadTextureFromHeader function is only called once.

Using LoadTexturefromFile the images where not displayed in the CI version because they where not found in the right path.

The CI version is not that important, supposedly. I don't know of any program sent out as a standalone executable file. Programs are sent out for users (or market) commonly as a pack comprising all dependencies needed for the executable to run correctly, such as images, sounds, .gif files, or even videos.

Right now the CI version is tested by users, not a package version. If we want them to see the pictures we need to include the pictures via a .h file.

The stb_image.h library looks pretty much like our other external library imfilebrowser.h that we have a link to it in our repo. Loading images using that library is not that complicated but quite easy and straightforward. It's not needed to load the file(s) on each frame either. Actually only the Dear ImGui widget ImGui::Image() is in the main loop, just like all other widgets.

Here is a resizable image in our project:

Can you provide an example of the code you used for this?

[R-Abbasi]

If the header is included in the main.cpp it works. If this is not c++ standard we can change the specifier to static or inline specifier and load the .hpp in the files where it is needed.

The point is not if it works or not. "working" alone isn't enough, I think. You should also think of readability, maintainability, concise code, modern features and more. Try to completely figure out what has been said in the pior posts and let me know of any part you have difficulties with understanding it, please.

In the code there is a boolean "loaded_png", if the picture is loaded once this boolean is set to true, so the LoadTextureFromHeader function is only called once.

The boolean is set to true no matter if loading succeeded or failed, because if (ret == NULL) must always be false.
As well as, static bool loaded_png[size] = {false}; makes no sense to me.
It seems the code has lots of inconsistencies.

Below is another example. If you set a dark theme, it should pretty much look like the current version but this is written using a few lines of code with the external library and load function. The logo changes in size along with the program window. Other buttons can be moved below "Search for Devices" quite easily supposedly. Take other .png files for other languages and they will support other languages too. It also should be very simple since there're only few buttons there.
The executable file and images can be zipped into one fie sent for users to test.

[AKMaily]

If the header is included in the main.cpp it works. If this is not c++ standard we can change the specifier to static or inline specifier and load the .hpp in the files where it is needed.

The point is not if it works or not. "working" alone isn't enough, I think. You should also think of readability, maintainability, concise code, modern features and more. Try to completely figure out what has been said in the pior posts and let me know of any part you have difficulties with understanding it, please.

That is exactly what i meant with c++ standard.

In the code there is a boolean "loaded_png", if the picture is loaded once this boolean is set to true, so the LoadTextureFromHeader function is only called once.

The boolean is set to true no matter if loading succeeded or failed, because if (ret == NULL) must always be false. As well as, static bool loaded_png[size] = {false}; makes no sense to me. It seems the code has lots of inconsistencies.

Below is another example. If you set a dark theme, it should pretty much look like the current version but this is written using a few lines of code with the external library and load function. The logo changes in size along with the program window. Other buttons can be moved below "Search for Devices" quite easily supposedly. Take other .png files for other languages and they will support other languages too. It also should be very simple since there're only few buttons there. The executable file and images can be zipped into one fie sent for users to test.

Okay i get what you mean. It should check if ret is false not NULL. It is set to false if the image_data == NULL. For the second part pls provide a code example. It is nice to know that it only takes a few lines of code but i would like to know what the exact code is.

[R-Abbasi]

That is exactly what i meant with c++ standard.

No. Standard C++ differs from and has nothing to do with that, usually. It's called "good code" and refers not specifically to C++ itself but all kind of programming.

Okay i get what you mean. It should check if ret is false not NULL. It is set to false if the image_data == NULL.

You could use this instead, I guess:

 for (int w = 0; w < size; w++)
      if (!loaded_png[w]) {
        if (LoadTextureFromHeader(imagesNames[w], imagesLen[w],
                                  &my_image_texture[w], &my_image_width[w],
                                  &my_image_height[w]))
          loaded_png[w] = true;
        else
        {
          fmt::println("Error Loading Png.");
          loaded_png[w] = false;
        }
     }

static bool ret[size]; looks unnecessary too.

For the second part pls provide a code example. It is nice to know that it only takes a few lines of code but i would like to know what the exact code is.

Loading images is done more or less the exact same way as suggested in the Dear ImGui Docs. The size used for ImGui::Image should be a fraction of main window size to let the logo fit its size with it. I also used two ImGui::BeginChilds to have the main window in two sides.

This is part of the code (still under development):

  int logo_width{}, srch_dvc_width{};
  int logo_height{}, srch_dvc_height {};
  GLuint logo_texture{}, srch_dvc_texture{};

  auto render = [&]() {
    load_settings(config);
    SetupImGuiStyle(false, 0.99f);
    auto windowSize = ImGui::GetIO().DisplaySize;

    ImGui::Begin("OmniScopev2 Data Capture Tool", nullptr,
                     ImGuiWindowFlags_NoCollapse |
                     ImGuiWindowFlags_NoResize | ImGuiWindowFlags_NoMove); 

     ImGui::BeginChild("Left Side",
     ImVec2(windowSize.x * 0.17f, windowSize.y), ImGuiChildFlags_AutoResizeX |
            ImGuiChildFlags_AutoResizeY | ImGuiChildFlags_AlwaysAutoResize);

     ImGui::Image((void*)(intptr_t)logo_texture, ImVec2(windowSize.x * 0.2f, windowSize.y * 0.2f));

     // Start only if devices are available, otherwise search for devices
      if (!sampler.has_value()) 
        if(ImGui::ImageButton("##search for dvc",
                              (void*)(intptr_t)srch_dvc_texture, 
                              ImVec2(srch_dvc_width, srch_dvc_height))) {
          devices.clear();
          deviceManager.clearDevices();
          initDevices();
        }
   // TODO: rest of menu

[R-Abbasi]

Another option to access the .png files is to put their folder next to the .config files folders on the server so that that folder is also downloaded along with the .config folders, at the program start up.

[AKMaily]

You could use this instead, I guess:

for (int w = 0; w < size; w++)
    if (!loaded_png[w]) {
      if (LoadTextureFromHeader(imagesNames[w], imagesLen[w],
                                &my_image_texture[w], &my_image_width[w],
                                &my_image_height[w]))
        loaded_png[w] = true;
      else
      {
        fmt::println("Error Loading Png.");
        loaded_png[w] = false;
      }
   }

static bool ret[size]; looks unnecessary too.

This really seem to be a better version. I will change the code.

For the second part pls provide a code example. It is nice to know that it only takes a few lines of code but i would like to know what the exact code is.

Loading images is done more or less the exact same way as suggested in the Dear ImGui Docs. The size used for ImGui::Image should be a fraction of main window size to let the logo fit its size with it. I also used two ImGui::BeginChilds to have the main window in two sides.

This is part of the code (still under development):

  int logo_width{}, srch_dvc_width{};
  int logo_height{}, srch_dvc_height {};
  GLuint logo_texture{}, srch_dvc_texture{};

  auto render = [&]() {
    load_settings(config);
    SetupImGuiStyle(false, 0.99f);
    auto windowSize = ImGui::GetIO().DisplaySize;

    ImGui::Begin("OmniScopev2 Data Capture Tool", nullptr,
                     ImGuiWindowFlags_NoCollapse |
                     ImGuiWindowFlags_NoResize | ImGuiWindowFlags_NoMove); 

     ImGui::BeginChild("Left Side",
     ImVec2(windowSize.x * 0.17f, windowSize.y), ImGuiChildFlags_AutoResizeX |
            ImGuiChildFlags_AutoResizeY | ImGuiChildFlags_AlwaysAutoResize);

     ImGui::Image((void*)(intptr_t)logo_texture, ImVec2(windowSize.x * 0.2f, windowSize.y * 0.2f));

     // Start only if devices are available, otherwise search for devices
      if (!sampler.has_value()) 
        if(ImGui::ImageButton("##search for dvc",
                              (void*)(intptr_t)srch_dvc_texture, 
                              ImVec2(srch_dvc_width, srch_dvc_height))) {
          devices.clear();
          deviceManager.clearDevices();
          initDevices();
        }
   // TODO: rest of menu

Did you need to use the LoadImagesFromHeader or another load function before the images a rendered? Or was ImGui::Image() itself enough ?

[R-Abbasi]

LoadTextureFromFile is used to load the images. Not sure if I got the rest of your question properly, but loading the images, as mentioned previously, is done only once and is outside the main loop (render).

Simply said, something like:

int main() {
 // ..
 int logo_width{}, srch_dvc_width{};
 int logo_height{}, srch_dvc_height {};
 GLuint logo_texture{}, srch_dvc_texture{};

  auto render = [&]() {
  // ..
 };
// load images