forked from aurelienpierreeng/ansel
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
6d3535a
commit c24f352
Showing
6 changed files
with
161 additions
and
23 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <!-- HTML footer for doxygen 1.10.0--> | ||
| <!-- start footer part --> | ||
| <!--BEGIN GENERATE_TREEVIEW--> | ||
| <div id="nav-path" class="navpath"><!-- id is needed for treeview function! --> | ||
| <ul> | ||
| $navpath | ||
| <li class="footer">$generatedby <a href="https://www.doxygen.org/index.html"><img class="footer" src="$relpath^doxygen.svg" width="104" height="31" alt="doxygen"/></a> $doxygenversion </li> | ||
| </ul> | ||
| </div> | ||
| <!--END GENERATE_TREEVIEW--> | ||
| <!--BEGIN !GENERATE_TREEVIEW--> | ||
| <hr class="footer"/><address class="footer"><small> | ||
| $generatedby <a href="https://www.doxygen.org/index.html"><img class="footer" src="$relpath^doxygen.svg" width="104" height="31" alt="doxygen"/></a> $doxygenversion | ||
| </small></address> | ||
| <!--END !GENERATE_TREEVIEW--> | ||
| <script> | ||
| mermaid.initialize({ | ||
| startOnLoad: true, | ||
| theme: 'base', | ||
| }); | ||
| </script> | ||
| </body> | ||
| </html> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| # Ansel code base reorganization | ||
|
|
||
| ## The current problem | ||
|
|
||
| Ansel is not modular, as the [/src dependency graph](@ref src) shows: everything is wired to the GUI, "modules" are all aware of everything. Modifying anything __somewhere__ typically breaks something unexpected __somewhere else__. | ||
|
|
||
| ## Target design | ||
|
|
||
| _Note: because of the plotting library, this is a reversed dependency graph_. | ||
|
|
||
| <pre class="mermaid"> | ||
| flowchart TD | ||
| libs(libs) --> math(math) | ||
| libs --> string(string) | ||
| libs --> CPU(CPU) | ||
| libs --> path(path) | ||
| libs --> color(colorspaces) | ||
| app --> database | ||
| app --> GUI | ||
| app --> iop | ||
| GUI --> views | ||
| views --> darkroom | ||
| views --> lighttable | ||
| history --> node[pipeline nodes] | ||
| history --> database | ||
| history --> styles | ||
| node --> pipe[pixel pipeline] | ||
| iop[IOP modules] --> node | ||
| iop --> history | ||
| database --> lighttable | ||
| darkroom --> pipe | ||
| darkroom --> iop | ||
| </pre> | ||
|
|
||
| We should here make distinctions between: | ||
|
|
||
| - __libs__ : basic shared libraries, within the scope of the project, that do basic-enough reusable operation, unaware of app-wise datatypes and data (plotted with rounded corners). | ||
| - __modules__ (in the programming sense) : self-enclosed code units that cover high-level functionnality (plotted with square corners). | ||
|
|
||
|
|
||
| ### IOP modules | ||
|
|
||
| IOP (_Image OPerations_) modules are more like plugins: it's actually how they are referred to in many early files. They define both a _pipeline node_ (aka pixel filtering code) and a GUI widget in darkroom. The early design shows initial intent of making them re-orderable in the pipelpine, and to allow third-party plugins. As such, the core was designed to be unaware of IOP internals. | ||
|
|
||
| As that initial project seemed to be abandonned, IOP modules became less and less enclosed from the core, which allowed some lazy mixes and confusions between what belongs to the scope of the pipeline, and what belongs to the scope of modules. The pipeline output profile can therefore be retrieved from the _colorout_ module or from the pipeline data. _color calibration_ reads the input profile from _colorin_. | ||
|
|
||
| IOP modules also commit their parameter history directly using [`darktable.develop`](@ref darktable_t) global data, instead of using their private link [`(dt_iop_module_t *)->dev`](@ref dt_iop_module_t), which is documented in an old comment to be the only thread-safe way of doing it. | ||
|
|
||
| TODO: | ||
|
|
||
| - [ ] make IOP modules only aware of history and GUI object, | ||
| - [ ] handle uniform ways of communicating between modules on the pipeline | ||
|
|
||
| ### History | ||
|
|
||
| The editing history is the snapshot of parameters for each IOP. It gets saved to the database. It gets read and flattened to copy parameters to pipeline nodes. The problem of the history code, right now, is it was hacked with masks later, so it mixes SQL code, GUI code, pixelpipe code and… well, history code. | ||
|
|
||
| History should only deal with push/pop/merge operations and communicate with pipeline through a flattened associative list: 1 parameter state <-> 1 pipeline node. | ||
|
|
||
|
|
||
| ### Develop | ||
|
|
||
| The [development](@ref /src/develop/develop.h) is an hybrid thing joining history with pipeline. This object will take care of reading the image cache to grab the buffer, reading the database history, initing a pipeline, and starting a new computing job. | ||
|
|
||
| But again, it has been hacked to lazily handle in a centralized fashion many things that only belong to the darkroom, like the colorpicker proxies, overexposed/gamut alerts, ISO 12626 preview, and such. Those belong to GUI, because development objects are also used when exporting images or rendering thumbnails. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # Redesign of selection API | ||
|
|
||
| [In progress] | ||
|
|
||
| `src/common/collections.h` and `src/common/selection.h` have been properly defiled since 2019. The code is duplicated by the thumbtable (`image_to_act.h`) and such. The whole thing has more than one way of setting and getting params, which sucks. But it also mixes C code (using `GList`) and SQL requests in a way that is really messy. | ||
|
|
||
| ## Description of the new API | ||
|
|
||
| ### Collections | ||
|
|
||
| Declared in `src/common/collections.h`. It's the backbone of the lighttable and filmstrip, extracted from the library database. | ||
|
|
||
| ```C | ||
| typedef struct dt_collection_t | ||
| { | ||
| gchar *query, *query_no_group; | ||
| gchar **where_ext; | ||
| unsigned int count, count_no_group; | ||
| unsigned int tagid; | ||
| dt_collection_params_t params; | ||
| dt_collection_params_t store; | ||
|
|
||
| // Unpack the list of imgid to avoid querying SQL al the time | ||
| GList *id_list; | ||
| } dt_collection_t; | ||
| ``` | ||
|
|
||
| The whole collection API uses a `dt_collection_t *collection` argument, but the whole software really only uses one collection, stored in the `darktable.collection` structure member and accessed globally from there. It seems a sane practice to keep the API able to deal with another collection in the future, but it's slightly annoying to call all functions with always the same argument. | ||
|
|
||
| The `gchar *query` and `gchar *query_no_group` contain the SQL commands, built from the config strings stored in `anselrc`. From there, we only care about the list of image ids extracted from the library DB that will be linked with thumbnails and mipmap cache. | ||
|
|
||
| So, basically, the collection is a list of `imgid` output from an SQL extraction of the library, using filtering and sorting parameters defined by users in the `src/lib/collect.c` and `src/lib/tools/filters.c` GUI modules. | ||
|
|
||
| Updating the SQL queries is done by the `dt_collection_update_query()` function, that is called only from GUI modules `src/lib/collect.c` and `src/lib/tools/filters.c` and reads only the `anselrc` params, meaning the GUI controls should commit the settings to `anselrc` before calling dt_collection_update_query()`. | ||
|
|
||
| The collection might filter images based on user-editable properties, like rating, color labels and metadata. On user edition events, the content of the collection may change and needs to be reloaded with `dt_collection_update()`. This function is connected to various signals like `DT_SIGNAL_FILMROLLS_IMPORTED`, `DT_SIGNAL_IMAGE_IMPORT`, `DT_SIGNAL_TAG_CHANGED`. When a GUI widget performs property edition that may affect the content of the collection, it needs to raise the relevant signal (that you may create for your needs). The function should not be called directly. | ||
|
|
||
| `dt_collection_update()` is called from `dt_collection_update_query()` as well. When it finishes, it raises the signal `DT_SIGNAL_COLLECTION_CHANGED` that should be captured by GUI events that need to update the list of thumbnails, in lighttable and filmstrip. | ||
|
|
||
| Since a collection is ultimately just an ordered list of `imgid`, in `dt_collection_update()` we also create the `GList *id_list` as a member of the `struct dt_collection_t`. From there, no further SQL messup is required in the GUI code, except to grab further data linked to an image from the database (and deal with ugly image groups… See below) | ||
|
|
||
| __Collections are a MODEL in the view-model-controller scheme, they should only deal with information and contain no Gtk/GUI code__. | ||
|
|
||
| ## Groups | ||
|
|
||
| Groups allow to define "sibling" images that can be collapsed or expanded in GUI. They are referenced by a unique ID in database. When collapsing/expanding, the ID of the current group is remembered at the app level and stored into `darktable.gui->expanded_group_id`. To reset it, it is set to `-1`. | ||
|
|
||
| Groups are handled in a particularly ugly way, through non-uniform special cases, in many places in the code. The most probable reason is they were introduced as an afterthought, after the initial design of collections. They need a better API. | ||
|
|
||
| Creating groups and duplicates currently raises the signal `DT_SIGNAL_FILMFOLDER_CHANGED` because they need to be inserted in the table. However, that may not work for collections filtering tags or metadata. |