Skip to content

Commit 92845d5

Browse files
authored
Complete refactor of options API (#1471)
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: #1568 - Rework application and simplify option logic: #1569 - Add more options types : #1570 - Add actual parsing for all options types: #1571 - Add complete documentation for options and option parsing: #1572 - Proper java and javascript bindings: #1573 #1574 - use exception translator in python bindings: #1575 - Improve compile-time opti in options_tools.h.in: #1576
1 parent 5d89ea3 commit 92845d5

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+1813
-1052
lines changed

.github/workflows/style-checks.yml

+3-2
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ jobs:
1010
- name: C++ Formatting
1111
uses: DoozyX/[email protected]
1212
with:
13-
source: 'library application python plugins'
14-
extensions: 'h,cxx'
13+
source: 'library application python plugins examples'
14+
extensions: 'h,cxx,in'
15+
exclude: '**/*.py.in'
1516
clangFormatVersion: 14 # Last Ubuntu LTS version (22.04)
1617
- name: Python Formatting
1718
uses: psf/black@stable

application/F3DConfig.h.in

+5-5
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55

66
namespace F3D
77
{
8-
const std::string AppName = "@PROJECT_NAME@";
9-
const std::string AppTitle = "@PROJECT_DESCRIPTION@";
10-
const std::string AppVersion = "@F3D_VERSION@";
11-
const std::string AppVersionFull = "@F3D_VERSION_FULL@";
12-
const std::string PluginsInstallDir = "@F3D_PLUGINS_INSTALL_DIR@";
8+
const std::string AppName = "@PROJECT_NAME@";
9+
const std::string AppTitle = "@PROJECT_DESCRIPTION@";
10+
const std::string AppVersion = "@F3D_VERSION@";
11+
const std::string AppVersionFull = "@F3D_VERSION_FULL@";
12+
const std::string PluginsInstallDir = "@F3D_PLUGINS_INSTALL_DIR@";
1313
}
1414

1515
// TODO: Use CMake definitions and get rid of these

application/F3DOptionsParser.cxx

+79-65
Large diffs are not rendered by default.

application/F3DStarter.cxx

+21-19
Original file line numberDiff line numberDiff line change
@@ -383,8 +383,8 @@ F3DStarter::F3DStarter()
383383
: Internals(std::make_unique<F3DStarter::F3DInternals>())
384384
{
385385
// Set option outside of command line and config file
386-
this->Internals->DynamicOptions.set(
387-
"ui.dropzone-info", "Drop a file or HDRI to load it\nPress H to show cheatsheet");
386+
this->Internals->DynamicOptions.ui.dropzone_info =
387+
"Drop a file or HDRI to load it\nPress H to show cheatsheet";
388388

389389
// Initialize dmon
390390
dmon_init();
@@ -510,9 +510,10 @@ int F3DStarter::Start(int argc, char** argv)
510510
// TODO: add a image::canRead
511511

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

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

558559
if (!fullPath.empty())
559560
{
560-
this->Internals->Engine->getOptions().set(
561-
"model.scivis.colormap", F3DColorMapTools::Read(fullPath));
561+
this->Internals->Engine->getOptions().model.scivis.colormap =
562+
F3DColorMapTools::Read(fullPath);
562563
}
563564
else
564565
{
565566
f3d::log::error("Cannot find the colormap ", this->Internals->AppOptions.ColorMapFile);
566-
this->Internals->Engine->getOptions().set("model.scivis.colormap", std::vector<double>{});
567+
this->Internals->Engine->getOptions().model.scivis.colormap = std::vector<double>{};
567568
}
568569
}
569570

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

897-
this->Internals->Engine->getOptions().set("ui.dropzone", !this->Internals->LoadedFile);
898-
this->Internals->Engine->getOptions().set("ui.filename-info", filenameInfo);
898+
f3d::options& options = this->Internals->Engine->getOptions();
899+
options.ui.dropzone = !this->Internals->LoadedFile;
900+
options.ui.filename_info = filenameInfo;
899901
}
900902

901903
//----------------------------------------------------------------------------
@@ -949,22 +951,22 @@ void F3DStarter::SaveScreenshot(const std::string& filenameTemplate, bool minima
949951
bool noBackground = this->Internals->AppOptions.NoBackground;
950952
if (minimal)
951953
{
952-
options.set("ui.bar", false);
953-
options.set("ui.cheatsheet", false);
954-
options.set("ui.filename", false);
955-
options.set("ui.fps", false);
956-
options.set("ui.metadata", false);
957-
options.set("ui.animation-progress", false);
958-
options.set("interactor.axis", false);
959-
options.set("render.grid.enable", false);
954+
options.ui.scalar_bar = false;
955+
options.ui.cheatsheet = false;
956+
options.ui.filename = false;
957+
options.ui.fps = false;
958+
options.ui.metadata = false;
959+
options.ui.animation_progress = false;
960+
options.interactor.axis = false;
961+
options.render.grid.enable = false;
960962
noBackground = true;
961963
}
962964

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

967-
options.getAsDoubleRef("render.light.intensity") *= 5;
969+
options.render.light.intensity *= 5;
968970
this->Render();
969971

970972
this->Internals->Engine->setOptions(optionsCopy);

application/testing/CMakeLists.txt

+4
Original file line numberDiff line numberDiff line change
@@ -915,6 +915,10 @@ f3d_test(NAME TestReadersList ARGS --readers-list REGEXP_FAIL "No registered rea
915915
add_test(NAME f3d::TestNoDryRun COMMAND $<TARGET_FILE:f3d> --no-render)
916916
set_tests_properties(f3d::TestNoDryRun PROPERTIES TIMEOUT 4)
917917

918+
# Test invalid CLI args
919+
add_test(NAME f3d::TestInvalidCLIArgs COMMAND $<TARGET_FILE:f3d> --up)
920+
set_tests_properties(f3d::TestInvalidCLIArgs PROPERTIES PASS_REGULAR_EXPRESSION "Error parsing command line arguments")
921+
918922
# Test failure without a reference, please do not create a TestNoRef.png file
919923
f3d_test(NAME TestNoRef DATA cow.vtp WILL_FAIL)
920924

cmake/f3dOptions.cmake

+156
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
#[==[
2+
@file f3dOptions.cmake
3+
4+
This module contains all the methods required to parse a JSON file specifying options
5+
and generate the associated CXX code.
6+
#]==]
7+
8+
#[==[
9+
@brief generate public and private headers for a provided options.json
10+
11+
~~~
12+
f3d_generate_options(
13+
INPUT_JSON "path/to/options.json"
14+
INPUT_PUBLIC_HEADER "path/to/options.h.in"
15+
INPUT_PRIVATE_HEADER "path/to/options_tools.h.in"
16+
DESTINATION "path/to/destination/folder"
17+
OUTPUT_NAME "options"
18+
)
19+
~~~
20+
21+
#]==]
22+
23+
function (f3d_generate_options)
24+
cmake_parse_arguments(PARSE_ARGV 0 _f3d_generate_options
25+
""
26+
"INPUT_JSON;INPUT_PUBLIC_HEADER;INPUT_PRIVATE_HEADER;DESTINATION;OUTPUT_NAME"
27+
"")
28+
29+
if (_f3d_generate_options_UNPARSED_ARGUMENTS)
30+
message(FATAL_ERROR
31+
"Unparsed arguments for f3d_generate_options: "
32+
"${_f3d_generate_options_UNPARSED_ARGUMENTS}")
33+
endif ()
34+
35+
if (NOT DEFINED _f3d_generate_options_INPUT_JSON)
36+
message(FATAL_ERROR
37+
"Missing INPUT_JSON argument for f3d_generate_options")
38+
endif ()
39+
40+
if (NOT DEFINED _f3d_generate_options_INPUT_PUBLIC_HEADER)
41+
message(FATAL_ERROR
42+
"Missing INPUT_PUBLIC_HEADER argument for f3d_generate_options")
43+
endif ()
44+
45+
if (NOT DEFINED _f3d_generate_options_INPUT_PRIVATE_HEADER)
46+
message(FATAL_ERROR
47+
"Missing INPUT_PRIVATE_HEADER argument for f3d_generate_options")
48+
endif ()
49+
50+
if (NOT DEFINED _f3d_generate_options_DESTINATION)
51+
message(FATAL_ERROR
52+
"Missing DESTINATION argument for f3d_generate_options")
53+
endif ()
54+
55+
if (NOT DEFINED _f3d_generate_options_OUTPUT_NAME)
56+
message(FATAL_ERROR
57+
"Missing OUTPUT_NAME argument for f3d_generate_options")
58+
endif ()
59+
60+
61+
# Parse options.json and generate headers
62+
set(_option_basename "")
63+
set(_option_indent "")
64+
65+
# Add a configure depends on the input file
66+
set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS ${_f3d_generate_options_INPUT_JSON})
67+
68+
## Read the .json file and complete the struct
69+
file(READ ${_f3d_generate_options_INPUT_JSON} _options_json)
70+
_parse_json_option(${_options_json})
71+
72+
list(JOIN _options_setter ";\n else " _options_setter)
73+
list(JOIN _options_getter ";\n else " _options_getter)
74+
list(JOIN _options_string_setter ";\n else " _options_string_setter)
75+
list(JOIN _options_string_getter ";\n else " _options_string_getter)
76+
list(JOIN _options_lister ",\n " _options_lister)
77+
78+
configure_file(
79+
"${_f3d_generate_options_INPUT_PUBLIC_HEADER}"
80+
"${_f3d_generate_options_DESTINATION}/public/${_f3d_generate_options_OUTPUT_NAME}.h")
81+
82+
configure_file(
83+
"${_f3d_generate_options_INPUT_PRIVATE_HEADER}"
84+
"${_f3d_generate_options_DESTINATION}/private/${_f3d_generate_options_OUTPUT_NAME}_tools.h")
85+
86+
endfunction()
87+
88+
#[==[
89+
@brief internal recursive method use to parse json structure and generate variables
90+
#]==]
91+
function(_parse_json_option _top_json)
92+
# TODO Add a deprecation mechanism
93+
string(JSON _options_length LENGTH ${_top_json})
94+
MATH(EXPR _options_length "${_options_length} - 1")
95+
foreach(_json_idx RANGE ${_options_length})
96+
# Recover the json element name
97+
string(JSON _member_name MEMBER ${_top_json} ${_json_idx})
98+
# Read the json element
99+
string(JSON _cur_json GET ${_top_json} ${_member_name})
100+
# Recover its type and default if it is a leaf option
101+
string(JSON _option_type ERROR_VARIABLE _type_error GET ${_cur_json} "type")
102+
string(JSON _option_default_value ERROR_VARIABLE _default_value_error GET ${_cur_json} "default_value")
103+
if(_type_error STREQUAL "NOTFOUND" AND _default_value_error STREQUAL "NOTFOUND")
104+
# Leaf option found!
105+
set(_option_name "${_option_basename}${_member_name}")
106+
107+
# Identify types
108+
set(_option_actual_type ${_option_type})
109+
set(_option_variant_type ${_option_type})
110+
set(_option_default_value_start "")
111+
set(_option_default_value_end "")
112+
if(_option_type STREQUAL "double_vector")
113+
set(_option_actual_type "std::vector<double>")
114+
set(_option_variant_type "std::vector<double>")
115+
set(_option_default_value_start "{")
116+
set(_option_default_value_end "}")
117+
elseif(_option_type STREQUAL "string")
118+
set(_option_actual_type "std::string")
119+
set(_option_variant_type "std::string")
120+
set(_option_default_value_start "\"")
121+
set(_option_default_value_end "\"")
122+
elseif(_option_type STREQUAL "ratio")
123+
set(_option_actual_type "f3d::ratio_t")
124+
set(_option_variant_type "double")
125+
endif()
126+
127+
# Add option to struct and methods
128+
string(APPEND _options_struct "${_option_indent} ${_option_actual_type} ${_member_name} = ${_option_default_value_start}${_option_default_value}${_option_default_value_end};\n")
129+
list(APPEND _options_setter "if (name == \"${_option_name}\") opt.${_option_name} = std::get<${_option_variant_type}>(value)")
130+
list(APPEND _options_getter "if (name == \"${_option_name}\") return opt.${_option_name}")
131+
list(APPEND _options_string_setter "if (name == \"${_option_name}\") opt.${_option_name} = options_tools::parse<${_option_actual_type}>(str)")
132+
list(APPEND _options_string_getter "if (name == \"${_option_name}\") return options_tools::format(opt.${_option_name})")
133+
list(APPEND _options_lister "\"${_option_name}\"")
134+
135+
else()
136+
# Group found, add in the struct and recurse
137+
set(_option_prevname ${_option_basename})
138+
set(_option_previndent ${_option_indent})
139+
string(APPEND _option_indent " ")
140+
string(APPEND _option_basename "${_member_name}.")
141+
string(APPEND _options_struct "${_option_indent}struct ${_member_name} {\n")
142+
_parse_json_option(${_cur_json})
143+
string(APPEND _options_struct "${_option_indent}} ${_member_name};\n\n")
144+
set(_option_indent ${_option_previndent})
145+
set(_option_basename ${_option_prevname})
146+
endif()
147+
endforeach()
148+
# Set appended variables and list in the parent before leaving the recursion
149+
# Always use quotes for string variable as it contains semi-colons
150+
set(_options_struct "${_options_struct}" PARENT_SCOPE)
151+
set(_options_setter ${_options_setter} PARENT_SCOPE)
152+
set(_options_getter ${_options_getter} PARENT_SCOPE)
153+
set(_options_string_setter ${_options_string_setter} PARENT_SCOPE)
154+
set(_options_string_getter ${_options_string_getter} PARENT_SCOPE)
155+
set(_options_lister ${_options_lister} PARENT_SCOPE)
156+
endfunction()

0 commit comments

Comments
 (0)