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.

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

"multi-file" file loading API could be moved to F3D application #597

Closed
mwestphal opened this issue Feb 1, 2023 · 16 comments · Fixed by #634
Closed

"multi-file" file loading API could be moved to F3D application #597

mwestphal opened this issue Feb 1, 2023 · 16 comments · Fixed by #634
Assignees
Labels
source:libf3d type:enhancement New feature or request
Milestone

Comments

@mwestphal
Copy link
Contributor

The multi-file part of the loader API is quite specific to F3D application so it could make sens to move it there and provide a simpler API in libf3d. eg:

The loading API could be reduced to:

  • loadModel(filename, options) clear scene, load a model from disk
  • loadModel(buffer, options) clear scene, load a model from memory
@mwestphal
Copy link
Contributor Author

I've been thinking about this and starting to like the idea, especially in regards to #127.

Here is what we could do:

  • drop geometry-only f3d::options
  • modify f3d::loader API like this:
loader::loadGeometry(std::string file, bool resetScene)
loader::loadScene(std::string file)
  • implement the resetScene == false case in the vtkF3DGenericImporter by using multiple reader when needed in the default scene

  • Move multi files management to F3D Application and use loadGeometry/loadScene accordingly to --geometry-only option

  • Add a --multi-files-mode command line option (global options only) with following possible parameters:

  1. switch: current behavior, when loading multiples files, one can switch between files using the arrow keys. default.
  2. load: load all provided files as multiple models in the same default scene.
  3. folder: show all files in a folder view (cf https://user-images.githubusercontent.com/3129530/145460406-bc9b190a-ad7b-4b45-80d3-95cd01f49bcc.png) will not be implemented right away.

wdyt @snoyer @Meakk @nermolov @SamuelTallet ?

@mwestphal
Copy link
Contributor Author

FYI @GitCodeBoy

@SamuelTallet
Copy link

"Multi files mode" option is a good idea! 🙂

@snoyer
Copy link
Contributor

snoyer commented Feb 2, 2023

* Add a `--multi-files-mode` command line option (global options only) with following possible parameters:


1. switch: current behavior, when loading multiples files, one can switch between files using the arrow keys. default.

2. load: load all provided files as multiple models in the same default scene.

3. folder: show all files in a folder view (cf https://user-images.githubusercontent.com/3129530/145460406-bc9b190a-ad7b-4b45-80d3-95cd01f49bcc.png) will not be implemented right away.

These points all feel orthogonal to one another.

The "folder view" sounds like an alternative way to render/navigate, independently of the multi-file stuff (ie. the folder view could be implemented with the current file loading as an alternative to left/right arrows if I'm undertanding correctly).

I think the general multi-file case should be somehow allowing to group filenames instead of treating them individually.
Then you could navigate (arrow keys, or grid view) between groups of files (if all groups are one file each then the behavior would be the same as the current one).

Now how to provide options to group files is another question... I'd argue there's a whole lot of cases between viewing "each files individually" and "all files together"

Consider the following list of files passed to F3D:

  1. ~/3dp/first-simple-object.stl
  2. ~/3dp/second-simple-object.stl
  3. ~/3dp/first-complex-object_part1of3.stl
  4. ~/3dp/first-complex-object_part2of3.stl
  5. ~/3dp/first-complex-object_part3of3.stl
  6. ~/3dp/second-complex-object_part1of2.stl
  7. ~/3dp/second-complex-object_part2of2.stl
  8. ~/3d-models/bunny.obj
  9. ~/3d-models/dragon.ply

Could we somehow come up with (preferably flexible and user-configurable) mechanism that would let us batch the files as:

  1. ~/3dp/first-simple-object.stl
  2. ~/3dp/second-simple-object.stl
  3. ~/3dp/first-complex-object_part1of3.stl, ~/3dp/first-complex-object_part2of3.stl, ~/3dp/first-complex-object_part3of3.stl
  4. ~/3dp/second-complex-object_part1of2.stl, ~/3dp/second-complex-object_part2of2.stl
  5. ~/3d-models/bunny.obj
  6. ~/3d-models/dragon.ply

Maybe a regular expression, or a simpler glob pattern?

@Meakk
Copy link
Member

Meakk commented Feb 2, 2023

From libf3d perspective, instead of loadGeometry and loadScene, I would prefer:

loader::addGeometry(std::string file)
loader::resetScene()
loader::loadFullScene(std::string file)

On the application side, I like @snoyer approach of a glob pattern, maybe something like:
Calling f3d simple*.stl "complex_*.stl" would work because the first argument is expanded by the shell, and the second one (with quotes) is a glob pattern.
Also, I agree that the folder view should be an alternate view, but unrelated to the multi-file stuff.

@mwestphal
Copy link
Contributor Author

From libf3d perspective, instead of loadGeometry and loadScene, I would prefer:

Indeed, sounds good.

the folder view should be an alternate view, but unrelated to the multi-file stuff.

Fair enough

Calling f3d simple*.stl "complex_*.stl" would work because the first argument is expanded by the shell, and the second one (with quotes) is a glob pattern.

This may work but may also be confusing to users. Does it work in all shells ?

@Meakk
Copy link
Member

Meakk commented Feb 2, 2023

This may work but may also be confusing to users. Does it work in all shells ?

I don't think we're responsible for user knowledge on shells. We need to provide a solution that makes sense, and we can document it. This is an advanced feature anyway for shell power users.
I think it should work on all POSIX shells (I have no idea about Windows).

@snoyer
Copy link
Contributor

snoyer commented Feb 2, 2023

To rephrase/elaborate: I'd like to have a grouping function g(str) -> str that would give colliding outputs for input filenames that should be grouped together. The usage could then look something like:

map<str, list<str>> groups
for(filename : filenames)
    groups[g(filename)].add(filename)

It would then be irrelevant where the filenames came from (shell expansion or not, drag-and-dropped into the window, ...), adding files incrementally would also be supported.

In my previous example just replacing _part* by an empty string does the trick and such cases would be easy to hardcode. The difficult part is to find a configurable way to do it. I don't think it can be done with pattern matching only, but a find-and-replace would work I believe.

@snoyer
Copy link
Contributor

snoyer commented Feb 2, 2023

dirty python POC using regex capture groups:

from collections import defaultdict
import re

filenames = [
    '~/3dp/first-simple-object.stl',
    '~/3dp/second-simple-object.stl',
    '~/3dp/first-complex-object_part1of3.stl',
    '~/3dp/first-complex-object_part2of3.stl',
    '~/3dp/first-complex-object_part3of3.stl',
    '~/3dp/first-complex-object_part1of3.step',
    '~/3dp/first-complex-object_part2of3.step',
    '~/3dp/first-complex-object_part3of3.step',
    '~/3dp/second-complex-object_part1of2.stl',
    '~/3dp/second-complex-object_part2of2.stl',
    '~/3dp/third-complex-object.1.stl',
    '~/3dp/third-complex-object.2.stl',
    '~/3d-models/bunny.obj',
    '~/3d-models/dragon.ply',
]

patterns = {
    'no grouping': r'(.*)',
    'by "_part*.ext"': r'(.+)_part.+(\..+)',
    'by ".*.ext"': r'(.+?)(?:\..+)*(\..+)',
    'by "_part*.ext" and ".*.ext"': r'(.+)_part.+(\..+)|(.+?)(?:\..+)*(\..+)',
    'by folder': r'(.+/)*.+',
}

for desc, pattern in patterns.items():
    def g(fn):
        match = re.match(pattern, fn)
        return match.groups() if match else fn

    groups = defaultdict(list)
    for filename in filenames:
        groups[g(filename)].append(filename)

    print(f'### {desc} (with pattern `{pattern}`)')
    for i,vs in enumerate(groups.values()):
        print(f'{i+1}.', ", ".join(f'`{v}`' for v in vs))
    print()

no grouping (with pattern (.*))

  1. ~/3dp/first-simple-object.stl
  2. ~/3dp/second-simple-object.stl
  3. ~/3dp/first-complex-object_part1of3.stl
  4. ~/3dp/first-complex-object_part2of3.stl
  5. ~/3dp/first-complex-object_part3of3.stl
  6. ~/3dp/first-complex-object_part1of3.step
  7. ~/3dp/first-complex-object_part2of3.step
  8. ~/3dp/first-complex-object_part3of3.step
  9. ~/3dp/second-complex-object_part1of2.stl
  10. ~/3dp/second-complex-object_part2of2.stl
  11. ~/3dp/third-complex-object.1.stl
  12. ~/3dp/third-complex-object.2.stl
  13. ~/3d-models/bunny.obj
  14. ~/3d-models/dragon.ply

by "_part*.ext" (with pattern (.+)_part.+(\..+))

  1. ~/3dp/first-simple-object.stl
  2. ~/3dp/second-simple-object.stl
  3. ~/3dp/first-complex-object_part1of3.stl, ~/3dp/first-complex-object_part2of3.stl, ~/3dp/first-complex-object_part3of3.stl
  4. ~/3dp/first-complex-object_part1of3.step, ~/3dp/first-complex-object_part2of3.step, ~/3dp/first-complex-object_part3of3.step
  5. ~/3dp/second-complex-object_part1of2.stl, ~/3dp/second-complex-object_part2of2.stl
  6. ~/3dp/third-complex-object.1.stl
  7. ~/3dp/third-complex-object.2.stl
  8. ~/3d-models/bunny.obj
  9. ~/3d-models/dragon.ply

by ".*.ext" (with pattern (.+?)(?:\..+)*(\..+))

  1. ~/3dp/first-simple-object.stl
  2. ~/3dp/second-simple-object.stl
  3. ~/3dp/first-complex-object_part1of3.stl
  4. ~/3dp/first-complex-object_part2of3.stl
  5. ~/3dp/first-complex-object_part3of3.stl
  6. ~/3dp/first-complex-object_part1of3.step
  7. ~/3dp/first-complex-object_part2of3.step
  8. ~/3dp/first-complex-object_part3of3.step
  9. ~/3dp/second-complex-object_part1of2.stl
  10. ~/3dp/second-complex-object_part2of2.stl
  11. ~/3dp/third-complex-object.1.stl, ~/3dp/third-complex-object.2.stl
  12. ~/3d-models/bunny.obj
  13. ~/3d-models/dragon.ply

by "_part*.ext" and ".*.ext" (with pattern (.+)_part.+(\..+)|(.+?)(?:\..+)*(\..+))

  1. ~/3dp/first-simple-object.stl
  2. ~/3dp/second-simple-object.stl
  3. ~/3dp/first-complex-object_part1of3.stl, ~/3dp/first-complex-object_part2of3.stl, ~/3dp/first-complex-object_part3of3.stl
  4. ~/3dp/first-complex-object_part1of3.step, ~/3dp/first-complex-object_part2of3.step, ~/3dp/first-complex-object_part3of3.step
  5. ~/3dp/second-complex-object_part1of2.stl, ~/3dp/second-complex-object_part2of2.stl
  6. ~/3dp/third-complex-object.1.stl, ~/3dp/third-complex-object.2.stl
  7. ~/3d-models/bunny.obj
  8. ~/3d-models/dragon.ply

by folder (with pattern (.+/)*.+)

  1. ~/3dp/first-simple-object.stl, ~/3dp/second-simple-object.stl, ~/3dp/first-complex-object_part1of3.stl, ~/3dp/first-complex-object_part2of3.stl, ~/3dp/first-complex-object_part3of3.stl, ~/3dp/first-complex-object_part1of3.step, ~/3dp/first-complex-object_part2of3.step, ~/3dp/first-complex-object_part3of3.step, ~/3dp/second-complex-object_part1of2.stl, ~/3dp/second-complex-object_part2of2.stl, ~/3dp/third-complex-object.1.stl, ~/3dp/third-complex-object.2.stl
  2. ~/3d-models/bunny.obj, ~/3d-models/dragon.ply

@mwestphal
Copy link
Contributor Author

@snoyer How do you envision that in terms of command lines options ?

@snoyer
Copy link
Contributor

snoyer commented Feb 2, 2023

From command line you could do something like f3d *.stl --group-by='(.+)_part.+(\..+)'

But realistically you'd want the --group-by argument in a config file according to your use-cases (I'm sure everyone would have different naming conventions) because it's not really convenient to type and too easy to get wrong, and command-line isn't the only entry point for F3D (eg. right-click -> open with F3D, drag and drop, ...).

Opening each file individually should still be the default, opening all files at once should have a simple argument like --multi, and then if there are widespread/standard advanced grouping patterns they could be aliased so you could type something like f3d */*.stl --by-folder instead of f3d */*.stl --group-by='(.+/)*.+'

I'm not fully clear on all the different use cases so I'm not claiming to have the perfect solution here, but regardless of what the user-facing system ends up being I believe the backend should be able to handle groups of one or more files where it handles individual files currently

@mwestphal
Copy link
Contributor Author

Great, I'll think about it.

@mwestphal mwestphal added this to the 2.0.0 milestone Feb 2, 2023
@mwestphal mwestphal self-assigned this Feb 2, 2023
@mwestphal
Copy link
Contributor Author

First step here: #634

@mwestphal
Copy link
Contributor Author

With @Meakk we discussed the future of F3D in regards to loading multiple files, full scene and default scene, which is loosely related to this issue. We will open an issue to track these future changes.

In the meantime, I may not implement everything that was discussed here but keep it simple.

@mwestphal
Copy link
Contributor Author

I've added a simple --group-geometries option for now.

@mwestphal mwestphal mentioned this issue Mar 16, 2023
8 tasks
mwestphal added a commit that referenced this issue Mar 30, 2023
Fix: #597
Fix: #417

New Loader API looks like this:

```
  /** 
   * Load a geometry from a provided file to the scene.
   * Reset the scene before loading if a full scene was loaded previously or if reset is set to false,
   * do not reset if only loaded geometries previously.
   * Geometries loader using this method will be available in a default scene and use all default
   * scene related options.
   * Throw a load_failure_exception on failure.
   */
  virtual loader& loadGeometry(const std::string& filePath, bool reset = false) = 0;

  /** 
   * Reset scene and load a full scene from provided file.
   * Please note default scene related options are not taken into account when loading a full scene.
   * Throw a load_failure_exception on failure.
   */
  virtual loader& loadScene(const std::string& filePath) = 0;

  /** 
   * Return true if the loader has a scene reader for the providen file, false otherwise.
   */
  virtual bool hasSceneReader(const std::string& filePath) = 0;

  /** 
   * Return true if the loader has a geometry reader for the providen file, false otherwise.
   */
  virtual bool hasGeometryReader(const std::string& filePath) = 0;
```

A new  command line option has been added to F3D:

`"group-geometries", "", "When opening multiple files, show them all in the same scene. Force geometry-only. The configuration file for the first file will be loaded."`
@mwestphal
Copy link
Contributor Author

mwestphal commented Mar 30, 2023

@snoyer @GitCodeBoy Let me know what you think.

mwestphal added a commit that referenced this issue Feb 10, 2024
Fix: #597
Fix: #417

New Loader API looks like this:

```
  /** 
   * Load a geometry from a provided file to the scene.
   * Reset the scene before loading if a full scene was loaded previously or if reset is set to false,
   * do not reset if only loaded geometries previously.
   * Geometries loader using this method will be available in a default scene and use all default
   * scene related options.
   * Throw a load_failure_exception on failure.
   */
  virtual loader& loadGeometry(const std::string& filePath, bool reset = false) = 0;

  /** 
   * Reset scene and load a full scene from provided file.
   * Please note default scene related options are not taken into account when loading a full scene.
   * Throw a load_failure_exception on failure.
   */
  virtual loader& loadScene(const std::string& filePath) = 0;

  /** 
   * Return true if the loader has a scene reader for the providen file, false otherwise.
   */
  virtual bool hasSceneReader(const std::string& filePath) = 0;

  /** 
   * Return true if the loader has a geometry reader for the providen file, false otherwise.
   */
  virtual bool hasGeometryReader(const std::string& filePath) = 0;
```

A new  command line option has been added to F3D:

`"group-geometries", "", "When opening multiple files, show them all in the same scene. Force geometry-only. The configuration file for the first file will be loaded."`
mwestphalnew pushed a commit to mwestphalnew/f3d that referenced this issue Feb 10, 2024
Fix: f3d-app#597
Fix: f3d-app#417

New Loader API looks like this:

```
  /** 
   * Load a geometry from a provided file to the scene.
   * Reset the scene before loading if a full scene was loaded previously or if reset is set to false,
   * do not reset if only loaded geometries previously.
   * Geometries loader using this method will be available in a default scene and use all default
   * scene related options.
   * Throw a load_failure_exception on failure.
   */
  virtual loader& loadGeometry(const std::string& filePath, bool reset = false) = 0;

  /** 
   * Reset scene and load a full scene from provided file.
   * Please note default scene related options are not taken into account when loading a full scene.
   * Throw a load_failure_exception on failure.
   */
  virtual loader& loadScene(const std::string& filePath) = 0;

  /** 
   * Return true if the loader has a scene reader for the providen file, false otherwise.
   */
  virtual bool hasSceneReader(const std::string& filePath) = 0;

  /** 
   * Return true if the loader has a geometry reader for the providen file, false otherwise.
   */
  virtual bool hasGeometryReader(const std::string& filePath) = 0;
```

A new  command line option has been added to F3D:

`"group-geometries", "", "When opening multiple files, show them all in the same scene. Force geometry-only. The configuration file for the first file will be loaded."`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
source:libf3d type:enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants