UI: Tooltips for Assets #106189

Merged
Harley Acheson merged 1 commits from Harley/blender:TooltipsSimple into main 2023-04-12 18:44:59 +02:00
Member

Show tooltips with minimal data for Asset Browser items.


We don't currently show any tooltips for Asset Browser items. This can be problematic because as the thumbnail size decreases you are more likely to have the name truncated.

This PR adds these tooltips with minimum code changes and with minimal information. This is meant to be an easy and uncontentious solution for 3.6 (mostly because of Assets) that I could (propose to) extend further for version 4.0.

Following shows a tooltip for an Asset, showing just name and description (if available). Note the utility of seeing the full name in this situation. We could also show license, copyright, and author but these items seemed less immediately necessary and can be found in the side panel if needed.

image

Show tooltips with minimal data for Asset Browser items. --- We don't currently show any tooltips for Asset Browser items. This can be problematic because as the thumbnail size decreases you are more likely to have the name truncated. This PR adds these tooltips with minimum code changes and with minimal information. This is meant to be an easy and uncontentious solution for **3.6** (mostly because of Assets) that I could (propose to) extend further for version 4.0. Following shows a tooltip for an Asset, showing just name and description (if available). Note the utility of seeing the full name in this situation. We could also show license, copyright, and author but these items seemed less immediately necessary and can be found in the side panel if needed. ![image](/attachments/c8e42cea-cf22-4353-8a71-c7bdf3178aa2)
Harley Acheson added this to the Module: User Interface project 2023-03-27 19:09:46 +02:00
Harley Acheson requested review from Julian Eisel 2023-03-27 19:09:57 +02:00
Harley Acheson force-pushed TooltipsSimple from f1e4dee2d0 to b5fbe189c5 2023-03-28 18:57:58 +02:00 Compare
First-time contributor

For the file system tooltip, should it only have the blend file name rather than having the file path? In a scenario where the path is long and also the path is already at the top.

