search

pthom / hello_imgui

Hello, Dear ImGui: unleash your creativity in app development and prototyping
https://pthom.github.io/hello_imgui
MIT License
604 stars 91 forks source link

Google Closure compiler support is almost perfect except for this one line. #101

Closed abinash18 closed 1 month ago

abinash18 commented 3 months ago

Hey, pthom. Great work with hello imgui. I use it at work for developing user interfaces with scientific instrumentation. Recently I have been using it as a web interface. When i compile with emscripten it generates perfect code except that it is too big to fit on space constrained embedded devices and takes too long to load on slower computers and phones. When compiled the application code takes about 25 to 30 MB of space on the web. which is a lot even though it is acceptable. I have managed to reduce the size down to 2.6 MB which is amazing its at the level to compete with a proper web app built with React or a similar framework. Although its almost perfect i had to remove some things for the Google Closure compiler to work. like in the abstract runner file I removed support for getting the devices pixel ratio here:

float AbstractRunner::ImGuiDefaultFontGlobalScale()
{
    float fontSizeIncreaseFactor = 1.f;

#ifdef __EMSCRIPTEN__
    // Query the brower to ask for devicePixelRatio
    double windowDevicePixelRatio = EM_ASM_DOUBLE({
        // scale = window.devicePixelRatio;
        // return scale;
        return 1;
    });
    printf("window.devicePixelRatio=%lf\n", windowDevicePixelRatio);

    fontSizeIncreaseFactor = windowDevicePixelRatio;
#endif

...

What I think is happening is that the compiler needs to export this as a function to assembly to run in the browser but the closure compiler will optimize the definition of scale out to reduce memory consumption so the compiler throws a error saying scale isnt defined. or something along those lines. I will try and fix this.

abinash18 commented 3 months ago

ok I fixed it, I didnt read the documentation on EM_ASM_DOUBLE i was assuming it was c code but no its JS so the fix is adding the keyword var to define the scale variable.

I will make pull request.

abinash18 commented 3 months ago

It seems as I have an older version of hellow imgui and this has been moved to the fuction 

void AbstractRunner::SetupDpiAwareParams()
...
pthom commented 3 months ago

Hello Abinash,

Thanks for contacting me!

When i compile with emscripten it generates perfect code except that it is too big to fit on space constrained embedded devices and takes too long to load on slower computers and phones. When compiled the application code takes about 25 to 30 MB of space on the web. which is a lot even though it is acceptable. I have managed to reduce the size down to 2.6 MB which is amazing

I'm very interested in this! Could you share more info about what you did to achieve this. If it works, this could be added to the FAQ.

One note however: I noticed that the build instructions for Emscripten did not include an advice to compile in release mode, which can lead to much smaller files. I updated this.

Would you mind to share how smaller the files become between a release build and when using the Google Closer compiler on this release build? If there is a sizeable reduction in the size of the files, I would be very interesting in having more information on how you obtained this result.

I will make pull request.

Thanks!

abinash18 commented 3 months ago

Yes of course i can show the difference let me cook up an example build tommorow for you. Although off the top of my head my build which has some proprietary libraries and Implot. was almost 30MB when compiled in debug mode. And 17MB when compiled in Release. One interesting thing is that if you say -sWASM=0 to disable wasm output and put everything in JS its actually bigger than building a wasm binary. Then if you enable optimizations with -Os and also some other compile options i can show you tommorow it will reduce by about 500kb. I can get the wasm module which is the biggest file out of all even below 2MB if i am successfully able to remove all exception catching. And thank you for the merge happy to improve my favorite ui library.

pthom commented 3 months ago

On my side, I would be interested if it would be possible to reduce the size of the demo for Dear ImGui bundle (which is based on Hello ImGui).

At the moment, the sizes are as follow:

   1.8K demo_imgui_bundle.html
   348K  demo_imgui_bundle.js
   5.4M  demo_imgui_bundle.wasm
   9.8M  demo_imgui_bundle.data

There are lots of assets, so the size of the data file is heavy (but this is normal, since it is a fully featured demo).

abinash18 commented 3 months ago

I am currently trying to compile imgui bundle with the closure compiler but it seems to be stuck at the imguial_term.cpp file. maybe you can compile on your end and see if you have different results. I added this line to the very top of the cmake file after the project declaration.

add_link_options(-Os --closure 1) I started the compile with emcmake cmake .. -DCMAKE_BUILD_TYPE=Release -DEMSCRIPTEN=ON in a build folder

EDIT: I am unable to compile right now because it keeps terminating it self mostlikey due to running out of memory i will try on a maching with more.

pthom commented 3 months ago

it keeps terminating it self mostlikey due to running out of memory

Yes, it may happen at link time when running compilation in parallel, the link uses a lot of memory with emscripten.

Are you using make -j or ninja. If so, you could reduce the number of concurrent build with make -j 2 or even make -j 1

The build works ok with add_link_options(-Os --closure 1) on my side.

abinash18 commented 3 months ago

yes i was using make -j i will re do single core What is the file size on your end?

pthom commented 3 months ago

I'm sorry, I answered t*o quickly: add_link_options(-Os --closure 1)was not active when I compiled.

On my side, I cannot compile with those options; it fails for HelloImGui and ImGui Bundle with the message:

building:ERROR: Closure compiler run failed:

building:ERROR: /var/folders/w1/w77kvvl9613022ksfxj7v8xh0000gn/T/emscripten_temp_pzslygg1/hello_imgui_demodocking.jso1.js:1010:9: ERROR - [JSC_UNDEFINED_VARIABLE] variable allocate is undeclared
  1010|   return allocate(intArrayFromString(reply), "i8", ALLOC_NORMAL);
                 ^^^^^^^^

/var/folders/w1/w77kvvl9613022ksfxj7v8xh0000gn/T/emscripten_temp_pzslygg1/hello_imgui_demodocking.jso1.js:1010:51: ERROR - [JSC_UNDEFINED_VARIABLE] variable ALLOC_NORMAL is undeclared
  1010|   return allocate(intArrayFromString(reply), "i8", ALLOC_NORMAL);

I'm running the latest emscripten version, on macOS

abinash18 commented 3 months ago

So the compiler continues now that I only use 1 core but I have encountered another issue: `building:ERROR: Closure compiler run failed:

building:ERROR: /tmp/emscripten_temp_kbh54yia/demo_imgui_bundle.js.pgrow.jso2.js:1942:9: ERROR - [JSC_UNDEFINED_VARIABLE] variable allocate is undeclared 1942| return allocate(intArrayFromString(reply), "i8", ALLOC_NORMAL); ^^^^^^^^

/tmp/emscripten_temp_kbh54yia/demo_imgui_bundle.js.pgrow.jso2.js:1942:51: ERROR - [JSC_UNDEFINED_VARIABLE] variable ALLOC_NORMAL is undeclared 1942| return allocate(intArrayFromString(reply), "i8", ALLOC_NORMAL); ^^^^^^^^^^^^

2 error(s), 0 warning(s)`

There still seem to be some JS code that is undefined, But i am not sure where this is

abinash18 commented 3 months ago

Oh ahah I was about to say the same thing

abinash18 commented 3 months ago

These errors seem to be from one of the external modules which uses malloc most likely. I have not encountered this with implot so it cant bethat. are you aware of any external lib that using dynamic memory allocation?

abinash18 commented 3 months ago

At my work I only use hello imgui and implot with emscripten websockets. so I know for sure hello imgui on its own works

pthom commented 3 months ago

I do have the same error when building hello imgui. Strange...

abinash18 commented 3 months ago

hmm I am currently on a previous version of hello imgui btw commit a687d09 https://github.com/pthom/hello_imgui/commit/a687d090fb073d741c3f0f0b2ad211c21099ed86

This is the commit i am on currently

pthom commented 3 months ago

I have the same error with https://github.com/pthom/hello_imgui/commit/a687d090fb073d741c3f0f0b2ad211c21099ed86

abinash18 commented 3 months ago

ah I think i see the problem by default memory growth is enabled i am compiling with it off in my project. Try it with that. I also only have SDL2 enabled and no other backend. 


        Hello ImGui build options:
    ===========================================================================
      Backends:  HELLOIMGUI_USE_SDL_OPENGL3
    ---------------------------------------------------------------------------
      Options:
        HELLOIMGUI_USE_FREETYPE:        ON  (Use target freetype)
        HELLOIMGUI_WITH_TEST_ENGINE:    OFF
        BUILD_DEMOS - TESTS - DOCS:     OFF - OFF - OFF
    ---------------------------------------------------------------------------
      ImGui:
        Build ImGui:                    ON
        ImGui source dir:               external/imgui
    ---------------------------------------------------------------------------
      Emscripten options
        HELLOIMGUI_EMSCRIPTEN_PTHREAD:  OFF
        HELLOIMGUI_EMSCRIPTEN_PTHREAD_ALLOW_MEMORY_GROWTH: OFF
    ---------------------------------------------------------------------------
      OpenGL - use glad loader          OFF
    ---------------------------------------------------------------------------
      Platform Backend(s):
        SDL:                            Use system Library
    ===========================================================================```
abinash18 commented 3 months ago

Yes Turning off Memory growth i got it to compile. Here is a demo https://github.com/abinash18/imgui-reduced-size

pthom commented 3 months ago

It still fails on my side, when compiling Hello ImGui (master branch) I added some more var keyword here

Now, I have this error:

 make hello_imgui_demodocking VERBOSE=1 

 ...

✗ cd /Users/pascal/dvp/OpenSource/ImGuiWork/_Bundle/hello_imgui_vcpkg/build/emscripten/src/hello_imgui_demos/hello_imgui_demodocking && /opt/homebrew/Cellar/cmake/3.27.7/bin/cmake -E cmake_link_script CMakeFiles/hello_imgui_demodocking.dir/link.txt --verbose=1
/Users/pascal/emsdk/upstream/emscripten/em++ -O3 -DNDEBUG -Os --closure 1 --preload-file /Users/pascal/dvp/OpenSource/ImGuiWork/_Bundle/hello_imgui_vcpkg/build/emscripten/src/hello_imgui_demos/hello_imgui_demodocking/tmp/common_assets@/ --preload-file /Users/pascal/dvp/OpenSource/ImGuiWork/_Bundle/hello_imgui_vcpkg/src/hello_imgui_demos/hello_imgui_demodocking/assets@/ --shell-file /Users/pascal/dvp/OpenSource/ImGuiWork/_Bundle/hello_imgui_vcpkg/build/emscripten/src/hello_imgui_demos/hello_imgui_demodocking/tmp/shell.emscripten_hello_imgui_demodockin
g.html -sUSE_SDL=2 -sINITIAL_MEMORY=125829120 -s USE_SDL=2 -sMAX_WEBGL_VERSION=2 @CMakeFiles/hello_imgui_demodocking.dir/objects1.rsp -o ../../../bin/hello_imgui_demodocking.html @CMakeFiles/hello_imgui_demodocking.dir/linkLibs.rsp

/Users/pascal/emsdk/upstream/emscripten/em++ -O3 -DNDEBUG -Os --closure 1 --preload-file /Users/pascal/dvp/OpenSource/ImGuiWork/_Bundle/hello_imgui_vcpkg/build/emscripten/src/hello_imgui_demos/hello_imgui_demodocking/tmp/common_assets@/ --preload-file /Users/pascal/dvp/OpenSource/ImGuiWork/_Bundle/hello_imgui_vcpkg/src/hello_imgui_demos/hello_imgui_demodocking/assets@/ --shell-file /Users/pascal/dvp/OpenSource/ImGuiWork/_Bundle/hello_imgui_vcpkg/build/emscripten/src/hello_imgui_demos/hello_imgui_demodocking/tmp/shell.emscripten_hello_imgui_demodocking.html -sUSE_SDL=2 -sINITIAL_MEMORY=125829120 -s USE_SDL=2 -sMAX_WEBGL_VERSION=2 @CMakeFiles/hello_imgui_demodocking.dir/objects1.rsp -o ../../../bin/hello_imgui_demodocking.html @CMakeFiles/hello_imgui_demodocking.dir/linkLibs.rsp
building:ERROR: Closure compiler run failed:

building:ERROR: /var/folders/w1/w77kvvl9613022ksfxj7v8xh0000gn/T/emscripten_temp_zigp8hz9/hello_imgui_demodocking.jso4.js:30:2837: ERROR - [JSC_UNDEFINED_VARIABLE] variable allocate is undeclared

/var/folders/w1/w77kvvl9613022ksfxj7v8xh0000gn/T/emscripten_temp_zigp8hz9/hello_imgui_demodocking.jso4.js:30:2877: ERROR - [JSC_UNDEFINED_VARIABLE] variable ALLOC_NORMAL is undeclared

...
324687:$0=>{var str=UTF8ToString($0)+"\n\n"+"Abort/Retry/Ignore/AlwaysIgnore? [ariA] :";var reply=window.prompt(str,"i");if(reply===null){reply="i"}return allocate(intArrayFromString(reply),"i8",ALLOC_NORMAL)}

And the code related to this error seems to come from SDL_assert.c

abinash18 commented 3 months ago

aha here is something useful to me: https://web.dev/articles/emscripten-embedding-js-snippets

The closure compiler optimizes these functions out of the JS context because they are not used until the wasm part so we need a way to implement them : https://emscripten.org/docs/api_reference/preamble.js.html

https://github.com/google/closure-compiler/wiki/Annotating-JavaScript-for-the-Closure-Compiler

This seems to be an issue with emscripten itself and not hello imgui.

https://emscripten.org/docs/optimizing/Optimizing-Code.html#unsafe-optimizations

I am assuming the allocate function is not properly exported.

i think compiling without imgui_md will complete, but it is a neccesary addon therefore that is not an option

abinash18 commented 3 months ago

I might have figured out why i could compile and you could not, image

Here is my CMake if i remove -flto -fno-rtti I get the same errors as you.

pthom commented 3 months ago

Hi,

Thanks for the info. Based on my tests, only the link option -flto is required.

Here are the size gains based on my tests:

Standard emscripten release compil:

5.0M bin_std/hello_imgui_demodocking.data
266K bin_std/hello_imgui_demodocking.js
2.2M bin_std/hello_imgui_demodocking.wasm

Adding -Os (optimize for size) as compile and link options leads to:

5.0M bin_os/hello_imgui_demodocking.data
265K bin_os/hello_imgui_demodocking.js
1.8M bin_os/hello_imgui_demodocking.wasm

Adding --closure 1 and. -fltoas link options leads to:

5.0M bin_lto/hello_imgui_demodocking.data
126K bin_lto/hello_imgui_demodocking.js
1.8M bin_lto/hello_imgui_demodocking.wasm