Skip to content

Commit

Permalink
Complete refactor of options API (f3d-app#1471)
Browse files Browse the repository at this point in the history
Complete refactor of options API based on generated struct and methods from a `options.json` file.

 - Add struct and methods generation code in f3dOptions.cmake
 - Add generation in library/CMakeLists.txt
 - Add options.json containing all options
 - Add new API in options.h and implement it in options.cxx, remove old API
 - Adapt code in library and in app for the new API
 - Add options testing
 - Added a quick doc about the three APIs and in header docs
 - Add a C++11 compatibility
 - Added examples
 - Improve clang-format CI and update files accordingly
 
Will be done in other PRs:

 - Add deprecation logic in generation code: f3d-app#1568
 - Rework application and simplify option logic: f3d-app#1569
 - Add more options types : f3d-app#1570
 - Add actual parsing for all options types: f3d-app#1571
 - Add complete documentation for options and option parsing: f3d-app#1572
 - Proper java and javascript bindings: f3d-app#1573 f3d-app#1574
 - use exception translator in python bindings: f3d-app#1575
  - Improve compile-time opti in options_tools.h.in:  f3d-app#1576
  • Loading branch information
mwestphal authored and Nokse22 committed Sep 21, 2024
1 parent 05c8368 commit de377bc
Show file tree
Hide file tree
Showing 57 changed files with 1,813 additions and 1,052 deletions.
5 changes: 3 additions & 2 deletions .github/workflows/style-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ jobs:
- name: C++ Formatting
uses: DoozyX/[email protected]
with:
source: 'library application python plugins'
extensions: 'h,cxx'
source: 'library application python plugins examples'
extensions: 'h,cxx,in'
exclude: '**/*.py.in'
clangFormatVersion: 14 # Last Ubuntu LTS version (22.04)
- name: Python Formatting
uses: psf/black@stable
Expand Down
10 changes: 5 additions & 5 deletions application/F3DConfig.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@

namespace F3D
{
const std::string AppName = "@PROJECT_NAME@";
const std::string AppTitle = "@PROJECT_DESCRIPTION@";
const std::string AppVersion = "@F3D_VERSION@";
const std::string AppVersionFull = "@F3D_VERSION_FULL@";
const std::string PluginsInstallDir = "@F3D_PLUGINS_INSTALL_DIR@";
const std::string AppName = "@PROJECT_NAME@";
const std::string AppTitle = "@PROJECT_DESCRIPTION@";
const std::string AppVersion = "@F3D_VERSION@";
const std::string AppVersionFull = "@F3D_VERSION_FULL@";
const std::string PluginsInstallDir = "@F3D_PLUGINS_INSTALL_DIR@";
}

// TODO: Use CMake definitions and get rid of these
Expand Down
144 changes: 79 additions & 65 deletions application/F3DOptionsParser.cxx

Large diffs are not rendered by default.

40 changes: 21 additions & 19 deletions application/F3DStarter.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ F3DStarter::F3DStarter()
: Internals(std::make_unique<F3DStarter::F3DInternals>())
{
// Set option outside of command line and config file
this->Internals->DynamicOptions.set(
"ui.dropzone-info", "Drop a file or HDRI to load it\nPress H to show cheatsheet");
this->Internals->DynamicOptions.ui.dropzone_info =
"Drop a file or HDRI to load it\nPress H to show cheatsheet";

// Initialize dmon
dmon_init();
Expand Down Expand Up @@ -510,9 +510,10 @@ int F3DStarter::Start(int argc, char** argv)
// TODO: add a image::canRead

// Load the file as an HDRI instead of adding it.
this->Internals->Engine->getOptions().set("render.hdri.file", file);
this->Internals->Engine->getOptions().set("render.hdri.ambient", true);
this->Internals->Engine->getOptions().set("render.background.skybox", true);
f3d::options& options = this->Internals->Engine->getOptions();
options.render.hdri.file = file;
options.render.hdri.ambient = true;
options.render.background.skybox = true;

// Rendering now is needed for correct lighting
this->Render();
Expand Down Expand Up @@ -557,13 +558,13 @@ int F3DStarter::Start(int argc, char** argv)

if (!fullPath.empty())
{
this->Internals->Engine->getOptions().set(
"model.scivis.colormap", F3DColorMapTools::Read(fullPath));
this->Internals->Engine->getOptions().model.scivis.colormap =
F3DColorMapTools::Read(fullPath);
}
else
{
f3d::log::error("Cannot find the colormap ", this->Internals->AppOptions.ColorMapFile);
this->Internals->Engine->getOptions().set("model.scivis.colormap", std::vector<double>{});
this->Internals->Engine->getOptions().model.scivis.colormap = std::vector<double>{};
}
}

Expand Down Expand Up @@ -894,8 +895,9 @@ void F3DStarter::LoadFile(int index, bool relativeIndex)
loader.loadGeometry("", true);
}

