Better source tree

[Hombre57]

RT's source tree for the code part is quite flat actually : only 3 folders (rtengine, rtexif - that might be dropped, and rtgui).

Would it be useful for newcomers to have a more self-explanatory source tree ? I don't think we can put each tool's GUI and engine in their own directory, because the engine part is mostly made os shared code (between tools)

I'm thinking about :

- [rtgui]
   - [custom-widgets]
      - adjuster.cc
      - adjuster.h
      - checkbox.cc
      - checkbox.h
      - coloredbar.cc
      - coloredbar.h
      - coordinateadjuster.cc
      - coordinateadjuster.h
      - curves
      - inspector.cc
      - inspector.h
      - lockablecolorpicker.cc
      - lockablecolorpicker.h
      - lwbutton.cc
      - lwbutton.h
      - lwbuttonset.cc
      - lwbuttonset.h
      - on-preview-editing
      - popupbutton.cc
      - popupbutton.h
      - popupcommon.cc
      - popupcommon.h
      - popuptogglebutton.cc
      - popuptogglebutton.h
      - previewwindow.cc
      - previewwindow.h
      - profilestorecombobox.cc
      - profilestorecombobox.h
      - rtimage.cc
      - rtimage.h
      - shcselector.cc
      - shcselector.h
      - splash.cc
      - splash.h
      - thresholdadjuster.cc
      - thresholdadjuster.h
      - thresholdselector.cc
      - thresholdselector.h

      - [curves]
         - curveeditor.cc
         - curveeditor.h
         - curveeditorgroup.cc
         - curveeditorgroup.h
         - curvelistener.h
         - diagonalcurveeditorsubgroup.cc
         - diagonalcurveeditorsubgroup.h
         - flatcurveeditorsubgroup.cc
         - flatcurveeditorsubgroup.h
         - mycurve.cc
         - mycurve.h
         - mydiagonalcurve.cc
         - mydiagonalcurve.h
         - myflatcurve.cc
         - myflatcurve.h

      - [on-preview-editing]
         - edit.cc
         - edit.h
         - editid.h

   - [tools]
      - bayerpreprocess.cc
      - bayerpreprocess.h
      - bayerprocess.cc
      - bayerprocess.h
      - bayerrawexposure.cc
      - bayerrawexposure.h
      - blackwhite.cc
      - blackwhite.h
      - chmixer.cc
      - chmixer.h
      - coarsepanel.cc
      - coarsepanel.h
      - colorappearance.cc
      - colorappearance.h
      - colortoning.cc
      - colortoning.h
      - crop.cc
      - crop.h
      - darkframe.cc
      - darkframe.h
      - defringe.cc
      - defringe.h
      - dirpyrdenoise.cc
      - dirpyrdenoise.h
      - dirpyrequalizer.cc
      - dirpyrequalizer.h
      - distortion.cc
      - distortion.h
      - epd.cc
      - epd.h
      - exifpanel.cc
      - exifpanel.h
      - fattaltonemap.cc
      - fattaltonemap.h
      - filmsimulation.cc
      - filmsimulation.h
      - flatfield.cc
      - flatfield.h
      - gradient.cc
      - gradient.h
      - hsvequalizer.cc
      - hsvequalizer.h
      - icmpanel.cc
      - icmpanel.h
      - impulsedenoise.cc
      - impulsedenoise.h
      - iptcpanel.cc
      - iptcpanel.h
      - labcurve.cc
      - labcurve.h
      - lensgeom.cc
      - lensgeom.h
      - lensgeomlistener.h
      - lensprofile.cc
      - lensprofile.h
      - pcvignette.cc
      - pcvignette.h
      - perspective.cc
      - perspective.h
      - preprocess.cc
      - preprocess.h
      - prsharpening.cc
      - prsharpening.h
      - rawcacorrection.cc
      - rawcacorrection.h
      - rawexposure.cc
      - rawexposure.h
      - resize.cc
      - resize.h
      - retinex.cc
      - retinex.h
      - rgbcurves.cc
      - rgbcurves.h
      - rotate.cc
      - rotate.h
      - sensorbayer.cc
      - sensorbayer.h
      - sensorxtrans.cc
      - sensorxtrans.h
      - shadowshighlights.cc
      - shadowshighlights.h
      - sharpenedge.cc
      - sharpenedge.h
      - sharpening.cc
      - sharpening.h
      - sharpenmicro.cc
      - sharpenmicro.h
      - tonecurve.cc
      - tonecurve.h
      - vibrance.cc
      - vibrance.h
      - vignetting.cc
      - vignetting.h
      - wavelet.cc
      - wavelet.h
      - wbprovider.h
      - whitebalance.cc
      - whitebalance.h
      - xtransprocess.cc
      - xtransprocess.h
      - xtransrawexposure.cc
      - xtransrawexposure.h

   - [window]
      - dynamicprofilepanel.cc
      - dynamicprofilepanel.h
      - editwindow.cc
      - editwindow.h
      - partialpastedlg.cc
      - partialpastedlg.h
      - preferences.cc
      - preferences.h
      - renamedlg.cc
      - renamedlg.h
      - rtwindow.cc
      - rtwindow.h
      - saveasdlg.cc
      - saveasdlg.h

   - addsetids.h
   - batchqueue.cc
   - batchqueue.h
   - batchqueuebuttonset.cc
   - batchqueuebuttonset.h
   - batchqueueentry.cc
   - batchqueueentry.h
   - batchqueuepanel.cc
   - batchqueuepanel.h
   - batchtoolpanelcoord.cc
   - batchtoolpanelcoord.h
   - bqentryupdater.cc
   - bqentryupdater.h
   - browserfilter.cc
   - browserfilter.h
   - cacheimagedata.cc
   - cacheimagedata.h
   - cachemanager.cc
   - cachemanager.h
   - cacorrection.cc
   - cacorrection.h
   - clipboard.cc
   - clipboard.h
   - CMakeLists.txt
   - colorprovider.h
   - config.h.in
   - cropguilistener.h
   - crophandler.cc
   - crophandler.h
   - cropwindow.cc
   - cropwindow.h
   - cursormanager.cc
   - cursormanager.h
   - dirbrowser.cc
   - dirbrowser.h
   - editedstate.h
   - editenums.h
   - editorpanel.cc
   - editorpanel.h
   - exiffiltersettings.cc
   - exiffiltersettings.h
   - exportpanel.cc
   - exportpanel.h
   - extprog.cc
   - extprog.h
   - favoritbrowser.cc
   - favoritbrowser.h
   - filebrowser.cc
   - filebrowser.h
   - filebrowserentry.cc
   - filebrowserentry.h
   - filecatalog.cc
   - filecatalog.h
   - filepanel.cc
   - filepanel.h
   - fileselectionchangelistener.h
   - fileselectionlistener.h
   - filethumbnailbuttonset.cc
   - filethumbnailbuttonset.h
   - filterpanel.cc
   - filterpanel.h
   - guiutils.cc
   - guiutils.h
   - histogrampanel.cc
   - histogrampanel.h
   - history.cc
   - history.h
   - ilabel.cc
   - ilabel.h
   - imagearea.cc
   - imagearea.h
   - imageareapanel.cc
   - imageareapanel.h
   - imageareatoollistener.h
   - indclippedpanel.cc
   - indclippedpanel.h
   - main-cli.cc
   - main.cc
   - mountselectionlistener.h
   - multilangmgr.cc
   - multilangmgr.h
   - myicon.rc
   - navigator.cc
   - navigator.h
   - options.cc
   - options.h
   - paramsedited.cc
   - paramsedited.h
   - pathutils.cc
   - pathutils.h
   - placesbrowser.cc
   - placesbrowser.h
   - pointermotionlistener.h
   - pparamschangelistener.h
   - ppversion.h
   - previewhandler.cc
   - previewhandler.h
   - previewloader.cc
   - previewloader.h
   - previewmodepanel.cc
   - previewmodepanel.h
   - procparamchangers.h
   - profilechangelistener.h
   - profilepanel.cc
   - profilepanel.h
   - progressconnector.h
   - quickzoomlistener.h
   - recentbrowser.cc
   - recentbrowser.h
   - recentselectionlistener.h
   - saveformatpanel.cc
   - saveformatpanel.h
   - soundman.cc
   - soundman.h
   - threadutils.cc
   - threadutils.h
   - thumbbrowserbase.cc
   - thumbbrowserbase.h
   - thumbbrowserentrybase.cc
   - thumbbrowserentrybase.h
   - thumbimageupdater.cc
   - thumbimageupdater.h
   - thumbnail.cc
   - thumbnail.h
   - thumbnailbrowser.h
   - thumbnaillistener.h
   - toolbar.cc
   - toolbar.h
   - toolenum.h
   - toolpanel.cc
   - toolpanel.h
   - toolpanelcoord.cc
   - toolpanelcoord.h
   - version.h.in
   - windirmonitor.cc
   - windirmonitor.h
   - zoompanel.cc
   - zoompanel.h

