Skip to content

Cli reader options #1860

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

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Cli reader options #1860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
917db39
implemented reader option
0xfedcafe Jan 2, 2025
a039ed0
Merge branch 'f3d-app:master' into cli-reader-options
0xfedcafe Jan 2, 2025
c85a20c
added test & bugfix
0xfedcafe Jan 3, 2025
45ba1a9
spelling correction
0xfedcafe Jan 3, 2025
5b00fcb
reformat
0xfedcafe Jan 3, 2025
be829e3
Merge branch 'f3d-app:master' into cli-reader-options
0xfedcafe Jan 3, 2025
e916daf
force-reader to scene & fixes
0xfedcafe Jan 3, 2025
17c2dda
bugfix
0xfedcafe Jan 3, 2025
8e69e60
Merge branch 'f3d-app:master' into cli-reader-options
0xfedcafe Jan 14, 2025
bc26894
reader: Applied suggestions & refactor
0xfedcafe Jan 14, 2025
fc68b05
Merge branch 'master' into cli-reader-options
0xfedcafe Mar 3, 2025
d260c55
apllication tests for force-reader
0xfedcafe Mar 3, 2025
c93e954
reformat
0xfedcafe Mar 3, 2025
63c0456
test fix
0xfedcafe Mar 12, 2025
9a5bca2
Apply suggestions from code review
0xfedcafe Mar 13, 2025
4bb1ce4
Merge branch 'master' into cli-reader-options
0xfedcafe Mar 25, 2025
2db35e2
fix
0xfedcafe Mar 25, 2025
dbaf7ba
Removed unnecessary test
0xfedcafe Apr 21, 2025
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
1 change: 1 addition & 0 deletions application/F3DOptionsTools.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ static inline const std::array<CLIGroup, 8> CLIOptions = {{
{ "no-background", "", "No background when render to file", "<bool>", "1" },
{ "help", "h", "Print help", "", "" }, { "version", "", "Print version details", "", "" },
{ "list-readers", "", "Print the list of readers", "", "" },
{ "force-reader", "", "Force a specific reader to be used, disrigarding the file extension", "<reader>", "1"},
{ "list-bindings", "", "Print the list of interaction bindings and exits, ignored with `--no-render`, only considers the first file group.", "<bool>", "1" },
{ "config", "", "Specify the configuration file to use. absolute/relative path or filename/filestem to search in configuration file locations", "<filePath/filename/fileStem>", "" },
{ "no-config", "", "Do not read the configuration file", "<bool>", "1" },
Expand Down
1 change: 1 addition & 0 deletions application/F3DOptionsTools.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ static inline const std::map<std::string_view, std::string_view> LibOptionsNames
{ "animation-autoplay", "scene.animation.autoplay" },
{ "animation-index", "scene.animation.index" },
{ "animation-speed-factor", "scene.animation.speed_factor" },
{ "force-reader", "scene.force_reader" },
{ "font-file", "ui.font_file" },
{ "font-scale", "ui.scale" },
{ "point-sprites", "model.point_sprites.enable" },
Expand Down
10 changes: 10 additions & 0 deletions application/testing/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1159,6 +1159,16 @@ if(F3D_PLUGIN_BUILD_ALEMBIC AND F3D_PLUGIN_BUILD_ASSIMP)
f3d_test(NAME TestReadersListMultiplePlugins ARGS --list-readers --load-plugins=assimp,alembic NO_BASELINE REGEXP_FAIL "Plugin failed to load")
endif()

if(F3D_PLUGIN_BUILD_EXODUS)
f3d_test(NAME TestForceReaderExodusFail DATA BoxAnimated.gltf ARGS --load-plugins=exodus --force-reader=ExodusII NO_BASELINE REGEXP "is not a file of a supported file format|failed to load scene")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
f3d_test(NAME TestForceReaderExodusFail DATA BoxAnimated.gltf ARGS --load-plugins=exodus --force-reader=ExodusII NO_BASELINE REGEXP "is not a file of a supported file format|failed to load scene")
f3d_test(NAME TestForceReaderExodusFail DATA BoxAnimated.gltf ARGS --load-plugins=exodus --force-reader=ExodusII NO_BASELINE REGEXP "failed to load scene")

f3d_test(NAME TestForceReaderExodusPass DATA disk_out_ref.ex2 ARGS --load-plugins=exodus --force-reader=ExodusII NO_BASELINE REGEXP_FAIL "is not a file of a supported file format|failed to load scene")
Copy link
Member

Choose a reason for hiding this comment

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

you need to put these test behind a if(VTK_VERSION VERSION_GREATER_EQUAL 9.3.20240707) because before that VTK did not provide feedback if a importer worked or not.

Copy link
Member

Choose a reason for hiding this comment

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

but these tests should not rely on exodus plugin being present tbh, you can just test using native provided importer, like obj and stl, eg;

 f3d_test(NAME TestForceReaderFail DATA suzanne.stl ARGS --force-reader=OBJ NO_BASELINE REGEXP "failed to load scene")
f3d_test(NAME TestForceReaderPass DATA suzanne.stl ARGS --force-reader=STL NO_BASELINE REGEXP "some log that you need to add")
f3d_test(NAME TestForceReaderInvalid DATA suzanne.stl ARGS --force-reader=INVALID NO_BASELINE REGEXP "some log that you need to add")

endif()

if(F3D_PLUGIN_BUILD_DRACO)
f3d_test(NAME TestForceReaderGLTFDraco DATA BoxAnimated.gltf ARGS --load-plugins=draco --force-reader=GLTFDraco NO_BASELINE REGEXP_FAIL "is not a file of a supported file format|failed to load scene")
f3d_test(NAME TestForceReaderGLTF DATA BoxAnimated.gltf ARGS --load-plugins=draco --force-reader=GLTF NO_BASELINE REGEXP_FAIL "is not a file of a supported file format|failed to load scene")
Copy link
Member

Choose a reason for hiding this comment

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

I dont get what these two tests are testing here. I think you want a test like this:

 f3d_test(NAME TestForceReaderGLTFDracoIntoGLTF DATA Box_draco.gltf ARGS --load-plugins=draco --force-reader=GLTF NO_BASELINE REGEXP "failed to load scene")

Basically, testing that you can force the usage of the GLTF importer instead of the GLTFDraco importer even when the draco importer is loaded. The test then check that the file is not loaded at all because it needs GLTFDraco to be able to load.

endif()

# Test bindings-list display
f3d_test(NAME TestBindingsList ARGS --list-bindings REGEXP "Any.5 Toggle Orthographic Projection")

Expand Down
1 change: 1 addition & 0 deletions doc/libf3d/OPTIONS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ See the [APIs](#APIs) details below for more info.
| scene.animation.time | double<br>optional<br>load | Set the animation time to load. | \-\-animation-time |
| scene.camera.index | int<br>optional<br>load | Select the scene camera to use when available in the file.<br>The default scene always uses automatic camera. | \-\-camera-index |
| scene.up_direction | direction<br>+Y<br>load | Define the Up direction. It impacts the grid, the axis, the HDRI and the camera. | \-\-up |
| scene.force_reader | string<br>optional<br>load | Force a specific reader to be used, disregarding the file extension. | \-\-force-reader |
| scene.camera.orthographic | bool<br>optional<br>load | Set to true to force orthographic projection. Model specified by default, which is false if not specified. | \-\-camera\-orthographic |

## Interactor Options
Expand Down
1 change: 1 addition & 0 deletions doc/user/OPTIONS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ F3D behavior can be fully controlled from the command line using the following o
| -h, \-\-help | | Print _help_ and exit. Ignore `--verbose`. |
| \-\-version | | Show _version_ information and exit. Ignore `--verbose`. |
| \-\-list-readers | | List available _readers_ and exit. Ignore `--verbose`. |
| \-\-force-reader=\<reader\> | string<br>- | Force a specific reader to be used, disregarding the file extension. |
| \-\-list-bindings | | List available _bindings_ and exit. Ignore `--verbose`. |
| \-\-list-rendering-backends | | List available _rendering backends_ and exit. Ignore `--verbose`. |
| \-\-config=\<config file path/name/stem\> | string<br>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. |
Expand Down
3 changes: 3 additions & 0 deletions library/options.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
"orthographic": {
"type": "bool"
}
},
"force_reader": {
"type": "string"
}
},
"render": {
Expand Down
10 changes: 5 additions & 5 deletions library/plugin/plugin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,31 +37,31 @@ class plugin
/**
* Get the name of this plugin
*/
const std::string& getName()
const std::string& getName() const
{
return this->Name;
}

/**
* Get the description of this plugin
*/
const std::string& getDescription()
const std::string& getDescription() const
{
return this->Description;
}

/**
* Get the version of this plugin
*/
const std::string& getVersion()
const std::string& getVersion() const
{
return this->Version;
}

/**
* Get the list of readers created by this plugin
*/
const std::vector<std::shared_ptr<reader>>& getReaders()
const std::vector<std::shared_ptr<reader>>& getReaders() const
{
return this->Readers;
}
Expand All @@ -71,7 +71,7 @@ class plugin
* Set/Get the origin of this plugin, usually static, system or an actual path
* Set by the engine.
*/
const std::string& getOrigin()
const std::string& getOrigin() const
{
return this->Origin;
}
Expand Down
4 changes: 3 additions & 1 deletion library/private/factory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include "reader.h"

#include <map>
#include <optional>
#include <string_view>
#include <vector>

namespace f3d
Expand Down Expand Up @@ -44,7 +46,7 @@ class factory
/**
* Get the reader that can read the given file, nullptr if none
*/
reader* getReader(const std::string& fileName);
reader* getReader(const std::string& fileName, std::optional<std::string> forceReader);

/**
* Get the list of the registered plugins
Expand Down
13 changes: 10 additions & 3 deletions library/src/factory.cxx.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,23 @@ factory::plugin_initializer_t factory::getStaticInitializer(const std::string& p
}

//----------------------------------------------------------------------------
reader* factory::getReader(const std::string& fileName)
reader* factory::getReader(const std::string& fileName, std::optional<std::string> forceReader)
{
int bestScore = -1;
reader* bestReader = nullptr;

for (const auto& plugin : this->Plugins)
for (const auto* plugin : this->Plugins)
{
for (const auto& reader : plugin->getReaders())
{
if (reader->getScore() > bestScore && reader->canRead(fileName))
if (forceReader.has_value())
{
if (reader->getName() == *forceReader)
{
return reader.get();
}
}
else if (reader->getScore() > bestScore && reader->canRead(fileName))
{
bestScore = reader->getScore();
bestReader = reader.get();
Expand Down
8 changes: 6 additions & 2 deletions library/src/scene_impl.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@
#include "interactor_impl.h"
#include "log.h"
#include "options.h"
#include "scene.h"
#include "window_impl.h"

#include "factory.h"
#include "vtkF3DGenericImporter.h"
#include "vtkF3DMemoryMesh.h"
#include "vtkF3DMetaImporter.h"

#include <optional>
#include <vtkCallbackCommand.h>
#include <vtkProgressBarRepresentation.h>
#include <vtkProgressBarWidget.h>
Expand Down Expand Up @@ -240,7 +242,8 @@ scene& scene_impl::add(const std::vector<fs::path>& filePaths)
}

// Recover the importer for the provided file path
f3d::reader* reader = f3d::factory::instance()->getReader(filePath.string());
f3d::reader* reader = f3d::factory::instance()->getReader(
filePath.string(), this->Internals->Options.scene.force_reader);
if (reader)
{
log::debug(
Copy link
Member

Choose a reason for hiding this comment

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

maybe improve this log to dinstinguish between:

"Found a reader for .."

"Forcing BLABLA reader for"

Copy link
Member

Choose a reason for hiding this comment

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

and maybe the exception below should also be improved, to dinstinguish when a reader was not found vs when the forced reader did not exists at all.

Expand Down Expand Up @@ -322,7 +325,8 @@ scene& scene_impl::clear()
//----------------------------------------------------------------------------
bool scene_impl::supports(const fs::path& filePath)
{
return f3d::factory::instance()->getReader(filePath.string()) != nullptr;
return f3d::factory::instance()->getReader(
filePath.string(), this->Internals->Options.scene.force_reader) != nullptr;
}

//----------------------------------------------------------------------------
Expand Down
Loading