UI: Improved Preview Feedback and Reduced Flickering #108486

Merged
Julian Eisel merged 5 commits from Harley/blender:BrowserWait into main 2023-06-19 12:24:50 +02:00
2 changed files with 36 additions and 18 deletions

View File

@ -359,6 +359,7 @@ static void file_draw_preview(const FileDirEntry *file,
bool show_outline = !is_icon &&
(file->typeflag & (FILE_TYPE_IMAGE | FILE_TYPE_MOVIE | FILE_TYPE_BLENDER));
const bool is_offline = (file->attributes & FILE_ATTR_OFFLINE);
const bool is_loading = file->flags & FILE_ENTRY_PREVIEW_LOADING;
BLI_assert(imb != nullptr);
@ -420,20 +421,23 @@ static void file_draw_preview(const FileDirEntry *file,
GPU_blend(GPU_BLEND_ALPHA_PREMULT);
}
IMMDrawPixelsTexState state = immDrawPixelsTexSetup(GPU_SHADER_3D_IMAGE_COLOR);
immDrawPixelsTexTiled_scaling(&state,
float(xco),
float(yco),
imb->x,
imb->y,
GPU_RGBA8,
true,
imb->byte_buffer.data,
scale,
scale,
1.0f,
1.0f,
document_img_col);
if (!is_loading) {
/* Don't show outer document image if loading - too flashy. */
IMMDrawPixelsTexState state = immDrawPixelsTexSetup(GPU_SHADER_3D_IMAGE_COLOR);
immDrawPixelsTexTiled_scaling(&state,
float(xco),
float(yco),
imb->x,
imb->y,
GPU_RGBA8,
true,
imb->byte_buffer.data,
scale,
scale,
1.0f,
1.0f,
document_img_col);
}
GPU_blend(GPU_BLEND_ALPHA);
@ -448,11 +452,17 @@ static void file_draw_preview(const FileDirEntry *file,
icon_color[1] = 255;
icon_color[2] = 255;
}
if (is_loading) {
/* Contrast with background since we are not showing the large document image. */
UI_GetThemeColor4ubv(TH_TEXT, icon_color);
Harley marked this conversation as resolved Outdated

const, also for the following vars.

`const`, also for the following vars.
}
Harley marked this conversation as resolved Outdated

Would move this to the top of the function, together with is_loading.

Would move this to the top of the function, together with `is_loading`.
icon_x = xco + (ex / 2.0f) - (icon_size / 2.0f);
icon_y = yco + (ey / 2.0f) - (icon_size * ((file->typeflag & FILE_TYPE_DIR) ? 0.78f : 0.75f));
Harley marked this conversation as resolved Outdated

Something reads wrong here: When the background is dark, we use a dark icon color?! I guess it works because we happen to be drawing a light document image in the back earlier. This dependency should be made explicit, so there are no surprises.

With a6e1caa1b2 in, I suggest to add a has_document_img boolean and check that to determine if either document_img_col or bgcolor should be used for the grayscale check.

Something reads wrong here: When the background is dark, we use a dark icon color?! I guess it works because we happen to be drawing a light document image in the back earlier. This dependency should be made explicit, so there are no surprises. With a6e1caa1b2 in, I suggest to add a `has_document_img` boolean and check that to determine if either `document_img_col` or `bgcolor` should be used for the grayscale check.
UI_icon_draw_ex(icon_x,
icon_y,
icon,
is_loading ? ICON_TEMP : icon,
icon_aspect / UI_SCALE_FAC,
icon_opacity,
0.0f,
@ -507,7 +517,7 @@ static void file_draw_preview(const FileDirEntry *file,
UI_NO_ICON_OVERLAY_TEXT);
}
}
else if (icon && !is_icon && !(file->typeflag & FILE_TYPE_FTFONT)) {
else if (icon && ((!is_icon && !(file->typeflag & FILE_TYPE_FTFONT)) || is_loading)) {
/* Smaller, fainter icon at bottom-left for preview image thumbnail, but not for fonts. */
float icon_x, icon_y;
const uchar dark[4] = {0, 0, 0, 255};

View File

@ -1628,7 +1628,15 @@ static void filelist_cache_previews_push(FileList *filelist, FileDirEntry *entry
* some time in heavy files, because otherwise for each missing preview and for each preview
* reload, we'd reopen the .blend to look for the preview. */
if ((entry->typeflag & FILE_TYPE_BLENDERLIB) &&
(entry->flags & FILE_ENTRY_BLENDERLIB_NO_PREVIEW)) {
(entry->flags & FILE_ENTRY_BLENDERLIB_NO_PREVIEW))
Harley marked this conversation as resolved Outdated

This makes the if statement unnecessarily complex and the comment above has no relation to this. Try to make if statements check one coherent condition, not multiple ones. So better make this a separate if statement with its own return.

This makes the `if` statement unnecessarily complex and the comment above has no relation to this. Try to make `if` statements check one coherent condition, not multiple ones. So better make this a separate `if` statement with its own `return`.
{
return;
}
/* External ID that is also a directory is never previewed. */
if ((entry->typeflag & (FILE_TYPE_BLENDERLIB | FILE_TYPE_DIR)) ==
(FILE_TYPE_BLENDERLIB | FILE_TYPE_DIR))
{
return;
}
@ -2604,7 +2612,6 @@ bool filelist_cache_previews_update(FileList *filelist)
/* Move ownership over icon. */
entry->preview_icon_id = preview->icon_id;
preview->icon_id = 0;
changed = true;
}
else {
/* We want to avoid re-processing this entry continuously!
@ -2613,6 +2620,7 @@ bool filelist_cache_previews_update(FileList *filelist)
entry->flags |= FILE_ENTRY_INVALID_PREVIEW;
}
entry->flags &= ~FILE_ENTRY_PREVIEW_LOADING;
changed = true;
}
else {
BKE_icon_delete(preview->icon_id);