This an is adjunct to the task subsystem discussions (Design Issues, Progress) specifically directed at storing the nodal UI geometry offline. The initial thinking is outlined below.
Strawman
The graphical node layout is a display feature of the project and task system and is not part of their respective states. For example, adjusting the position of a node in the scene does not change a project from the clean to modified condition.
To store the geometry, the basic plan is to write a taskscene.json file to the project directory with the graphics info for each node:
position coordinates (XY)
content style (minimal, summary, details)
outline style (normal, active)
The taskflow.json file is written out when a project is closed and read in when a project is opened.
Strategy
Detecting when a project is closing is not always straightforward.
For historical reasons, the code for closing a project is klugey. Plugins like ace3p-extensions have a “pqCloseProjectBehavior” class to close the project by traversing its constituent resources and removing them from the resource manager, and then removing the project resource from both the resource manager and project manager.
My thinking is that we should move this logic to a new smtk::project::Close operation.
With that change, then qtTaskScene instances can attach an observer to write out the taskscene.json file before the operation runs.
Potential Issue with task configuration ids visibility and persistence
It does not appear that the integer ids used in the task configuration file are accessible to the rest of the application. (I am also not even sure they are required for nodes that aren’t a dependency for other nodes, but I will presume they are or that we can internally generate them if needed.) We need some way to make the task Configuration instance (nlohmann::json object) accessible to qtTaskScene or at least the “id” field. In lieu of anything clever, this could be as simple as adding a public method int configId() const. The doc string should include a warning that it is intended for internal use.
It will be far easier to save the node locations as part of each task’s JSON rather than creating a separate JSON file to hold the layout. The issue is that tasks do not have persistent, unique identifiers that can be written to disk so there is not an easy way to specify a mapping to their nodal-UI location.
It should be simple to detect when a project is about to close (the project manager signals observers), but that is irrelevant if the nodal locations are saved with the project as part of the smtk::project::Write operation (which has access to all application state as well as the project’s task-manager).
For resources, the pattern we have adopted is to have a single close operation that does nothing but pass the resource(s) to be closed into a specially-named result variable. The base Operation class (outside the subclass’s operateInternal() but while locks are still held) removes the resources from the resource manager. Similarly, we should provide for projects in that list to be removed from the project manager. This avoids issues with user interface elements having their observers invoked on non-GUI threads.
A nice way to separate concerns while interleaving the nodal-UI (x,y) locations in the output JSON is to provide the smtk::task::Manager with a viewInstances() method (or the smtk::view::Manager with a taskViewInstances() method). Then smtk/task/json/jsonTask.cxx can iterate over views and ask each one for the “location” of its task in the view.
Usng smtk::project::Write to save the graphics layout would seem to make the graphics layout a first-class part of the project. So that if the user repositions a node in the UI, modelbuilder would be marked as modified. Is that your position? I’m not a real fan of that, but yes, it would make the code alot easier.
1. From a UX perspective, I think it’s suboptimal for (i) users to remember to save projects after making cosmetic changes, and for (ii) applications to provide a way to save projects that aren’t in the modified state. Would you consider adding a “modified” flag to qtTaskView or qtTaskScene which could be used on project close to automatically save the project if needed?
2. But my bigger issue is the tight coupling of task system data and its display properties. I feel that breaking the design standard of separating data from presentation deserves more justification than “it’s easier”.
3. Related to that, sort of: when we implement other task UI designs, I presume we would embed their properties in the taskManager part of the project file also. For example, I’ll want a tree-style task UI for simpler projects. So would the json representation for task manager include optional sections for nodal_ui_properties and treeview_ui_properties? On second thought, I guess there would be a single ui_properties section and each UI code would have their own format?
4. @Bob_Obara we probably need you to weigh in and perhaps mediate/negotiate.
I would prefer to make changes to node position modify the project. It appears to be something you really want preserved; you should be nagged to save the project if you start fiddling. You can always choose to discard it.
I strongly dislike writing files without the user being aware, especially if that file contains other state the user cares about. Something always goes wrong eventually and I don’t want to be the one who destroyed someone’s work without even asking whether it was OK.
Just because the information is in the same file does not mean the same piece of code has to handle everything (though I would do it this way until there was a need for something different).
On a related/side note, I would advise adding a modified flag to smtk::task::Manager to keep track of its state separate from the rest of the project.
And in the interest of moving on…
What API would you envision to insert the UI data into Task objects? I would presume passing a json object, either the full “ui” object or the object to insert inside the “ui” object. I would recommend the latter:
I don’t think the task class needs to know anything about that. Only the json/jsonTask.cxx file needs to know to deserialize that portion of the JSON at the same time it is processing the task (so it has a Task* as well as the UI JSON).
When a task’s json object is deserialized by the from_json() method in json/jsonTask.cxx, how would its “ui” part get to the qtTaskNode (and/or qtTaskScene?) object without going through the Task instance?