this->Internals->Engine->getOptions().set("ui.dropzone", !this->Internals->LoadedFile);
this->Internals->Engine->getOptions().set("ui.filename-info", filenameInfo);
f3d::options& options = this->Internals->Engine->getOptions();
options.ui.dropzone = !this->Internals->LoadedFile;
options.ui.filename_info = filenameInfo;
}

//----------------------------------------------------------------------------
Expand Down Expand Up @@ -949,22 +951,22 @@ void F3DStarter::SaveScreenshot(const std::string& filenameTemplate, bool minima
bool noBackground = this->Internals->AppOptions.NoBackground;
if (minimal)
{
options.set("ui.bar", false);
options.set("ui.cheatsheet", false);
options.set("ui.filename", false);
options.set("ui.fps", false);
options.set("ui.metadata", false);
options.set("ui.animation-progress", false);
options.set("interactor.axis", false);
options.set("render.grid.enable", false);
options.ui.scalar_bar = false;
options.ui.cheatsheet = false;
options.ui.filename = false;
options.ui.fps = false;
options.ui.metadata = false;
options.ui.animation_progress = false;
options.interactor.axis = false;
options.render.grid.enable = false;
noBackground = true;
}

f3d::image img = this->Internals->Engine->getWindow().renderToImage(noBackground);
this->Internals->addOutputImageMetadata(img);
img.save(path.string(), f3d::image::SaveFormat::PNG);

options.getAsDoubleRef("render.light.intensity") *= 5;
options.render.light.intensity *= 5;
this->Render();

this->Internals->Engine->setOptions(optionsCopy);
Expand Down
4 changes: 4 additions & 0 deletions application/testing/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,10 @@ f3d_test(NAME TestReadersList ARGS --readers-list REGEXP_FAIL "No registered rea
add_test(NAME f3d::TestNoDryRun COMMAND $<TARGET_FILE:f3d> --no-render)
set_tests_properties(f3d::TestNoDryRun PROPERTIES TIMEOUT 4)

# Test invalid CLI args
add_test(NAME f3d::TestInvalidCLIArgs COMMAND $<TARGET_FILE:f3d> --up)
set_tests_properties(f3d::TestInvalidCLIArgs PROPERTIES PASS_REGULAR_EXPRESSION "Error parsing command line arguments")

# Test failure without a reference, please do not create a TestNoRef.png file
f3d_test(NAME TestNoRef DATA cow.vtp WILL_FAIL)

Expand Down
156 changes: 156 additions & 0 deletions cmake/f3dOptions.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
#[==[
@file f3dOptions.cmake

This module contains all the methods required to parse a JSON file specifying options
and generate the associated CXX code.
#]==]

#[==[
@brief generate public and private headers for a provided options.json

~~~
f3d_generate_options(
INPUT_JSON "path/to/options.json"
INPUT_PUBLIC_HEADER "path/to/options.h.in"
INPUT_PRIVATE_HEADER "path/to/options_tools.h.in"
DESTINATION "path/to/destination/folder"
OUTPUT_NAME "options"
)
~~~

#]==]

function (f3d_generate_options)
cmake_parse_arguments(PARSE_ARGV 0 _f3d_generate_options
""
"INPUT_JSON;INPUT_PUBLIC_HEADER;INPUT_PRIVATE_HEADER;DESTINATION;OUTPUT_NAME"
"")

if (_f3d_generate_options_UNPARSED_ARGUMENTS)
message(FATAL_ERROR
"Unparsed arguments for f3d_generate_options: "
"${_f3d_generate_options_UNPARSED_ARGUMENTS}")
endif ()

if (NOT DEFINED _f3d_generate_options_INPUT_JSON)
message(FATAL_ERROR
"Missing INPUT_JSON argument for f3d_generate_options")
endif ()

if (NOT DEFINED _f3d_generate_options_INPUT_PUBLIC_HEADER)
message(FATAL_ERROR
"Missing INPUT_PUBLIC_HEADER argument for f3d_generate_options")
endif ()

if (NOT DEFINED _f3d_generate_options_INPUT_PRIVATE_HEADER)
message(FATAL_ERROR
"Missing INPUT_PRIVATE_HEADER argument for f3d_generate_options")
endif ()

if (NOT DEFINED _f3d_generate_options_DESTINATION)
message(FATAL_ERROR
"Missing DESTINATION argument for f3d_generate_options")
endif ()

if (NOT DEFINED _f3d_generate_options_OUTPUT_NAME)
message(FATAL_ERROR
"Missing OUTPUT_NAME argument for f3d_generate_options")
endif ()


# Parse options.json and generate headers
set(_option_basename "")
set(_option_indent "")

# Add a configure depends on the input file
set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS ${_f3d_generate_options_INPUT_JSON})

## Read the .json file and complete the struct
file(READ ${_f3d_generate_options_INPUT_JSON} _options_json)
_parse_json_option(${_options_json})

list(JOIN _options_setter ";\n else " _options_setter)
list(JOIN _options_getter ";\n else " _options_getter)
list(JOIN _options_string_setter ";\n else " _options_string_setter)
list(JOIN _options_string_getter ";\n else " _options_string_getter)
list(JOIN _options_lister ",\n " _options_lister)

configure_file(
"${_f3d_generate_options_INPUT_PUBLIC_HEADER}"
"${_f3d_generate_options_DESTINATION}/public/${_f3d_generate_options_OUTPUT_NAME}.h")

configure_file(
"${_f3d_generate_options_INPUT_PRIVATE_HEADER}"
"${_f3d_generate_options_DESTINATION}/private/${_f3d_generate_options_OUTPUT_NAME}_tools.h")

endfunction()

#[==[
@brief internal recursive method use to parse json structure and generate variables
#]==]
function(_parse_json_option _top_json)
# TODO Add a deprecation mechanism
string(JSON _options_length LENGTH ${_top_json})
MATH(EXPR _options_length "${_options_length} - 1")
foreach(_json_idx RANGE ${_options_length})
# Recover the json element name
string(JSON _member_name MEMBER ${_top_json} ${_json_idx})
# Read the json element
string(JSON _cur_json GET ${_top_json} ${_member_name})
# Recover its type and default if it is a leaf option
string(JSON _option_type ERROR_VARIABLE _type_error GET ${_cur_json} "type")
string(JSON _option_default_value ERROR_VARIABLE _default_value_error GET ${_cur_json} "default_value")
if(_type_error STREQUAL "NOTFOUND" AND _default_value_error STREQUAL "NOTFOUND")
# Leaf option found!
set(_option_name "${_option_basename}${_member_name}")

# Identify types
set(_option_actual_type ${_option_type})
set(_option_variant_type ${_option_type})
set(_option_default_value_start "")
set(_option_default_value_end "")
if(_option_type STREQUAL "double_vector")
set(_option_actual_type "std::vector<double>")
set(_option_variant_type "std::vector<double>")
set(_option_default_value_start "{")
set(_option_default_value_end "}")
elseif(_option_type STREQUAL "string")
set(_option_actual_type "std::string")
set(_option_variant_type "std::string")
set(_option_default_value_start "\"")
set(_option_default_value_end "\"")
elseif(_option_type STREQUAL "ratio")
set(_option_actual_type "f3d::ratio_t")
set(_option_variant_type "double")
endif()

# Add option to struct and methods
string(APPEND _options_struct "${_option_indent} ${_option_actual_type} ${_member_name} = ${_option_default_value_start}${_option_default_value}${_option_default_value_end};\n")
list(APPEND _options_setter "if (name == \"${_option_name}\") opt.${_option_name} = std::get<${_option_variant_type}>(value)")
list(APPEND _options_getter "if (name == \"${_option_name}\") return opt.${_option_name}")
list(APPEND _options_string_setter "if (name == \"${_option_name}\") opt.${_option_name} = options_tools::parse<${_option_actual_type}>(str)")
list(APPEND _options_string_getter "if (name == \"${_option_name}\") return options_tools::format(opt.${_option_name})")
list(APPEND _options_lister "\"${_option_name}\"")

else()
# Group found, add in the struct and recurse
set(_option_prevname ${_option_basename})
set(_option_previndent ${_option_indent})
string(APPEND _option_indent " ")
string(APPEND _option_basename "${_member_name}.")
string(APPEND _options_struct "${_option_indent}struct ${_member_name} {\n")
_parse_json_option(${_cur_json})
string(APPEND _options_struct "${_option_indent}} ${_member_name};\n\n")
set(_option_indent ${_option_previndent})
set(_option_basename ${_option_prevname})
endif()
endforeach()
# Set appended variables and list in the parent before leaving the recursion
# Always use quotes for string variable as it contains semi-colons
set(_options_struct "${_options_struct}" PARENT_SCOPE)
set(_options_setter ${_options_setter} PARENT_SCOPE)
set(_options_getter ${_options_getter} PARENT_SCOPE)
set(_options_string_setter ${_options_string_setter} PARENT_SCOPE)
set(_options_string_getter ${_options_string_getter} PARENT_SCOPE)
set(_options_lister ${_options_lister} PARENT_SCOPE)
endfunction()
Loading

0 comments on commit de377bc

Please sign in to comment.