...though I don't know if it's worth loosing the files' history for bisecting.

[Floessie]

That sounds like a very good idea. I have a recursive ImportSourcesFromSubdir CMake function lying around somewhere...

[Beep6581]

I like the idea.

though I don't know if it's worth loosing the files' history for bisecting.

What makes you think you'd lose history?

[Hombre57]

@Beep6581

What makes you think you'd lose history?

I may not know git enough, but moving a file means deleting it in its old location and creating it in its new location. But I may be wrong.

[Hombre57]

@Beep6581 I'm also worried about active branch and how to merge them.

[Beep6581]

@Hombre57 git will not lose history when moving files. Use git mv to be safe. https://git-scm.com/book/en/v2/Git-Internals-Moving-Files

[Beep6581]

And here's what Gordon Ramsay has to say about it: https://web.archive.org/web/20150209075907/http://permalink.gmane.org:80/gmane.comp.version-control.git/217

[Hombre57]

@Beep6581 I'll make test with git mv later this week or by the w.e., thanks for the hint.

[Beep6581]

@Hombre57 shoot if you need help with this.

[Beep6581]

Maybe now would be a good time to go ahead with this.

[Hombre57]

@Beep6581 I'm not at home until Sunday night, with limited access to my laptop, so if not too late, I'll try that on Monday or Tuesday.

