Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] issue 1637 #1720

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
+22 −19
Open

[FIX] issue 1637 #1720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
787dac4
[FIX] issue 1637
t-h2o Nov 16, 2024
24f00c5
no-render flag has its own variable
t-h2o Nov 18, 2024
32cd459
OptionsTools: change no-render description
t-h2o Nov 19, 2024
3cc382f
Starter: remove no-render
t-h2o Nov 19, 2024
60a63b8
rename dry-run by no-config
t-h2o Nov 19, 2024
b0ca199
rename dryRun by noConfig
t-h2o Nov 19, 2024
722952d
rename --dry-run by --no-config
t-h2o Nov 19, 2024
4dda7f6
rename dry-run by no-config
t-h2o Nov 19, 2024
fd03097
test: rename NoNoRender -> NoNoConfig
t-h2o Nov 20, 2024
d9b36bb
test: add NoConfigWithConfig
t-h2o Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/bug_report.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ A clear and concise description of what the bug is, a screenshot helps a lot.

**To Reproduce**
Steps to reproduce the behavior:
1. Open the file using `f3d --dry-run example.glb`
1. Open the file using `f3d --no-config example.glb`
2. step 2
3. step 3

Expand Down
4 changes: 2 additions & 2 deletions application/F3DOptionsTools.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ static inline const std::array<CLIGroup, 8> CLIOptions = {{
{ "readers-list", "", "Print the list of readers", "", "" },
{ "bindings-list", "", "Print the list of interaction bindings and exits, ignored with `--no-render`, only considers the first file group.", "", "" },
{ "config", "", "Specify the configuration file to use. absolute/relative path or filename/filestem to search in configuration file locations", "<filePath/filename/fileStem>", "" },
{ "dry-run", "", "Do not read the configuration file", "<bool>", "1" },
{ "no-render", "", "Do not read the configuration file", "<bool>", "1" },
{ "no-config", "", "Do not read the configuration file", "<bool>", "1" },
{ "no-render", "", "Do not render anything and quit right after loading the first file, use with --verbose to recover information about a file.", "<bool>", "1" },
{ "rendering-backend", "", "Backend to use when rendering (auto|glx|wgl|egl|osmesa)", "<string>", "" },
{ "max-size", "", "Maximum size in Mib of a file to load, leave empty for unlimited", "<size in Mib>", "" },
#if F3D_MODULE_DMON
Expand Down
2 changes: 1 addition & 1 deletion application/F3DOptionsTools.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ static inline const OptionsDict DefaultAppOptions = {
{ "bindings-list", "false" },
{ "no-background", "false" },
{ "config", "" },
{ "dry-run", "false" },
{ "no-config", "false" },
{ "no-render", "false" },
{ "rendering-backend", "auto" },
{ "max-size", "" },
Expand Down
12 changes: 6 additions & 6 deletions application/F3DStarter.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,17 +812,17 @@ int F3DStarter::Start(int argc, char** argv)
// Store in a option entries for easier processing
this->Internals->CLIOptionsEntries.emplace_back(cliOptionsDict, fs::path(), "CLI options");

// Check dry-run, config CLI, output and verbose options first
// Check no-config, config CLI, output and verbose options first
// XXX: the local variable are initialized manually for simplicity
// but this duplicate the initialization value as it is present in
// F3DOptionTools::DefaultAppOptions too
bool dryRun = false;
if (cliOptionsDict.find("no-render") != cliOptionsDict.end())
bool noConfig = false;
if (cliOptionsDict.find("no-config") != cliOptionsDict.end())
{
dryRun = f3d::options::parse<bool>(cliOptionsDict["no-render"]);
noConfig = f3d::options::parse<bool>(cliOptionsDict["no-config"]);
}
mwestphal marked this conversation as resolved.
Show resolved Hide resolved
std::string config;
if (cliOptionsDict.find("config") != cliOptionsDict.end())
if (!noConfig && cliOptionsDict.find("config") != cliOptionsDict.end())
{
config = f3d::options::parse<std::string>(cliOptionsDict["config"]);
}
Expand All @@ -844,7 +844,7 @@ int F3DStarter::Start(int argc, char** argv)
f3d::log::debug("========== Initializing Options ==========");

// Read config files
if (!dryRun)
if (!noConfig)
{
std::tie(this->Internals->ConfigOptionsEntries, this->Internals->ConfigBindingsEntries) =
F3DConfigFileTools::ReadConfigFiles(config);
Expand Down
15 changes: 9 additions & 6 deletions application/testing/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ function(f3d_test)
if(F3D_TEST_CONFIG)
list(APPEND F3D_TEST_ARGS "--config=${F3D_TEST_CONFIG}")
else()
list(APPEND F3D_TEST_ARGS "--dry-run")
list(APPEND F3D_TEST_ARGS "--no-config")
endif()

if (F3D_TEST_DATA)
Expand Down Expand Up @@ -261,17 +261,17 @@ endif()
function(f3d_ss_test)
cmake_parse_arguments(F3D_SS_TEST "MINIMAL" "NAME;TEMPLATE;EXPECTED;DEPENDS" "ARGS" ${ARGN})
if(NOT F3D_SS_TEST_MINIMAL)
f3d_test(NAME TestScreenshot${F3D_SS_TEST_NAME} DATA suzanne.ply ARGS --screenshot-filename=${F3D_SS_TEST_TEMPLATE} --dry-run --interaction-test-play=${F3D_SOURCE_DIR}/testing/recordings/TestScreenshot.log NO_BASELINE DEPENDS TestSetupScreenshots)
f3d_test(NAME TestScreenshot${F3D_SS_TEST_NAME} DATA suzanne.ply ARGS --screenshot-filename=${F3D_SS_TEST_TEMPLATE} --no-config --interaction-test-play=${F3D_SOURCE_DIR}/testing/recordings/TestScreenshot.log NO_BASELINE DEPENDS TestSetupScreenshots)
f3d_test(NAME TestScreenshot${F3D_SS_TEST_NAME}File DATA suzanne.ply ARGS --reference=${F3D_SS_TEST_EXPECTED} DEPENDS TestScreenshot${F3D_SS_TEST_NAME} ${F3D_SS_TEST_DEPENDS} NO_BASELINE)
else()
# show filename, axes, fps before the "minimal screenshot" interaction; compare with --no-background only
f3d_test(NAME TestScreenshot${F3D_SS_TEST_NAME} DATA suzanne.ply ARGS --screenshot-filename=${F3D_SS_TEST_TEMPLATE} --dry-run -nxz --interaction-test-play=${F3D_SOURCE_DIR}/testing/recordings/TestScreenshotMinimal.log NO_BASELINE DEPENDS TestSetupScreenshots)
f3d_test(NAME TestScreenshot${F3D_SS_TEST_NAME} DATA suzanne.ply ARGS --screenshot-filename=${F3D_SS_TEST_TEMPLATE} --no-config -nxz --interaction-test-play=${F3D_SOURCE_DIR}/testing/recordings/TestScreenshotMinimal.log NO_BASELINE DEPENDS TestSetupScreenshots)
f3d_test(NAME TestScreenshot${F3D_SS_TEST_NAME}File DATA suzanne.ply ARGS --no-background --reference=${F3D_SS_TEST_EXPECTED} DEPENDS TestScreenshot${F3D_SS_TEST_NAME} ${F3D_SS_TEST_DEPENDS} NO_BASELINE)
endif()
endfunction()
function(f3d_ss_template_test)
cmake_parse_arguments(F3D_SS_TEMPLATE_TEST "" "NAME;TEMPLATE;EXPECTED_REGEX" "ARGS" ${ARGN})
f3d_test(NAME TestScreenshot${F3D_SS_TEMPLATE_TEST_NAME} DATA suzanne.ply ARGS --screenshot-filename=${F3D_SS_TEMPLATE_TEST_TEMPLATE} --dry-run --interaction-test-play=${F3D_SOURCE_DIR}/testing/recordings/TestScreenshot.log
f3d_test(NAME TestScreenshot${F3D_SS_TEMPLATE_TEST_NAME} DATA suzanne.ply ARGS --screenshot-filename=${F3D_SS_TEMPLATE_TEST_TEMPLATE} --no-config --interaction-test-play=${F3D_SOURCE_DIR}/testing/recordings/TestScreenshot.log
REGEXP "saving screenshot to .+[/\\]${F3D_SS_TEMPLATE_TEST_EXPECTED_REGEX}" NO_BASELINE DEPENDS TestSetupScreenshots)
endfunction()

Expand Down Expand Up @@ -1049,6 +1049,9 @@ f3d_test(NAME TestConfigFileQuiet DATA nonExistentFile.vtp CONFIG ${F3D_SOURCE_D
# Test no file with config file
f3d_test(NAME TestNoFileConfigFile CONFIG ${F3D_SOURCE_DIR}/testing/configs/verbose.json ARGS --verbose REGEXP "No files to load provided" NO_BASELINE)

# Test that --no-config set no-config value to 1
f3d_test(NAME TestNoConfigWithConfig ARGS --no-config --config=config_build --verbose REGEXP "'no-config' = '1'" NO_BASELINE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of testing the verbosity of no-config, how about opening a file a taking a screenshot ? This way we ensure the config_build is not taken into account

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense.


# Test help display
f3d_test(NAME TestHelp ARGS --help REGEXP "Usage:")
f3d_test(NAME TestHelpPositional ARGS --help REGEXP "file1 file2 \.\.\.")
Expand Down Expand Up @@ -1104,8 +1107,8 @@ endif()
f3d_test(NAME TestRenderingBackendInvalid DATA cow.vtp RENDERING_BACKEND invalid ARGS --verbose REGEXP "rendering-backend value is invalid, falling back to" NO_BASELINE)

# Test that f3d can try to read a system config file
add_test(NAME f3d::TestNoDryRun COMMAND $<TARGET_FILE:f3d> --no-render)
set_tests_properties(f3d::TestNoDryRun PROPERTIES TIMEOUT 4)
add_test(NAME f3d::TestNoNoConfig COMMAND $<TARGET_FILE:f3d> --no-render)
set_tests_properties(f3d::TestNoNoConfig PROPERTIES TIMEOUT 4)

mwestphal marked this conversation as resolved.
Show resolved Hide resolved
# Test invalid CLI args
add_test(NAME f3d::TestInvalidCLIArgs COMMAND $<TARGET_FILE:f3d> --up)
Expand Down
2 changes: 1 addition & 1 deletion doc/user/CONFIGURATION_FILE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ The third block specifies raytracing usage for .gltf and .glb files.
The last block specifies that volume rendering should be used with .mhd files.

The following options <b> cannot </b> be set via config file:
`help`, `version`, `readers-list`, `config`, `dry-run` and `input`.
`help`, `version`, `readers-list`, `config`, `no-config` and `input`.

The following options <b>are only taken on the first load</b>:
`no-render`, `output`, `position`, `resolution`, `frame-rate` and all testing options.
Expand Down
2 changes: 1 addition & 1 deletion doc/user/OPTIONS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Options|Default|Description
\-\-version||Show *version* information and exit. Ignore `--verbose`.
\-\-readers-list||List available *readers* and exit. Ignore `--verbose`.
\-\-config=\<config file path/name/stem\>|config|Specify the [configuration file](CONFIGURATION_FILE.md) to use. Supports absolute/relative path but also filename/filestem to search for in standard configuration file locations.
\-\-dry-run||Do not read any configuration file and consider only the command line options.
\-\-no-config||Do not read any configuration file and consider only the command line options.
\-\-no-render||Do not render anything and quit just after loading the first file, use with \-\-verbose to recover information about a file.
\-\-max-size=\<size in MiB\>||Prevent F3D to load a file bigger than the provided size in Mib, leave empty for unlimited, useful for thumbnails.
\-\-watch||Watch current file and automatically reload it whenever it is modified on disk.
Expand Down
2 changes: 1 addition & 1 deletion examples/plugins/example-plugin/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ if(BUILD_TESTING)
find_package(f3d REQUIRED COMPONENTS application)
add_test(NAME TestPluginExample
COMMAND "$<TARGET_FILE:f3d::f3d>"
"--dry-run"
"--no-config"
"--no-render"
"--verbose"
"--load-plugins=$<TARGET_FILE:f3d-plugin-example>"
Expand Down
Loading