Adds an "Auto" option to blend thumbnail types that will automatically
use Screenshot if there is no camera and a 3dview, or workbench render
otherwise with shading settings taken from the largest 3dview.
Details
Details
- Reviewers
Campbell Barton (campbellbarton) - Group Reviewers
User Interface - Commits
- rB4fa0bbb5ac3d: UI: Automatic Blend Thumbnail Selection
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
@Peter Fog (tintwotin) - Maybe there could also be a enum for Workspace types?
If you are thinking about the VSE note that "Screenshot" captures the entire window so shows VSE projects nicely. And "Area Capture" captures the largest editor, regardless of type, so also works great for that.
Have you tried this patch and found it did not work for some particular usage?
Comment Actions
I'd rather see D10492 be committed to master and get feedback on it before considering this patch.
Which 3D view options exactly are exposed is open ended, this patch exposes quite a few, perhaps users want shadows or X-ray too... I don't think we want to go down this path of expanding too many 3D viewport options into the preferences.
Instead there could be an Auto shading option, which uses the current shading option from the largest 3D view (if one exists). Otherwise it falls back to the current default (solid shading).
This has the advantage that:
- File save wont load have textures to load or compile shaders (causing unexpected slowdowns when saving). Although this might not always be the case if the user is in local-view for example.
- We don't have to expose a lot of options in the preferences.
- In most cases this is what users would expect.
Comment Actions
@Campbell Barton (campbellbarton) - I'd rather see D10492 be committed to master and get feedback on it before considering this patch.
Yes, me too. After this other goes I can turn into an "optional possible addon" that we can test and think about.
Which 3D view options exactly are exposed is open ended, this patch exposes quite a few, perhaps users want shadows or X-ray too... I don't think we want to go down this path of expanding too many 3D viewport options into the preferences.
When I was playing these a long time ago I found there weren't a lot of options in that some things just didn't make a difference at this small size while other options were nice in thumbnails but not in a regular render.
Instead there could be an Auto shading option, which uses the current shading option from the largest 3D view (if one exists). Otherwise it falls back to solid shading.
This has the advantage that:
- File save wont load have textures to load or compile shaders (causing unexpected slowdowns when saving). Although this might not always be the case if the user is in local-view for example.
- We don't have to expose a lot of options in the preferences.
- In most cases this is users would expect.
Yes, that does sound interesting.
I (obviously) don't mind if this particular patch goes nowhere but there might be utility is seeing what is possible, even if this just helps to eliminate what is not needed.
Comment Actions
Setting as requesting changes as I don't think the patch is acceptable in it's current form.
| source/blender/makesdna/DNA_userdef_types.h | ||
|---|---|---|
| 1018–1022 | Note that I don't think it makes sense to mix in shading types with the preview type. Shading could be a separate option, although (as noted previously), this could be detected from the current viewports. | |
Comment Actions
@Campbell Barton (campbellbarton) - Note that I don't think it makes sense to mix in shading types with the preview type…
For sure, I just hate the thought of adding options.
What if… we removed None. Then just have Auto (default), Camera, and Screenshot. With Auto we do Camera if we have both a 3dViewport and a scene camera, screenshot otherwise, which would work with VSE nicely. With Camera we always take shading from the largest 3DView. Not sure what you’d get if you explicitly set Camera though if you don’t have a camera or 3dview. Maybe use current code if either, just do screenshot if neither.
Comment Actions
Seems reasonable although I don't know why None would be removed, how would the option to disable thumbnails be selected?
Comment Actions
Well they wouldn’t disable them in that particular dream - I just like the thought of them always being available. But no worries, having some good luck with using existing shading settings. Seems like it could turn into something. But I do worry about people making a new habit of changing their current shading before saving just to have particular preview image (either better or faster), which would be sucky.
Comment Actions
There are some use-cases you may not want to enable them - having many small blend files for storing assets for access over the internet - for e.g. The file size overhead might matter in this case.
This is why I'd keep solid as an option since there is some small chance using the current shading settings causes problems.
Comment Actions
@Campbell Barton (campbellbarton) - Note that I don't think it makes sense to mix in shading types with the preview type
This version is a bit fun, with shading separated from preview type. Both with "Auto" choices, and keeping "None" of course.
This is why I'd keep solid as an option since there is some small chance using the current shading settings causes problems
I'm actually unclear if there is some combination that you liked that could be done with a single enum. "None, Auto, Camera, Screenshot" where auto uses v3d settings while "Camera" is just solid might be confusing.
This is cool to play with, but I don't like adding new preferences and I doubt anyone else feels a need for this. My motivation for being in here at all was the idea that if our previews were a little prettier then things like D10484 could be more interesting. And that was mostly to get some visual flourishes in for 3.0. But both could be added some some later wish list.
| source/blender/makesrna/intern/rna_userdef.c | ||
|---|---|---|
| 341–351 | Don't think this is needed U.file_preview_shading use can be limited to when U.file_preview_type is set to camera. | |
| source/blender/windowmanager/intern/wm_files.c | ||
| 1625 | This looks more like a bug-fix that could be committed to master. | |
| 1640–1643 | The region check here seems redundant. Only the area is needed. | |
| 1646 | typo == | |
| 1826–1837 | Since this is some more involved logic in this function, this should be surrounded by an USER_FILE_PREVIEW_NONE check. | |
Comment Actions
We could make "Camera" always use the 3D view shading setting, falling back to solid when none is found. In this case an extra setting wouldn't have to be added.
If users run into some cases where previews are not working well, we could always add a user-visible shading setting as a last resort.
Comment Actions
@Campbell Barton (campbellbarton) - We could make "Camera" always use the 3D view shading setting, falling back to solid when none is found...
Yes, that does make this much simpler.
[WM_redraw_windows] This looks more like a bug-fix that could be committed to master. Shouldn't this (also) be done for screenshot?
I took that out, along with the added WM_cursor_wait. I think these should be set earlier in the saving process, not just in this small part of it. Will look at that separately - will make a nice matching bookend for D11070.
| source/blender/windowmanager/intern/wm_files.c | ||
|---|---|---|
| 1632 | can be moved into the else block after if (v3d) {...}. | |
| 1639–1650 | These values deserve some explanation. For example, object color isn't used often compared to material color. Why does the outline color need to be set? In general suggest sticking to defaults, but if the defaults don't work well for thumbnails, there should be comments explaining why thumbnails cannot use a default. | |
| 1790–1798 | These checks can be expanded to be more readable, also, don't do a screenshot if user requested camera thumbnails (if users wanted that they can select Auto). int file_preview_type = U.file_preview_type;
if (file_preview_type == USER_FILE_PREVIEW_AUTO) {
... be clever ...
}
switch (file_preview_type) {
case USER_FILE_PREVIEW_SCREENSHOT: {...}
case USER_FILE_PREVIEW_CAMERA: {...}
default: BLI_assert_unceachable();
} | |
Comment Actions
@Campbell Barton (campbellbarton) - In general suggest sticking to defaults...
Actually if we are actually thinking about doing this, best to remove all the opinionated shading changes and just stick with the "Auto" idea alone. This version now becomes much simpler.
These checks can be expanded to be more readable
Yes, that is much more readable. Thanks!