[Hombre57]

@Beep6581 @Floessie I've made some test with the git mv command. Files has been moved, but all include statment related to the moved files has to be updated, and I don't know what will happen when merging this branch to dev, regarding the other opened branches. This is too much work for me. So unless you have a magical cmake command to add the the new subfolder to the include search path, I'd better close this issue or pass over.

[Beep6581]

First, do we have a consensus that the proposed structure in the first post is the way to go?

[Hombre57]

@Beep6581 I've tested to this new structure, but there's a lot of work to do for the include declaration, so I gave up. A better source tree would be nice, but I don't think it'll be ready for 5.5. I'd remove it from this milestone.

[Floessie]

@Hombre57 I've pushed a new branch with the structure from your first proposal. Feel free to shuffle other files as well.

I dislike the relative includes needed ATM. Shouldn't we make the source root path the only include path (apart from the current dir), and all relative paths absolute to that? Ideas welcome. Things like ../../../rtengine/something.h are just ugly, rtengine/something.h would be so much nicer.

Best, Flössie

[Hombre57]

@Floessie Wow, that was fast ! I'm testing that now.

I dislike the relative includes needed ATM. Shouldn't we make the source root path the only include path (apart from the current dir), and all relative paths absolute to that?

If possible, I'd set 2 include path (in that order) :

1. the source root path, as you suggest
2. the current source file's path

The idea is to be able to simply specify #include "myheader.h" to load the header of the current file or files from subdirectories, and e.g. #include "rtgui/window/editwindow.h" for files from parent directories.

To avoid error, source files shouldn't have the same name. Never.

If you find this too confusing, or error prone, I'd stick to your suggestion.

[Hombre57]

@Floessie There's some other files to shuffle, yes, that appears in between. I'll do that tomorrow, too late now.

[Floessie]

@Hombre57

The idea is to be able to simply specify #include "myheader.h" to load the header of the current file or files from subdirectories, and e.g. #include "rtgui/window/editwindow.h" for files from parent directories.

That's exactly what I was thinking about. :smile: Something like include_directories(${CMAKE_SOURCE_DIR}) should do, because the current dir is always searched first by the preprocessor/compiler AFAIK. The ../../../ lunacy could be replaced with the "full path" by a regex.

This is the CMake snippet I was talking about much earlier:

function(IMPORT_SOURCES_FROM_SUBDIR subdir sources_variable)
    set(result ${${sources_variable}})
    set(sources)
    include("${CMAKE_CURRENT_LIST_DIR}/${subdir}/SOURCES.cmake")
    foreach(src ${sources})
        list(APPEND result "${subdir}/${src}")
    endforeach()
    set(${sources_variable} ${result} PARENT_SCOPE)
endfunction()

A SOURCES.cmake in a subdir looks like this:

set(sources
    file_a.cpp
    file_b.cpp
)

Used like this:

include(ImportSourcesFromSubdir)

IMPORT_SOURCES_FROM_SUBDIR(dir_a all_sources)
IMPORT_SOURCES_FROM_SUBDIR(dir_b all_sources)

HTH, Flössie

[Hombre57]

@Floessie @heckflosse @Beep6581 @Thanatomanic Just tested today to locally merge dev into better-source-tree and the other way round, as expected git doesn't follow the file relocation. At least through Eclipse's git. So rearanging files without breaking other branches seem difficult (if not impossible). Should we leave things as is and close this issue ?

[Beep6581]

Git tracks content, not filenames.

We should clean up the tree, but let's wait till after 5.5.