Repository navigation
Conversation
BigRoy
added
type: enhancement
c9c494b to
586808e
Compare
8984ead to
d512079
Compare
586808e to
60ff137
Compare
d512079 to
95f5631
Compare
| if not content.is_valid: | ||
| return "" | ||
| is_jpeg = content.content_type and "jpeg" in content.content_type | ||
| with tempfile.NamedTemporaryFile( |
There was a problem hiding this comment.
Shouldn't the thumbnail be stored using ImageCache logic? This just add another way how the thumbnail are downloaded, to a different place using different logic.
There was a problem hiding this comment.
That function is gone (bf5310a), there is no separate download code for the feed anymore. The widget asks its controller, and each tool answers with the thumbnail loading it already has:
- Browser: the existing
_thumbnail_loaderof_browser_thumbnails, so the sameImageCacheentries as the table, the cards and the inspector (ab8618d). - Workfiles:
ThumbnailsModelfromcommon_models, the one the Loader controller uses, with a newget_thumbnail_path()for a known thumbnail id (62fad62, 03dc40c).
So no new way of downloading, but the two tools still store thumbnails in two different caches, as they did before this PR (UI ImageCache in the Browser, the pipeline thumbnails cache in the Loader and now Workfiles). In the Browser the method also lives on the Qt-side BrowserWidgetController, because its thumbnail loading is frontend code today. If you would rather have one cache behind a backend model for both, I would do that as a follow up, it is listed in the to-do of the description.
| controller.register_event_callback( | ||
| "selection.folder.changed", | ||
| self._on_folder_selection_change, | ||
| ) | ||
| controller.register_event_callback( | ||
| "selection.task.changed", | ||
| self._on_task_selection_change, | ||
| ) |
There was a problem hiding this comment.
These should not be needed. The context is changed with "selection.representation.changed" and "selection.workarea.changed", or no?
There was a problem hiding this comment.
They are needed for the activity, as far as I can tell from the selection model. Both of those events are only emitted when the selected file changes:
set_selected_workfile_path()returns early when the path is the same as before, so"selection.workarea.changed"is not emitted when you go from one task to another and no workfile is selected in either (pathNone->None).set_selected_representation_id()does the same for"selection.representation.changed".
The details of the side panel are about the selected file, so those two events are enough there. The activity is about the task (or the folder until a task is selected), and should update when the task changes even if the workarea is empty, which is the case where there is no file event. That is why it listens to "selection.folder.changed" and "selection.task.changed".
This is from reading models/selection.py, I have not confirmed it in the running tool yet.
175dc9f to
265a91b
Compare
265a91b to
d5ca2fd
Compare
The Browser inspector and the Workfiles side panel get a Details | Activity tab pair, so the activity of the selected versions or task is available without making the tools wider. The feed is only fetched while its tab is visible.
The widget creates a cache only when none is passed. The Browser inspector uses the cache of the table, so avatars are not downloaded and rendered twice.
'get_activity_items' returns backend items with 'to_data' and 'from_data' instead of the UI models, so activities can be passed from a controller to the frontend. Entity ids are a list or a set, which can be converted to json. The feed of UI models is kept until the activity widget stops using it.
'get_thumbnail_paths' queries the entities to find their thumbnail ids. That is not needed when the thumbnail id is known, e.g. for versions of an activity feed.
The backend controller owns the activities model and exposes the activity and user items. The widget controller forwards them with the project statuses, and gets version thumbnails the way the table and the inspector do, from separate project name, version id and thumbnail id.
The controller owns the activities model and exposes the activity and user items to the frontend. Version thumbnails are loaded by the thumbnails model, as in the Loader. 'get_project_status_items' was implemented already, it is now part of the frontend abstraction.
The widget takes the controller of the tool as a required argument and does not create a model. 'ActivityController' describes the methods it needs, so other tools can implement them. Activity, status and user items of the controller are converted to the models of the activity stream in the frontend. Thumbnails of published versions are requested from the controller with their project name, version id and thumbnail id.
The model returned UI models, converted with a function of a module that imports Qt, and cached project users and statuses on its own. The activity widget gets those from the controller now, so the model only fetches activity items and does not import the frontend. Version thumbnails were downloaded by a function next to the model, they are loaded by the controllers too.
The model does not import the frontend anymore, so it can be imported from the package like the other common models.
The rows of the stream are reused on refresh, which needs the previous feed to stay until the new one arrives. The loading message is shown only for a different context.
Avatars are stored in a directory shared by all AYON processes of the user, like thumbnails, so an avatar is downloaded once per machine. A cached avatar is refreshed after a day and users without an avatar are remembered. The users model exposes it for the controllers.
The Browser downloaded thumbnails and avatars in its frontend and the activity feed added its own paths. All of them now ask the controller: thumbnails come from the shared thumbnails model and its cache on local storage, avatars from the avatars cache. The avatar cache of the UI accepts a loader for that. The Browser table takes user names from the users model as well, so project users are queried once for the table and the activity feed.
10741fa to
9a0ea6a
Compare
| @@ -0,0 +1,162 @@ | |||
| """Cache of user avatars on local storage. | |||
There was a problem hiding this comment.
I don't think this has "pipeline" usage. Probably makes sense only for tools ayon_core/tools/utils/avatars.py?
There was a problem hiding this comment.
Agreed, and looking at it again it should not exist at all. The file is removed in b575b91, together with get_user_avatar_path on the controllers and UsersModel, and the avatar_loader argument of UserAvatarCache.
With #2189 merged, develop already has UserAvatarCache downloading into the ImageCache, used by the views of the Browser, Workfiles and Launcher. This was a second way to download and store the same images, so the activity feed now uses UserAvatarCache like those views do. The Browser inspector still shares the cache instance of the table.
That does mean avatars are downloaded by the frontend and do not go through the controller, the same as in Workfiles and Launcher on develop. It is listed under "Still open" in the description.
|
In browser we do show activities when selecting product group (only for product groups). The header label is then |
…/browser-workfiles-activity-tab
…/browser-workfiles-activity-tab
Avatars are downloaded by 'UserAvatarCache' into the image cache, the same as in the views of the Browser, Workfiles and Launcher tools. The avatars cache on local storage and 'get_user_avatar_path' of the controllers are removed, they were a second way to download and store the same images.
A selected group of a product shows the activity of its featured version, while the path in the header of the inspector is empty for it. A label above the feed has the product and version name, or the count of selected versions.
A product group row carries its featured version, so the inspector shows that version: thumbnail, details, representations and now also its activity. Only the path in the header is empty for it. Kept it, and added a label above the feed that tells what the activity is of (dd89252):
The path label in the header is not changed, it is still Checked in an offscreen render of the inspector and with a test, not in the running tool yet. |
Warning
WIP - prototype to get a sense of how viable this is in production. Not ready for review of the details, feedback on the approach is welcome.
Changelog Description
Add an Activity tab to the Browser inspector and to the Workfiles side panel, to give access to comments and other activity without cluttering the tools or making them wider.
Browser inspector, Details and Activity tab:
Workfiles side panel, Details and Activity tab at 280px, and Activity at the default 190px:
Rendered offscreen from the real widgets with sample data.
Additional info
Part of a stack:
develop, includes the avatars of UI: Show user avatars in the activity stream #2146 which is merged into itUserAvatarCachethat the views of the Browser, Workfiles and Launcher already use. It downloads into the image cache on disk, which is shared by all AYON tools and processes of the user, so an avatar is downloaded once per machine.ayon_api.get_activities(limit=...)crashes in releasedayon_apiversions, fixed in Fix 'get_activities' crashing when 'limit' is passed ayon-python-api#365. Thelimitargument is not used here so it works with theayon_apiof current launchers.New pieces:
AYTabBar: tab strip for side panels.ActivitiesModel(backend,tools/common_models): fetches the feed and returns serializableActivityItems (to_data()/from_data()). It has noayon_core.uior Qt import.ActivityWidget(controller): the stream with background loading. It only talks to the controller of the tool, through theActivityControllerinterface:get_activity_items,get_project_status_items,get_user_itemsandget_version_thumbnail_path. It converts the items to the UI models, and creates aUserAvatarCacheunless one is passed (the Browser inspector shares the cache of the table).Backend / frontend separation
The Browser and Workfiles tools keep the frontend behind a controller, so it can run over IPC at some point. Done in this PR:
ActivityWidgetrequests the feed through the controller of each tool, the model is owned by the backend controller.ActivitiesModelreturns its own serializable items, converted to the UI models in the frontend.Entity ids are
list[str] | set[str].Version thumbnails go through the controller with project name, version id and thumbnail id as arguments. The feed has no download code of its own: the Browser uses its existing thumbnail loader, Workfiles the shared
ThumbnailsModel.Project statuses and users come from the
ProjectsModel/UsersModelof the controllers.Everything of the activity in the Workfiles side panel goes through its controller.
Version thumbnails are downloaded by the backend for both tools, through the shared
ThumbnailsModelinto the thumbnails cache on local storage (ayon_core.pipeline.thumbnails). That cache is shared by all AYON tools and processes of the user, so a thumbnail is downloaded once per machine. The Browser no longer downloads thumbnails in its frontend, its table, cards and inspector ask the controller too.The Browser queries project users once: the table takes the names from the
UsersModelthat also serves the feed.Still open:
ImageCache, which its thumbnail widgets use to show an image by key without waiting. It is filled from the path the controller returns, not by a download.UserAvatarCachedownloads them in the frontend, the same as it does for the views of Workfiles and Launcher on develop. The avatars cache on local storage andget_user_avatar_pathof the controllers, which an earlier state of this PR had, are removed to not have a second way to download and store the same images.UserAvatarCache(the Browser inspector shares the one of the table). Only the rendered pixmaps are per instance, the downloaded images are shared.ProjectsModelare emitted from that thread. In Workfiles that happens on every load of the feed.Functionality:
Testing notes:
./tools/manage.sh run pytest tests/client/ayon_core/tools -v