For the file system tooltip, should it only have the blend file name rather than having the file path? In a scenario where the path is long and also the path is already at the top. > ![](https://projects.blender.org/attachments/a0810972-00cc-4748-a799-77ae802043d6)
Harley Acheson force-pushed TooltipsSimple from b5fbe189c5 to 56d33ddb9f 2023-04-06 01:54:45 +02:00 Compare
Author
Member

@shwaky - For the file system tooltip, should it only have the blend file name rather than having the file path? In a scenario where the path is long and also the path is already at the top.

Good point. There is a situation where you would still want the file path, and that is when you have recursion turned on.

So have update the PR to only show the path if recursion_level > 0. I have updated the capture in the first comment to show what it looks like in most situations.

> @shwaky - For the file system tooltip, should it only have the blend file name rather than having the file path? In a scenario where the path is long and also the path is already at the top. Good point. There is a situation where you would still want the file path, and that is when you have recursion turned on. So have update the PR to only show the path if recursion_level > 0. I have updated the capture in the first comment to show what it looks like in most situations.
Julian Eisel requested changes 2023-04-11 16:25:00 +02:00
Julian Eisel left a comment
Member

Let's treat asset and file tooltips as separate patches. For assets things are straight forward, for files further discussion may be needed (like which information to display there, formatting, etc). I'm also not convinced the tooltip is the best place for this information, we could also display file information in the sidebar or in some extra region (like some other file browsers do).

Let's treat asset and file tooltips as separate patches. For assets things are straight forward, for files further discussion may be needed (like which information to display there, formatting, etc). I'm also not convinced the tooltip is the best place for this information, we could also display file information in the sidebar or in some extra region (like some other file browsers do).
@ -112,0 +139,4 @@
char description[2048];
size_t slen = 0;
if (file->asset) {
Member

Let's not have functions with such big-ish if-else if-else blocks. Simply set a dedicated tooltip callback for assets, this can be much more concise then. All we need here is the asset representation I think, all the other variables/data just add noise.

Let's not have functions with such big-ish `if`-`else if`-`else` blocks. Simply set a dedicated tooltip callback for assets, this can be much more concise then. All we need here is the asset representation I think, all the other variables/data just add noise.
Harley marked this conversation as resolved
@ -112,0 +179,4 @@
if (file->typeflag & (FILE_TYPE_BLENDER | FILE_TYPE_BLENDER_BACKUP) &&
!(file->attributes & FILE_ATTR_OFFLINE)) {
/* Load Blender version directly from the file. */
short version = BLO_version_from_file(full_path);
Member

While using "minimal" .blend reading, BLO_version_from_file() still has to open a .blend file from disk, which can't just be assumed to be entirely trivial. E.g. on network drives, displaying this tooltip could freeze Blender for seconds, since everything happens on the main thread. We could query the version on file list loading (which uses a thread).

Opinion @mont29 / @ideasman42?

While using "minimal" .blend reading, `BLO_version_from_file()` still has to open a .blend file from disk, which can't just be assumed to be entirely trivial. E.g. on network drives, displaying this tooltip could freeze Blender for seconds, since everything happens on the main thread. We could query the version on file list loading (which uses a thread). Opinion @mont29 / @ideasman42?
Harley marked this conversation as resolved
Harley Acheson changed title from UI: Tooltips for Assets and File Browser Thumbnails to WIP UI: Tooltips for Assets 2023-04-11 17:40:03 +02:00
Harley Acheson force-pushed TooltipsSimple from 56d33ddb9f to 149b6dc338 2023-04-11 18:49:08 +02:00 Compare
Harley Acheson changed title from WIP UI: Tooltips for Assets to UI: Tooltips for Assets 2023-04-11 18:50:09 +02:00
Author
Member

@JulianEisel - Let's treat asset and file tooltips as separate patches

Done. This PR is now greatly simplified by only dealing with asset tooltips.

> @JulianEisel - Let's treat asset and file tooltips as separate patches Done. This PR is now greatly simplified by only dealing with asset tooltips.
Julian Eisel reviewed 2023-04-11 18:58:19 +02:00
@ -114,0 +117,4 @@
static char *file_draw_asset_tooltip_func(bContext * /*C*/, void *argN, const char * /*tip*/)
{
const AssetRepresentation *asset = static_cast<AssetRepresentation *>(argN);
char description[2048];
Member

I would suggest using std::string instead of a fixed size buffer. Removes the length limit and makes the code a lot more readable. Check node_errors_tooltip_fn() as reference.
(Sorry for only bringing this up now)

I would suggest using `std::string` instead of a fixed size buffer. Removes the length limit and makes the code a lot more readable. Check `node_errors_tooltip_fn()` as reference. (Sorry for only bringing this up now)
Harley marked this conversation as resolved
Author
Member

@JulianEisel - ...using std::string...a lot more readable.

Yes, can't argue with that; looks much nicer. Thanks!

@JulianEisel - ...using std::string...a lot more readable. Yes, can't argue with that; looks much nicer. Thanks!
Julian Eisel approved these changes 2023-04-12 13:19:13 +02:00
@ -114,0 +117,4 @@
static char *file_draw_asset_tooltip_func(bContext * /*C*/, void *argN, const char * /*tip*/)
{
const AssetRepresentation *asset = static_cast<AssetRepresentation *>(argN);
std::string complete_string (AS_asset_representation_name_get(asset));
Member

Would initialize like this std::string complete_string = AS_asset_representation_name_get(asset);.

Would initialize like this `std::string complete_string = AS_asset_representation_name_get(asset);`.
Harley marked this conversation as resolved
Harley Acheson force-pushed TooltipsSimple from 613c0584c2 to da31cc9b1b 2023-04-12 18:42:46 +02:00 Compare
Harley Acheson merged commit 0ee0e8c0d4 into main 2023-04-12 18:44:59 +02:00
Harley Acheson deleted branch TooltipsSimple 2023-04-12 18:45:00 +02:00
Sign in to join this conversation.
No reviewers
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Code Documentation
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
FBX
Interest
Freestyle
Interest
Geometry Nodes
Interest
glTF
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Asset System
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#106189
No description provided.