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

Refactoring ideas #44

Closed
10 tasks
mwestphal opened this issue Nov 12, 2021 · 7 comments
Closed
10 tasks

Refactoring ideas #44

mwestphal opened this issue Nov 12, 2021 · 7 comments
Labels
type:enhancement New feature or request
Milestone

Comments

@mwestphal
Copy link
Contributor

This thread is meant to discuss code improvements. It can be edited by everyone.

Use sub-folders

Architecture may be improve by splitting files in folders:

  • Core
  • Data
  • Filters
  • Rendering
  • Interactors
  • Options
  • ...
    please, edit this list, I am not the best fit to fill it.

Use namespaces

  • create new namespaces for
    • f3d
    • exceptions f3d::except?
    • subfolders (see thread: f3d::filter, f3d::data, ... may be too verbose.)
  • change the clang-format to avoid re-indenting everything (see NamespaceIndentation)

Add CMake Preset for build / testing

  • build
    • minimal (the lightest version)
    • ray-tracing
    • testing

Class splitting/reorganization

...

Misc

  • enforce C++ 11 in clang-format and CMake
    • switch to C++ 14 ?
  • use #pragma once
@mwestphal
Copy link
Contributor Author

@mwestphal mwestphal added this to the 1.3.0 milestone Nov 12, 2021
@mwestphal mwestphal added the type:enhancement New feature or request label Nov 12, 2021
@mwestphal
Copy link
Contributor Author

mwestphal commented Dec 17, 2021

Looking at this again.

Here is my suggestion:

cmake
  .cmake files
testing
  baselines
    lfs .png baseline files
  data
    lfs testing data files
  recordings
    .log files
documentation
  unchanged
resources
  resources file
libf3d
  core
      F3DAnimationManager
      F3DException
      F3DLoader (Split in two with a F3DInit/F3DStarter)
      F3DLog
      F3DNSDelegate
      F3DOffscreenRender
      F3DOptions
      Config -> Should be renammed F3DConfig
      F3DIncludes -> To be merged with F3DConfig ?
   readers
      unchanged
   specific
      F3DNSDelegate
   third_party
      cxxopts.hpp
   VTKExtensions
    IO
      vtkF3DGenericImporter
      vtkF3DMetaReader
      vtkF3DOCCTReader
      vtkF3DAssimpImporter
    Interaction
      vtkF3DInteractorEventRecorder
      vtkF3DInteractorStyle
    Rendering
      vtkF3DOpenGLGridMapper
      vtkF3DPolyDataMapper
      vtkF3DRenderPass
      vtkF3DRenderer
      vtkF3DRendererWithColoring
    Misc
      vtkF3DConsoleOutputWindow
      vtkF3DObjectFactory
      vtkF3DPostProcessFilter
      vtkF3DWin32OutputWindow
exec
  embed.cxx
  main.cxx
winshellext
  unchanged

@Meakk @CharlesGueunet @Meakk

@mwestphal
Copy link
Contributor Author

@CharlesGueunet @Meakk : Regarding this namespace discussion: https://gitlab.kitware.com/f3d/f3d/-/issues/227#note_942471

I still am not sure of what you mean and how to set it up, I'll let you take the lead on this.

@mwestphal
Copy link
Contributor Author

@mwestphal
Copy link
Contributor Author

Regarding the F3DLoader/F3DStarter separation, it try to separate the starting from the file loading, the resulting signature for the constructor of the F3DLoader would looke like this:

F3DLoader(const F3DOption& CommandLineOptions, F3DOptionsParser& parser, F3DOptions& options, F3DAnimationManager& animationManager, vtkF3DRenderer* renderer, vtkRenderWindow* renWin);

It can be done be not that beneficial

@mwestphal
Copy link
Contributor Author

This will need to wait #173 and #52

@mwestphal
Copy link
Contributor Author

This has largely been done in #203 #273 #278 #255 #285 #293

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant