[Feat/aut586] - Updated ImGui::Tables API #281

ahmedskhalil · 2020-10-17T15:10:57Z

Description
SmartPeak is updated to the latest ImGui tables branch namely against f3184b82b0a709e045bb09e00fa4be81311bae9c commit.

Fixes

Updated ImGui legacy function calls to the latest function definitions in Widget.cpp, GUI.cpp.
Updated FindImGui.cmake module to the latest ImGui source code structure.
Updated SmartPeak to depend on Boost 1.71 or later, this includes removing the FindBoost.cmake module and relying solely on the modules supplied by cmake. As of this PR, boost::filesystem is still in use.

Further Informations

The ImGui version used to pushed as part of SmartPeak codebase, this ensures the stability of the upcoming SmartPeak releases.

Future Improvements
The tables branch is constantly evolving, any future modifications to this branch can affect the stability of SmartPeak, any major updates to the tables branch will be integrated into SmartPeak until the branch stabilizes and is integrated in the v1.80 release.

… module for boost

…build boost

…of boost

dmccloskey

This looks nice.

My only concerns are the location we are downloading the Boost binaries from and the lines added for the OpenMS build. My other comments are mostly questions to better understand the changes made to the Tables API for my own edification.

dmccloskey · 2020-10-18T09:38:00Z

.circleci/config.yml

          choco install doxygen.install
          choco install nsis.portable
          refreshenv
          $qt_url = 'https://github.com/martinrotter/qt5-minimalistic-builds/releases/download/5.12.9/qt-5.12.9-dynamic-msvc2019-x86_64.7z'
          $sdl2_url = 'https://www.libsdl.org/release/SDL2-devel-2.0.12-VC.zip'
+          $boost_url = 'https://github.com/ahmedskhalil/Boost-1.73-Prebuilts/releases/download/0.0.1/boost_1_73_0.7z'


What was our conclusion on this from our last conversation? Was it to just include the zip files in the SmartPeak repo?

Github excludes file sizes that exceeds the 100MB mark Conditions for large files. OpenMS prebuilt contribs do include the required boost libraries except for boost::filesystem so once we get away from it and move to the std::filesystem we can rely solely on OpenMS prebuilt contribs for both OpenMS and SmartPeak.

Thanks for that clarification. OK let's go with this as a temporary solution for now until C++17 is integrated.

dmccloskey · 2020-10-18T09:38:57Z

.circleci/config.yml

+          cd ~
+          git clone --branch develop --depth 1 https://github.com/OpenMS/OpenMS.git
+          cd ~/OpenMS
+          ((Get-Content -path src\openswathalgo\include\OpenMS\OPENSWATHALGO\ALGO\Scoring.h -Raw) -replace '#include <vector>',"#include <vector>`r`n#include <algorithm>") | Set-Content -Path src\openswathalgo\include\OpenMS\OPENSWATHALGO\ALGO\Scoring.h


Can you elaborate on the reasoning for this? Is this a bug in OpenMS?

We have to add #include <algorithm> otherwise the compiler won't build OpenSwathAlgo, I have submitted an issue to the OpenMS team : [Error] - Failed to compile with VS2019 MSVC 14.2

I just looked over the bug report in OpenMS. If you don't mind helping to get the fix included in OpenMS so we do not need this workaround, I think that would be the cleanest solution.

I have submitted a PR to get this resolved, once it's in place we can remove this line.

dmccloskey · 2020-10-18T09:40:41Z

.circleci/config.yml

-          cmake -G "Visual Studio 16 2019" -A x64 -DBUILD_TYPE=HDF5 ~/OpenMS/contrib
+          cd ~/OpenMS
+          mkdir contrib_build; cd contrib_build
+          $url_contrib = 'https://abibuilder.informatik.uni-tuebingen.de/archive/openms/contrib/windows/x64/msvc-14.2/contrib_build.tar.gz'


dmccloskey · 2020-10-18T09:43:03Z

src/smartpeak/source/ui/Widget.cpp

@@ -123,16 +123,16 @@ namespace SmartPeak

    // headers
    const ImGuiTableFlags table_flags = ImGuiTableFlags_Resizable | ImGuiTableFlags_Hideable |
-      //ImGuiTableFlags_Sortable | 
-      ImGuiTableFlags_Borders | ImGuiTableFlags_Scroll | ImGuiTableFlags_ScrollFreezeTopRow | ImGuiTableFlags_ScrollFreezeLeftColumn;


Are the flags for freezing the top row and freezing the left column no longer available or did the names change?

They are no longer included as flags, but possible with a function call as per ocornut's comment : BREAKING CHANGES (SEPT 2020)

Thanks for clarifying this as well and sharing the link.

Hopefully, this information will come in handy for upgrading the old columns API to the tables API in the next round of PRs.

dmccloskey · 2020-10-18T09:43:58Z

src/examples/source/GUI.cpp

@@ -795,9 +796,10 @@ int main(int argc, char **argv)
        if (show_spectra_line_plot && ImGui::BeginTabItem("Spectra", &show_spectra_line_plot))
        {
          // Filter for the position
+          const ImGuiSliderFlags slider_flags = ImGuiSliderFlags_Logarithmic;


That is a good addition for the Tables API. Nice job on deducing that too.

dmccloskey · 2020-10-18T09:45:34Z

src/smartpeak/source/ui/Widget.cpp


    if (ImGui::BeginTable(table_id_.c_str(), headers_.size(), table_flags)) {
      // First row headers
      for (int col = 0; col < headers_.size(); col++) {
        ImGui::TableSetupColumn(headers_(col).c_str());
      }
-      ImGui::TableAutoHeaders();
-
+      ImGui::TableSetupScrollFreeze(headers_.size(), 1);


I think I understand: so this line replaces the above flags then?

Yes, however this needs some more testing as I wasn't able to load some files to see if the headers stay in-place while scrolling.

I would recommend testing that by running the GUI and loading in a file. Please let me know if you are having issues loading a dummy data set. I posted several videos on the SmartPeak_user slack channel, but if we should have a quick Zoom meeting, I don't mind doing that.

Unfortunately we do not yet have a nice integration test for the GUI that tests out these visual features, so this part still needs to be confirmed by running the GUI on a sample data set.

I have done a couple of manual tests and the headers are frozen as previously.

dmccloskey · 2020-10-18T09:46:44Z

src/tests/class_tests/smartpeak/CMakeLists.txt

@@ -12,7 +12,7 @@ configure_file(${PROJECT_SOURCE_DIR}/include/SmartPeak/test_config.h.in ${CONFIG
 #------------------------------------------------------------------------------
 # Boost
 #------------------------------------------------------------------------------
-find_package(boost COMPONENTS unit_test_framework)
+find_package(Boost COMPONENTS filesystem unit_test_framework)


It will be nice when we can merge in C++17 to get rid of the filesystem once and for all.

Would be great and we can merely depend on the OpenMS prebuilt contribs for all the required libs for Windows CI.

…4be81311bae9c - Superbuild for ImGui is disabled

…4be81311bae9c - Superbuild for ImGui is disabled - updating CI superbuild directories

ahmedskhalil added 10 commits October 16, 2020 12:27

Updated ImGui::Tables API

7f77da6

appending a missing include for OpenSwathAlgo

9814abe

appending a missing include for OpenSwathAlgo - typo fix

d5d5d41

appending a missing include for OpenSwathAlgo - fixes

87c7ed0

appending a missing include for OpenSwathAlgo - fixes

919b8bd

appending a missing include for OpenSwathAlgo - fixes

ffafb38

appending a missing include for OpenSwathAlgo - removing unused cmake…

6caf993

… module for boost

appending a missing include for OpenSwathAlgo - switching to contrib_…

6ed885e

…build boost

appending a missing include for OpenSwathAlgo - using hosted version …

be923db

…of boost

updating GUI to latest ImGui API tables

816927c

ahmedskhalil requested a review from dmccloskey October 17, 2020 15:10

dmccloskey requested changes Oct 18, 2020

View reviewed changes

ahmedskhalil added 5 commits October 22, 2020 13:45

integrating ImGui in SmartPeak - ImGui v.:f3184b82b0a709e045bb09e00fa…

ce7c15b

…4be81311bae9c - Superbuild for ImGui is disabled

integrating ImGui in SmartPeak - ImGui v.:f3184b82b0a709e045bb09e00fa…

9e26599

…4be81311bae9c - Superbuild for ImGui is disabled - updating CI superbuild directories

integrating ImGui in SmartPeak - ImGui v.:f3184b82b0a709e045bb09e00fa…

bdae1d4

…4be81311bae9c - Superbuild for ImGui is disabled - updating CI superbuild directories

integrating ImGui in SmartPeak - ImGui v.:f3184b82b0a709e045bb09e00fa…

831c3cd

…4be81311bae9c - Superbuild for ImGui is disabled - updating CI superbuild directories

removing unused CI mod

43185f0

dmccloskey approved these changes Oct 22, 2020

View reviewed changes

dmccloskey merged commit 88b0679 into develop Oct 22, 2020

dmccloskey deleted the feat/aut586 branch October 22, 2020 20:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feat/aut586] - Updated ImGui::Tables API #281

[Feat/aut586] - Updated ImGui::Tables API #281

ahmedskhalil commented Oct 17, 2020 •

edited

Loading

dmccloskey left a comment

dmccloskey Oct 18, 2020

ahmedskhalil Oct 18, 2020

dmccloskey Oct 19, 2020

dmccloskey Oct 18, 2020

ahmedskhalil Oct 18, 2020

dmccloskey Oct 19, 2020

ahmedskhalil Oct 20, 2020

dmccloskey Oct 18, 2020

dmccloskey Oct 18, 2020

ahmedskhalil Oct 18, 2020

dmccloskey Oct 19, 2020

dmccloskey Oct 18, 2020

dmccloskey Oct 18, 2020

ahmedskhalil Oct 18, 2020

dmccloskey Oct 19, 2020

ahmedskhalil Oct 22, 2020

dmccloskey Oct 18, 2020

ahmedskhalil Oct 18, 2020 •

edited

Loading

[Feat/aut586] - Updated ImGui::Tables API #281

[Feat/aut586] - Updated ImGui::Tables API #281

Conversation

ahmedskhalil commented Oct 17, 2020 • edited Loading

dmccloskey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahmedskhalil Oct 18, 2020 • edited Loading

Choose a reason for hiding this comment

ahmedskhalil commented Oct 17, 2020 •

edited

Loading

ahmedskhalil Oct 18, 2020 •

edited

Loading