Implementation of new blendfile compatibility policy #110109

Closed
Bastien Montagne wants to merge 16 commits from mont29/blender:F-109151 into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
19 changed files with 659 additions and 45 deletions

View File

@ -28,7 +28,7 @@ class STATUSBAR_HT_header(Header):
row.alignment = 'RIGHT'
# Stats & Info
row.label(text=context.screen.statusbar_info(), translate=False)
layout.template_status_info()
Review

Rather than just moving this all to an ad-hoc template (which kind of defeats the purpose of templates), I'd propose something like this:

if context.blend_data.has_compatibility_issues:
  layout.template_compatibility_issues()
else:
  row.label(text=context.screen.statusbar_info(), translate=False)
Rather than just moving this all to an ad-hoc template (which kind of defeats the purpose of templates), I'd propose something like this: ```py if context.blend_data.has_compatibility_issues: layout.template_compatibility_issues() else: row.label(text=context.screen.statusbar_info(), translate=False) ```
Review

I would rather not. I do think the whole status info area does deserve a template - this warning about forward compatibility is only the initial complex UI change here, there are potentially many more (missing linked data? missing external files? etc.). not to mention also the rest of the status info could use some love.

Find it way more logical and coherent as a single template

I would rather not. I do think the whole status info area does deserve a template - this warning about forward compatibility is only the initial complex UI change here, there are potentially many more (missing linked data? missing external files? etc.). not to mention also the rest of the status info could use some love. Find it way more logical and coherent as a single template
classes = (

View File

@ -3,6 +3,8 @@
* SPDX-License-Identifier: GPL-2.0-or-later */
#pragma once
#include "BLI_utildefines.h"
#ifdef __cplusplus
extern "C" {
#endif
@ -41,6 +43,19 @@ const char *BKE_blender_version_string(void);
/* Returns true when version cycle is alpha, otherwise (beta, rc) returns false. */
bool BKE_blender_version_is_alpha(void);
/** Fill in given string buffer with user-readable formated file version and subversion (if
* provided).
*
* \param str_buff a char buffer where the formated string is written, minimal recommended size is
* 8, or 16 if subversion is provided.
*
* \param file_subversion the file subversion, if given value < 0, it is ignored, and only the
* `file_version` is used. */
mont29 marked this conversation as resolved Outdated

Is it the size of the buffer which needs to be 8, or the length (which makes it a requirement of the buffer size of 9) ?

Is it the size of the buffer which needs to be 8, or the length (which makes it a requirement of the buffer size of 9) ?
void BKE_blender_version_blendfile_string_from_values(char *str_buff,
const size_t str_buff_len,
const short file_version,
const short file_subversion);
#ifdef __cplusplus
}
#endif

View File

@ -48,6 +48,13 @@ bool BKE_blendfile_library_path_explode(const char *path,
char **r_group,
char **r_name);
/**
* Check whether a given path is actually a Blender-readable, valid .blend file.
*
* \note Currently does attempt to open and read (part of) the given file.
mont29 marked this conversation as resolved Outdated

The name & comment bother me a bit. This looks like a simple query, maybe checking if the path exists. But it actually opens the file and collects reports from that. So it's more of a BKE_blendfile_attempt_to_open_and_close(). A bit of an awkward name though, at least the comment should be more clear.

This may not actually be needed though (see below regarding library hint drawing).

The name & comment bother me a bit. This looks like a simple query, maybe checking if the path exists. But it actually opens the file and collects reports from that. So it's more of a `BKE_blendfile_attempt_to_open_and_close()`. A bit of an awkward name though, at least the comment should be more clear. This may not actually be needed though (see below regarding library hint drawing).
*/
bool BKE_blendfile_is_readable(const char *path, struct ReportList *reports);
/**
* Shared setup function that makes the data from `bfd` into the current blend file,
* replacing the contents of #G.main.

View File

@ -138,6 +138,13 @@ typedef struct Main {
char filepath[1024]; /* 1024 = FILE_MAX */
short versionfile, subversionfile; /* see BLENDER_FILE_VERSION, BLENDER_FILE_SUBVERSION */
short minversionfile, minsubversionfile;
/** The currently opened .blend file was written from a newer version of Blender, and has forward
* compatibility issues (data loss).
*
* \note: In practice currently this is only based on the version numbers, in the future it
* could try to use more refined detection on load. */
bool has_forward_compatibility_issues;
/** Commit timestamp from `buildinfo`. */
uint64_t build_commit_timestamp;
/** Commit Hash from `buildinfo`. */
@ -458,6 +465,10 @@ int set_listbasepointers(struct Main *main, struct ListBase *lb[]);
((main)->versionfile < (ver) || \
((main)->versionfile == (ver) && (main)->subversionfile < (subver)))
#define MAIN_VERSION_FILE_OLDER_OR_EQUAL(main, ver, subver) \
mont29 marked this conversation as resolved Outdated

MAIN_VERSION_OLDER_OR_EQUAL -> MAIN_VERSION_FILE_OLDER_OR_EQUAL for clarity?

My first guess without looking at the implementation was that this was not about the file version.

MAIN_VERSION_OLDER_OR_EQUAL -> MAIN_VERSION_FILE_OLDER_OR_EQUAL for clarity? My first guess without looking at the implementation was that this was not about the file version.

This follows naming of existing MAIN_VERSION_ATLEAST and MAIN_VERSION_OLDER.

If we want to state these are for file version, all need to be cleanedup first (which I don't mind doing at all, actually ;) ).

This follows naming of existing `MAIN_VERSION_ATLEAST` and `MAIN_VERSION_OLDER`. If we want to state these are for file version, all need to be cleanedup first (which I don't mind doing at all, actually ;) ).
((main)->versionfile < (ver) || \
((main)->versionfile == (ver) && (main)->subversionfile <= (subver)))
/**
* The size of thumbnails (optionally) stored in the `.blend` files header.
*

View File

@ -125,6 +125,26 @@ const char *BKE_blender_version_string()
return blender_version_string;
}
void BKE_blender_version_blendfile_string_from_values(char *str_buff,
const size_t str_buff_len,
const short file_version,
const short file_subversion)
{
const short file_version_major = file_version / 100;
const short file_version_minor = file_version % 100;
if (file_subversion >= 0) {
BLI_snprintf(str_buff,
str_buff_len,
"%d.%d (sub %d)",
file_version_major,
file_version_minor,
file_subversion);
}
else {
BLI_snprintf(str_buff, str_buff_len, "%d.%d", file_version_major, file_version_minor);
}
}
bool BKE_blender_version_is_alpha()
{
bool is_alpha = STREQ(STRINGIFY(BLENDER_VERSION_CYCLE), "alpha");

View File

@ -142,6 +142,18 @@ bool BKE_blendfile_library_path_explode(const char *path,
return true;
}
bool BKE_blendfile_is_readable(const char *path, ReportList *reports)
{
BlendFileReadReport readfile_reports;
readfile_reports.reports = reports;
BlendHandle *bh = BLO_blendhandle_from_file(path, &readfile_reports);
if (bh != nullptr) {
BLO_blendhandle_close(bh);
return true;
}
return false;
}
/** \} */
/* -------------------------------------------------------------------- */

View File

@ -65,6 +65,7 @@
#include "BKE_anim_data.h"
#include "BKE_animsys.h"
#include "BKE_asset.h"
#include "BKE_blender_version.h"
#include "BKE_collection.h"
#include "BKE_global.h" /* for G */
#include "BKE_idprop.h"
@ -410,6 +411,8 @@ void blo_split_main(ListBase *mainlist, Main *main)
libmain->curlib = lib;
libmain->versionfile = lib->versionfile;
libmain->subversionfile = lib->subversionfile;
libmain->has_forward_compatibility_issues = !MAIN_VERSION_FILE_OLDER_OR_EQUAL(
libmain, BLENDER_FILE_VERSION, BLENDER_FILE_SUBVERSION);
BLI_addtail(mainlist, libmain);
lib->temp_index = i;
lib_main_array[i] = libmain;
@ -1078,6 +1081,53 @@ static FileData *filedata_new(BlendFileReadReport *reports)
return fd;
}
/** Check if minversion of the file is older than current Blender, return false if it is not.
* Should only be called after #read_file_dna was successfuly executed. */
static bool is_minversion_older_than_blender(FileData *fd, ReportList *reports)
mont29 marked this conversation as resolved Outdated

The "check" semantic of functions is hard to understand by themselves. Having a function which "check condition" and return false if the condition is true makes it even harder.

Not sure the blo_ prefix helps here, so is_minversion_older_than_blender will be very hard to misread when reading code later on if (is_minversion_older_than_blender(fd, reports)) { ... }

The "check" semantic of functions is hard to understand by themselves. Having a function which "check condition" and return false if the condition is true makes it even harder. Not sure the `blo_` prefix helps here, so `is_minversion_older_than_blender` will be very hard to misread when reading code later on `if (is_minversion_older_than_blender(fd, reports)) { ... } `

I guess it's strictly older or equal. Could be named is_minversion_newer_than_blender and return value flipped if we want to brief.

I guess it's strictly older or equal. Could be named `is_minversion_newer_than_blender` and return value flipped if we want to brief.
Review

The name is actually fine (it does a strictly 'older than' check), but returned values were reversed compared to documentation and expected logical behavior. Thanks for catching this!

The name is actually fine (it does a strictly 'older than' check), but returned values were reversed compared to documentation and expected logical behavior. Thanks for catching this!
{
BLI_assert(fd->filesdna != nullptr);
for (BHead *bhead = blo_bhead_first(fd); bhead; bhead = blo_bhead_next(fd, bhead)) {
if (bhead->code != BLO_CODE_GLOB) {
mont29 marked this conversation as resolved Outdated

Yikes @ this big if-block - makes me scroll back and forth to check the scope :) Could use continue here?

Yikes @ this big if-block - makes me scroll back and forth to check the scope :) Could use `continue` here?
continue;
}
FileGlobal *fg = static_cast<FileGlobal *>(read_struct(fd, bhead, "Global"));
if ((fg->minversion > BLENDER_FILE_VERSION) ||
(fg->minversion == BLENDER_FILE_VERSION && fg->minsubversion > BLENDER_FILE_SUBVERSION))
{
char writer_ver_str[16];
char min_reader_ver_str[16];
if (fd->fileversion == fg->minversion) {
BKE_blender_version_blendfile_string_from_values(
writer_ver_str, sizeof(writer_ver_str), short(fd->fileversion), fg->subversion);
BKE_blender_version_blendfile_string_from_values(
min_reader_ver_str, sizeof(min_reader_ver_str), fg->minversion, fg->minsubversion);
}
else {
BKE_blender_version_blendfile_string_from_values(
writer_ver_str, sizeof(writer_ver_str), short(fd->fileversion), -1);
BKE_blender_version_blendfile_string_from_values(
min_reader_ver_str, sizeof(min_reader_ver_str), fg->minversion, -1);
}
BKE_reportf(reports,
RPT_ERROR,
TIP_("The file was saved by a newer version, open it with Blender %s or later"),
min_reader_ver_str);
CLOG_WARN(&LOG,
"%s: File saved by a newer version of Blender (%s), Blender %s or later is "
"needed to open it.",
fd->relabase,
writer_ver_str,
min_reader_ver_str);
mont29 marked this conversation as resolved Outdated

Break to avoid going through the entire file? Or can this exist multiple times?

Break to avoid going through the entire file? Or can this exist multiple times?

Indeed

Indeed
MEM_freeN(fg);
return true;
}
MEM_freeN(fg);
return false;
}
return false;
}
static FileData *blo_decode_and_check(FileData *fd, ReportList *reports)
{
decode_blender_header(fd);
@ -1090,6 +1140,10 @@ static FileData *blo_decode_and_check(FileData *fd, ReportList *reports)
blo_filedata_free(fd);
fd = nullptr;
}
if (is_minversion_older_than_blender(fd, reports)) {
blo_filedata_free(fd);
fd = nullptr;
}
}
else {
BKE_reportf(
@ -3010,10 +3064,15 @@ static BHead *read_global(BlendFileData *bfd, FileData *fd, BHead *bhead)
{
FileGlobal *fg = static_cast<FileGlobal *>(read_struct(fd, bhead, "Global"));
/* copy to bfd handle */
/* NOTE: `bfd->main->versionfile` is supposed to have already been set from `fd->fileversion`
* beforehand by calling code. */
bfd->main->subversionfile = fg->subversion;
bfd->main->has_forward_compatibility_issues = !MAIN_VERSION_FILE_OLDER_OR_EQUAL(
bfd->main, BLENDER_FILE_VERSION, BLENDER_FILE_SUBVERSION);
bfd->main->minversionfile = fg->minversion;
bfd->main->minsubversionfile = fg->minsubversion;
bfd->main->build_commit_timestamp = fg->build_commit_timestamp;
STRNCPY(bfd->main->build_hash, fg->build_hash);

View File

@ -18,6 +18,10 @@ struct wmWindowManager;
/* info_stats.c */
void ED_info_stats_clear(struct wmWindowManager *wm, struct ViewLayer *view_layer);
const char *ED_info_statusbar_string_ex(struct Main *bmain,
struct Scene *scene,
struct ViewLayer *view_layer,
const char statusbar_flag);
const char *ED_info_statusbar_string(struct Main *bmain,
struct Scene *scene,
struct ViewLayer *view_layer);

View File

@ -2496,6 +2496,7 @@ void uiTemplateHeader3D_mode(uiLayout *layout, struct bContext *C);
void uiTemplateEditModeSelection(uiLayout *layout, struct bContext *C);
void uiTemplateReportsBanner(uiLayout *layout, struct bContext *C);
void uiTemplateInputStatus(uiLayout *layout, struct bContext *C);
void uiTemplateStatusInfo(uiLayout *layout, struct bContext *C);
void uiTemplateKeymapItemProperties(uiLayout *layout, struct PointerRNA *ptr);
bool uiTemplateEventFromKeymapItem(struct uiLayout *layout,

View File

@ -42,6 +42,7 @@
#include "BLT_translation.h"
#include "BKE_action.h"
#include "BKE_blender_version.h"
#include "BKE_blendfile.h"
#include "BKE_cachefile.h"
#include "BKE_colorband.h"
@ -72,6 +73,7 @@
#include "DEG_depsgraph_query.h"
#include "ED_fileselect.h"
#include "ED_info.h"
#include "ED_object.h"
#include "ED_render.h"
#include "ED_screen.h"
@ -6507,6 +6509,126 @@ void uiTemplateInputStatus(uiLayout *layout, bContext *C)
}
}
void uiTemplateStatusInfo(uiLayout *layout, bContext *C)
{
Main *bmain = CTX_data_main(C);
Scene *scene = CTX_data_scene(C);
ViewLayer *view_layer = CTX_data_view_layer(C);
if (!bmain->has_forward_compatibility_issues) {
const char *status_info_txt = ED_info_statusbar_string(bmain, scene, view_layer);
uiItemL(layout, status_info_txt, ICON_NONE);
return;
}
/* Blender version part is shown as warning area when there are forward compatibility issues with
* currently loaded .blend file. */
const char *status_info_txt = ED_info_statusbar_string_ex(
bmain, scene, view_layer, (U.statusbar_flag & ~STATUSBAR_SHOW_VERSION));
uiItemL(layout, status_info_txt, ICON_NONE);
status_info_txt = ED_info_statusbar_string_ex(bmain, scene, view_layer, STATUSBAR_SHOW_VERSION);
uiBut *but;
const uiStyle *style = UI_style_get();
uiLayout *ui_abs = uiLayoutAbsolute(layout, false);
mont29 marked this conversation as resolved
Review

Yikes... But I see that's what regular reports do too... :/

Yikes... But I see that's what regular reports do too... :/
uiBlock *block = uiLayoutGetBlock(ui_abs);
eUIEmbossType previous_emboss = UI_block_emboss_get(block);
UI_fontstyle_set(&style->widgetlabel);
int width = int(
mont29 marked this conversation as resolved Outdated

Use functional style cast (int width = int(...)) https://wiki.blender.org/wiki/Style_Guide/C_Cpp#C.2B.2B_Type_Cast

Use functional style cast (`int width = int(...)`) https://wiki.blender.org/wiki/Style_Guide/C_Cpp#C.2B.2B_Type_Cast
BLF_width(style->widgetlabel.uifont_id, status_info_txt, strlen(status_info_txt)));
width = max_ii(width, int(10 * UI_SCALE_FAC));
mont29 marked this conversation as resolved Outdated

Functional style cast.

Functional style cast.
UI_block_align_begin(block);
/* Background for icon. */
but = uiDefBut(block,
UI_BTYPE_ROUNDBOX,
0,
"",
0,
0,
UI_UNIT_X + (6 * UI_SCALE_FAC),
UI_UNIT_Y,
nullptr,
0.0f,
0.0f,
0,
0,
"");
/* UI_BTYPE_ROUNDBOX's bg color is set in but->col. */
UI_GetThemeColorType4ubv(TH_INFO_WARNING, SPACE_INFO, but->col);
/* Background for the rest of the message. */
but = uiDefBut(block,
UI_BTYPE_ROUNDBOX,
0,
"",
UI_UNIT_X + (6 * UI_SCALE_FAC),
0,
UI_UNIT_X + width,
UI_UNIT_Y,
nullptr,
0.0f,
0.0f,
0,
0,
"");
/* Use icon background at low opacity to highlight, but still contrasting with area TH_TEXT. */
UI_GetThemeColorType4ubv(TH_INFO_WARNING, SPACE_INFO, but->col);
but->col[3] = 64;
UI_block_align_end(block);
UI_block_emboss_set(block, UI_EMBOSS_NONE);
/* The report icon itself. */
static char compat_error_msg[256];
char writer_ver_str[12];
BKE_blender_version_blendfile_string_from_values(
writer_ver_str, sizeof(writer_ver_str), bmain->versionfile, -1);
SNPRINTF(compat_error_msg,
TIP_("File saved by newer Blender\n(%s), expect loss of data"),
writer_ver_str);
but = uiDefIconBut(block,
UI_BTYPE_BUT,
0,
ICON_ERROR,
int(3 * UI_SCALE_FAC),
mont29 marked this conversation as resolved Outdated

Functional style cast.

Functional style cast.
0,
UI_UNIT_X,
UI_UNIT_Y,
nullptr,
0.0f,
0.0f,
0.0f,
0.0f,
compat_error_msg);
UI_GetThemeColorType4ubv(TH_INFO_WARNING_TEXT, SPACE_INFO, but->col);
but->col[3] = 255; /* This theme color is RBG only, so have to set alpha here. */
/* The report message. */
but = uiDefBut(block,
UI_BTYPE_BUT,
0,
status_info_txt,
UI_UNIT_X,
0,
short(width + UI_UNIT_X),
mont29 marked this conversation as resolved Outdated

Functional style cast.

Functional style cast.
UI_UNIT_Y,
nullptr,
0.0f,
0.0f,
0.0f,
0.0f,
compat_error_msg);
UI_block_emboss_set(block, previous_emboss);
}
/** \} */
/* -------------------------------------------------------------------- */

View File

@ -26,6 +26,7 @@
#include "BKE_blendfile.h"
#include "BKE_context.h"
#include "BKE_report.h"
#include "BLT_translation.h"
@ -1177,12 +1178,11 @@ void file_draw_list(const bContext *C, ARegion *region)
layout->curr_size = params->thumbnail_size;
}
static void file_draw_invalid_library_hint(const bContext *C,
const SpaceFile *sfile,
ARegion *region)
static void file_draw_invalid_asset_library_hint(const bContext *C,
const SpaceFile *sfile,
ARegion *region,
FileAssetSelectParams *asset_params)
{
const FileAssetSelectParams *asset_params = ED_fileselect_get_asset_params(sfile);
char library_ui_path[PATH_MAX];
file_path_to_ui_path(asset_params->base_params.dir, library_ui_path, sizeof(library_ui_path));
@ -1237,21 +1237,112 @@ static void file_draw_invalid_library_hint(const bContext *C,
}
}
static void file_draw_invalid_library_hint(const bContext * /*C*/,
const SpaceFile *sfile,
ARegion *region,
const char *blendfile_path,
ReportList *reports)
{
uchar text_col[4];
UI_GetThemeColor4ubv(TH_TEXT, text_col);
const View2D *v2d = &region->v2d;
const int pad = sfile->layout->tile_border_x;
const int width = BLI_rctf_size_x(&v2d->tot) - (2 * pad);
const int line_height = sfile->layout->textheight;
int sx = v2d->tot.xmin + pad;
/* For some reason no padding needed. */
int sy = v2d->tot.ymax;
{
const char *message = TIP_("Unreadable Blender library file:");
file_draw_string_multiline(sx, sy, message, width, line_height, text_col, nullptr, &sy);
sy -= line_height;
file_draw_string(sx, sy, blendfile_path, width, line_height, UI_STYLE_TEXT_LEFT, text_col);
}
/* Separate a bit further. */
sy -= line_height * 2.2f;
LISTBASE_FOREACH (Report *, report, &reports->list) {
const short report_type = report->type;
if (report_type <= RPT_INFO) {
continue;
}
int icon = ICON_INFO;
if (report_type > RPT_WARNING) {
icon = ICON_ERROR;
}
UI_icon_draw(sx, sy - UI_UNIT_Y, icon);
file_draw_string_multiline(sx + UI_UNIT_X,
sy,
TIP_(report->message),
width - UI_UNIT_X,
line_height,
text_col,
nullptr,
&sy);
sy -= line_height;
}
}
bool file_draw_hint_if_invalid(const bContext *C, const SpaceFile *sfile, ARegion *region)
{
FileAssetSelectParams *asset_params = ED_fileselect_get_asset_params(sfile);
/* Only for asset browser. */
if (!ED_fileselect_is_asset_browser(sfile)) {
return false;
}
/* Check if the library exists. */
if ((asset_params->asset_library_ref.type == ASSET_LIBRARY_LOCAL) ||
filelist_is_dir(sfile->files, asset_params->base_params.dir))
{
return false;
char blendfile_path[FILE_MAX_LIBEXTRA];
const bool is_asset_browser = ED_fileselect_is_asset_browser(sfile);
const bool is_library_browser = !is_asset_browser &&
filelist_islibrary(sfile->files, blendfile_path, nullptr);
mont29 marked this conversation as resolved
Review

Would remove this since the conditions are checked below anyway (redundant), and it has to be modified as new cases get added (which isn't clear while appending at the bottom of the function).

Would remove this since the conditions are checked below anyway (redundant), and it has to be modified as new cases get added (which isn't clear while appending at the bottom of the function).
if (is_asset_browser) {
FileAssetSelectParams *asset_params = ED_fileselect_get_asset_params(sfile);
/* Check if the asset library exists. */
if (!((asset_params->asset_library_ref.type == ASSET_LIBRARY_LOCAL) ||
filelist_is_dir(sfile->files, asset_params->base_params.dir)))
{
file_draw_invalid_asset_library_hint(C, sfile, region, asset_params);
return true;
}
}
file_draw_invalid_library_hint(C, sfile, region);
/* Check if the blendfile library is valid (has entries). */
if (is_library_browser) {
if (!filelist_is_ready(sfile->files)) {
return false;
}
return true;
const int numfiles = filelist_files_num_entries(sfile->files);
if (numfiles > 0) {
return false;
}
/* This could all be part of the file-list loading:
* - When loading fails this could be saved in the file-list, e.g. when
* `BLO_blendhandle_from_file()` returns null in `filelist_readjob_list_lib()`, a
* `FL_HAS_INVALID_LIBRARY` file-list flag could be set.
* - Reports from it could also be stored in `FileList` rather than being ignored
mont29 marked this conversation as resolved Outdated

Is this opening the file on every redraw? That seems like it could be a performance problem, especially with network drivers where there might be significant latency.

Can this be cached, like I assume other file data show in the file browser is?

Is this opening the file on every redraw? That seems like it could be a performance problem, especially with network drivers where there might be significant latency. Can this be cached, like I assume other file data show in the file browser is?

Eeeeek, good catch!

Eeeeek, good catch!
* (`RPT_STORE` must be set!).

This could all be part of the file-list loading:

  • When loading fails this can be saved in the file-list, e.g. when BLO_blendhandle_from_file() returns null in filelist_readjob_list_lib(), a FL_HAS_INVALID_LIBRARY file-list flag can be set.
  • Reports from it can also be stored in FileList rather than being ignored (RPT_STORE must be set!).
  • Then we can just check for is_library_browser and the FL_HAS_INVALID_LIBRARY flag here, and draw the hint with the reports in the file-list. (We won't draw a hint for recursive loading, even if the file-list has the "has invalid library" flag set, which seems like the wanted behavior.)
  • BKE_blendfile_is_readable() isn't needed then.

I'm not feeling too strongly about this though. It seems sensible to do this way and avoids hacking this in with the runtime struct. But may be more complicated than worth it. I admit it's also nice to keep things together in this rather small scope.

This could all be part of the file-list loading: - When loading fails this can be saved in the file-list, e.g. when `BLO_blendhandle_from_file()` returns null in `filelist_readjob_list_lib()`, a `FL_HAS_INVALID_LIBRARY` file-list flag can be set. - Reports from it can also be stored in `FileList` rather than being ignored (`RPT_STORE` must be set!). - Then we can just check for `is_library_browser` and the `FL_HAS_INVALID_LIBRARY` flag here, and draw the hint with the reports in the file-list. (We won't draw a hint for recursive loading, even if the file-list has the "has invalid library" flag set, which seems like the wanted behavior.) - `BKE_blendfile_is_readable()` isn't needed then. ---- I'm not feeling too strongly about this though. It seems sensible to do this way and avoids hacking this in with the runtime struct. But may be more complicated than worth it. I admit it's also nice to keep things together in this rather small scope.

i would rather keep that for a potential future improvement. Also keep in mind that this patch also needs to land in current LTS releases, so trying to keep it as limited as possible is important. It's already waaaaaayyy to big to my liking tbh :(

i would rather keep that for a potential future improvement. Also keep in mind that this patch also needs to land in current LTS releases, so trying to keep it as limited as possible is important. It's already waaaaaayyy to big to my liking tbh :(
* - Then we could just check for `is_library_browser` and the `FL_HAS_INVALID_LIBRARY` flag
* here, and draw the hint with the reports in the file-list. (We would not draw a hint for
* recursive loading, even if the file-list has the "has invalid library" flag set, which
* seems like the wanted behavior.)
* - The call to BKE_blendfile_is_readable() would not be needed then.
*/
if (!sfile->runtime->is_blendfile_status_set) {
BKE_reports_clear(&sfile->runtime->is_blendfile_readable_reports);
sfile->runtime->is_blendfile_readable = BKE_blendfile_is_readable(
blendfile_path, &sfile->runtime->is_blendfile_readable_reports);
sfile->runtime->is_blendfile_status_set = true;
}
if (!sfile->runtime->is_blendfile_readable) {
file_draw_invalid_library_hint(
C, sfile, region, blendfile_path, &sfile->runtime->is_blendfile_readable_reports);
return true;
}
}
return false;
}

View File

@ -9,6 +9,7 @@
#pragma once
#include "DNA_space_types.h"
#include "DNA_windowmanager_types.h"
#ifdef __cplusplus
extern "C" {
@ -180,6 +181,12 @@ typedef struct SpaceFile_Runtime {
* Use file_on_reload_callback_register() to register a callback. */
onReloadFn on_reload;
onReloadFnData on_reload_custom_data;
/* Indicates, if the current filepath is a blendfile library one, if its status has been checked,
* and if it is readable. */
bool is_blendfile_status_set;
bool is_blendfile_readable;
ReportList is_blendfile_readable_reports;
} SpaceFile_Runtime;
/**

View File

@ -1961,6 +1961,11 @@ BlendHandle *filelist_lib(FileList *filelist)
return filelist->libfiledata;
}
int filelist_files_num_entries(FileList *filelist)
mont29 marked this conversation as resolved
Review

Better make it clear in the name that this includes filtered out items. If I wanted to have the count of visible items, I'd just choose this function if it pops up in auto-completion (an actual bug).

Better make it clear in the name that this includes filtered out items. If I wanted to have the count of visible items, I'd just choose this function if it pops up in auto-completion (an actual bug).
Review

I would expect such a function returning the amount of filtered entries to be called filelist_files_num_entries_filtered or something like that... This would also match the internal data (entries_num vs. entries_filtered_num).

Will add comment, but imho name itself is fine.

I would expect such a function returning the amount of filtered entries to be called `filelist_files_num_entries_filtered` or something like that... This would also match the internal data (`entries_num` vs. `entries_filtered_num`). Will add comment, but imho name itself is fine.
{
return filelist->filelist.entries_num;
}
static const char *fileentry_uiname(const char *root, FileListInternEntry *entry, char *buff)
{
if (entry->asset) {

View File

@ -206,6 +206,10 @@ struct BlendHandle *filelist_lib(struct FileList *filelist);
bool filelist_islibrary(struct FileList *filelist, char *dir, char **r_group);
void filelist_freelib(struct FileList *filelist);
/** Return the total raw number of entries listed in the given `filelist`, whether they are
* filtered out or not. */
int filelist_files_num_entries(struct FileList *filelist);
void filelist_readjob_start(struct FileList *filelist,
int space_notifier,
const struct bContext *C);

View File

@ -20,6 +20,7 @@
#include "BKE_global.h"
#include "BKE_lib_remap.h"
#include "BKE_main.h"
#include "BKE_report.h"
#include "BKE_screen.h"
#include "RNA_access.h"
@ -126,6 +127,9 @@ static void file_free(SpaceLink *sl)
MEM_SAFE_FREE(sfile->params);
MEM_SAFE_FREE(sfile->asset_params);
if (sfile->runtime != nullptr) {
BKE_reports_clear(&sfile->runtime->is_blendfile_readable_reports);
}
MEM_SAFE_FREE(sfile->runtime);
MEM_SAFE_FREE(sfile->layout);
@ -143,6 +147,7 @@ static void file_init(wmWindowManager * /*wm*/, ScrArea *area)
if (sfile->runtime == nullptr) {
sfile->runtime = static_cast<SpaceFile_Runtime *>(
MEM_callocN(sizeof(*sfile->runtime), __func__));
BKE_reports_init(&sfile->runtime->is_blendfile_readable_reports, RPT_STORE);
}
/* Validate the params right after file read. */
fileselect_refresh_params(sfile);
@ -206,6 +211,10 @@ static void file_refresh(const bContext *C, ScrArea *area)
fileselect_refresh_params(sfile);
folder_history_list_ensure_for_active_browse_mode(sfile);
if (sfile->runtime != nullptr) {
sfile->runtime->is_blendfile_status_set = false;
}
if (sfile->files && (sfile->tags & FILE_TAG_REBUILD_MAIN_FILES) &&
filelist_needs_reset_on_main_changes(sfile->files))
{

View File

@ -602,10 +602,10 @@ static void get_stats_string(char *info,
info + *ofs, len - *ofs, TIP_(" | Objects:%s/%s"), stats_fmt->totobjsel, stats_fmt->totobj);
}
static const char *info_statusbar_string(Main *bmain,
Scene *scene,
ViewLayer *view_layer,
char statusbar_flag)
const char *ED_info_statusbar_string_ex(Main *bmain,
Scene *scene,
ViewLayer *view_layer,
const char statusbar_flag)
{
char formatted_mem[BLI_STR_FORMAT_INT64_BYTE_UNIT_SIZE];
size_t ofs = 0;
@ -685,7 +685,7 @@ static const char *info_statusbar_string(Main *bmain,
const char *ED_info_statusbar_string(Main *bmain, Scene *scene, ViewLayer *view_layer)
{
return info_statusbar_string(bmain, scene, view_layer, U.statusbar_flag);
return ED_info_statusbar_string_ex(bmain, scene, view_layer, U.statusbar_flag);
}
const char *ED_info_statistics_string(Main *bmain, Scene *scene, ViewLayer *view_layer)
@ -695,7 +695,7 @@ const char *ED_info_statistics_string(Main *bmain, Scene *scene, ViewLayer *view
STATUSBAR_SHOW_VERSION |
STATUSBAR_SHOW_SCENE_DURATION;
return info_statusbar_string(bmain, scene, view_layer, statistics_status_bar_flag);
return ED_info_statusbar_string_ex(bmain, scene, view_layer, statistics_status_bar_flag);
}
static void stats_row(int col1,

View File

@ -1840,6 +1840,9 @@ void RNA_api_ui_layout(StructRNA *srna)
func = RNA_def_function(srna, "template_input_status", "uiTemplateInputStatus");
RNA_def_function_flag(func, FUNC_USE_CONTEXT);
func = RNA_def_function(srna, "template_status_info", "uiTemplateStatusInfo");
RNA_def_function_flag(func, FUNC_USE_CONTEXT);
func = RNA_def_function(srna, "template_node_link", "uiTemplateNodeLink");
RNA_def_function_flag(func, FUNC_USE_CONTEXT);
parm = RNA_def_pointer(func, "ntree", "NodeTree", "", "");

View File

@ -63,6 +63,7 @@
#include "BKE_appdir.h"
#include "BKE_autoexec.h"
#include "BKE_blender.h"
#include "BKE_blender_version.h"
#include "BKE_blendfile.h"
#include "BKE_callbacks.h"
#include "BKE_context.h"
@ -3321,6 +3322,12 @@ static int wm_save_as_mainfile_exec(bContext *C, wmOperator *op)
return OPERATOR_CANCELLED;
}
if (!is_save_as) {
/* If saved as current file, there are technically no more compatibility issues, the file on
* disk now matches the currently opened data version-wise. */
bmain->has_forward_compatibility_issues = false;
}
WM_event_add_notifier(C, NC_WM | ND_FILESAVE, nullptr);
if (!is_save_as && RNA_boolean_get(op->ptr, "exit")) {
@ -3425,7 +3432,13 @@ static int wm_save_mainfile_invoke(bContext *C, wmOperator *op, const wmEvent *
}
if (blendfile_path[0] != '\0') {
ret = wm_save_as_mainfile_exec(C, op);
if (CTX_data_main(C)->has_forward_compatibility_issues) {
wm_save_file_forwardcompat_dialog(C, op);
ret = OPERATOR_INTERFACE;
}
else {
ret = wm_save_as_mainfile_exec(C, op);
}
}
else {
WM_event_add_fileselect(C, op);
@ -3723,6 +3736,206 @@ void wm_test_autorun_warning(bContext *C)
/** \} */
/* -------------------------------------------------------------------- */
/** \name Save File Forward Compatibility Dialog
* \{ */
static void free_post_file_close_action(void *arg)
{
wmGenericCallback *action = (wmGenericCallback *)arg;
WM_generic_callback_free(action);
}
static void wm_free_operator_properties_callback(void *user_data)
{
IDProperty *properties = (IDProperty *)user_data;
IDP_FreeProperty(properties);
}
static const char *save_file_forwardcompat_dialog_name = "save_file_forwardcompat_popup";
static void file_forwardcompat_detailed_info_show(uiLayout *parent_layout, Main *bmain)
{
uiLayout *layout = uiLayoutColumn(parent_layout, true);
/* Trick to make both lines of text below close enough to look like they are part of a same
* block. */
uiLayoutSetScaleY(layout, 0.70f);
char writer_ver_str[16];
char current_ver_str[16];
if (bmain->versionfile == BLENDER_VERSION) {
BKE_blender_version_blendfile_string_from_values(
writer_ver_str, sizeof(writer_ver_str), bmain->versionfile, bmain->subversionfile);
BKE_blender_version_blendfile_string_from_values(
current_ver_str, sizeof(current_ver_str), BLENDER_FILE_VERSION, BLENDER_FILE_SUBVERSION);
}
else {
BKE_blender_version_blendfile_string_from_values(
writer_ver_str, sizeof(writer_ver_str), bmain->versionfile, -1);
BKE_blender_version_blendfile_string_from_values(
current_ver_str, sizeof(current_ver_str), BLENDER_VERSION, -1);
}
char message_line1[256];
char message_line2[256];
SNPRINTF(message_line1,
TIP_("This file was saved by a newer version of Blender (%s)"),
writer_ver_str);
SNPRINTF(message_line2,
TIP_("Saving it with this Blender (%s) may cause loss of data"),
current_ver_str);
uiItemL(layout, message_line1, ICON_NONE);
uiItemL(layout, message_line2, ICON_NONE);
}
static void save_file_forwardcompat_cancel(bContext *C, void *arg_block, void * /*arg_data*/)
{
wmWindow *win = CTX_wm_window(C);
UI_popup_block_close(C, win, static_cast<uiBlock *>(arg_block));
}
static void save_file_forwardcompat_cancel_button(uiBlock *block, wmGenericCallback *post_action)
{
uiBut *but = uiDefIconTextBut(
block, UI_BTYPE_BUT, 0, 0, IFACE_("Cancel"), 0, 0, 0, UI_UNIT_Y, nullptr, 0, 0, 0, 0, "");
UI_but_func_set(but, save_file_forwardcompat_cancel, block, post_action);
UI_but_drawflag_disable(but, UI_BUT_TEXT_LEFT);
}
static void save_file_forwardcompat_overwrite(bContext *C, void *arg_block, void *arg_data)
{
wmWindow *win = CTX_wm_window(C);
UI_popup_block_close(C, win, static_cast<uiBlock *>(arg_block));
/* Re-use operator properties as defined for the initial 'save' operator, which triggered this
* 'forward compat' popup. */
wmGenericCallback *callback = WM_generic_callback_steal(
static_cast<wmGenericCallback *>(arg_data));
PointerRNA operator_propptr = {};
PointerRNA *operator_propptr_p = &operator_propptr;
IDProperty *operator_idproperties = static_cast<IDProperty *>(callback->user_data);
WM_operator_properties_alloc(&operator_propptr_p, &operator_idproperties, "WM_OT_save_mainfile");
WM_operator_name_call(C, "WM_OT_save_mainfile", WM_OP_EXEC_DEFAULT, operator_propptr_p, nullptr);
WM_generic_callback_free(callback);
}
static void save_file_forwardcompat_overwrite_button(uiBlock *block,
wmGenericCallback *post_action)
{
uiBut *but = uiDefIconTextBut(
block, UI_BTYPE_BUT, 0, 0, IFACE_("Overwrite"), 0, 0, 0, UI_UNIT_Y, nullptr, 0, 0, 0, 0, "");
UI_but_func_set(but, save_file_forwardcompat_overwrite, block, post_action);
UI_but_drawflag_disable(but, UI_BUT_TEXT_LEFT);
UI_but_flag_enable(but, UI_BUT_REDALERT);
}
static void save_file_forwardcompat_saveas(bContext *C, void *arg_block, void * /*arg_data*/)
{
wmWindow *win = CTX_wm_window(C);
UI_popup_block_close(C, win, static_cast<uiBlock *>(arg_block));
WM_operator_name_call(C, "WM_OT_save_as_mainfile", WM_OP_INVOKE_DEFAULT, nullptr, nullptr);
}
static void save_file_forwardcompat_saveas_button(uiBlock *block, wmGenericCallback *post_action)
{
uiBut *but = uiDefIconTextBut(block,
UI_BTYPE_BUT,
0,
0,
IFACE_("Save As..."),
0,
0,
0,
UI_UNIT_Y,
nullptr,
0,
0,
0,
0,
"");
UI_but_func_set(but, save_file_forwardcompat_saveas, block, post_action);
UI_but_drawflag_disable(but, UI_BUT_TEXT_LEFT);
UI_but_flag_enable(but, UI_BUT_ACTIVE_DEFAULT);
}
static uiBlock *block_create_save_file_forwardcompat_dialog(bContext *C,
ARegion *region,
void *arg1)
{
wmGenericCallback *post_action = static_cast<wmGenericCallback *>(arg1);
Main *bmain = CTX_data_main(C);
uiBlock *block = UI_block_begin(C, region, save_file_forwardcompat_dialog_name, UI_EMBOSS);
UI_block_flag_enable(
block, UI_BLOCK_KEEP_OPEN | UI_BLOCK_LOOP | UI_BLOCK_NO_WIN_CLIP | UI_BLOCK_NUMSELECT);
UI_block_theme_style_set(block, UI_BLOCK_THEME_STYLE_POPUP);
uiLayout *layout = uiItemsAlertBox(block, 34, ALERT_ICON_WARNING);
/* Title. */
uiItemL_ex(
layout, TIP_("Overwrite file with an older Blender version?"), ICON_NONE, true, false);
/* Filename. */
const char *blendfile_path = BKE_main_blendfile_path(CTX_data_main(C));
char filename[FILE_MAX];
if (blendfile_path[0] != '\0') {
BLI_path_split_file_part(blendfile_path, filename, sizeof(filename));
}
else {
SNPRINTF(filename, "%s.blend", DATA_("untitled"));
/* Since this dialog should only be shown when re-saving an existing file, current filepath
* should never be empty. */
BLI_assert_unreachable();
}
uiItemL(layout, filename, ICON_NONE);
/* Detailed message info. */
file_forwardcompat_detailed_info_show(layout, bmain);
uiItemS_ex(layout, 4.0f);
/* Buttons. */
uiLayout *split = uiLayoutSplit(layout, 0.3f, true);
uiLayoutSetScaleY(split, 1.2f);
uiLayoutColumn(split, false);
save_file_forwardcompat_overwrite_button(block, post_action);
uiLayout *split_right = uiLayoutSplit(split, 0.1f, true);
uiLayoutColumn(split_right, false);
/* Empty space. */
uiLayoutColumn(split_right, false);
save_file_forwardcompat_cancel_button(block, post_action);
uiLayoutColumn(split_right, false);
save_file_forwardcompat_saveas_button(block, post_action);
UI_block_bounds_set_centered(block, 14 * UI_SCALE_FAC);
return block;
}
void wm_save_file_forwardcompat_dialog(bContext *C, wmOperator *op)
{
if (!UI_popup_block_name_exists(CTX_wm_screen(C), save_file_forwardcompat_dialog_name)) {
wmGenericCallback *callback = MEM_cnew<wmGenericCallback>(__func__);
callback->exec = nullptr;
callback->user_data = IDP_CopyProperty(op->properties);
callback->free_user_data = wm_free_operator_properties_callback;
UI_popup_block_invoke(
C, block_create_save_file_forwardcompat_dialog, callback, free_post_file_close_action);
}
}
/** \} */
/* -------------------------------------------------------------------- */
/** \name Close File Dialog
* \{ */
@ -3774,11 +3987,23 @@ static void wm_block_file_close_save(bContext *C, void *arg_block, void *arg_dat
bool file_has_been_saved_before = BKE_main_blendfile_path(bmain)[0] != '\0';
if (file_has_been_saved_before) {
if (WM_operator_name_call(C, "WM_OT_save_mainfile", WM_OP_EXEC_DEFAULT, nullptr, nullptr) &
OPERATOR_CANCELLED)
{
if (bmain->has_forward_compatibility_issues) {
/* Need to invoke to get the filebrowser and choose where to save the new file.
* This also makes it impossible to keep on going with current operation, which is why
* callback cannot be executed anymore.
*
* This is the same situation as what happens when the file has never been saved before
* (outer `else` statement, below). */
WM_operator_name_call(C, "WM_OT_save_as_mainfile", WM_OP_INVOKE_DEFAULT, nullptr, nullptr);
execute_callback = false;
}
else {
if (WM_operator_name_call(C, "WM_OT_save_mainfile", WM_OP_EXEC_DEFAULT, nullptr, nullptr) &
OPERATOR_CANCELLED)
{
execute_callback = false;
}
}
}
else {
WM_operator_name_call(C, "WM_OT_save_mainfile", WM_OP_INVOKE_DEFAULT, nullptr, nullptr);
@ -3820,10 +4045,27 @@ static void wm_block_file_close_discard_button(uiBlock *block, wmGenericCallback
UI_but_drawflag_disable(but, UI_BUT_TEXT_LEFT);
}
static void wm_block_file_close_save_button(uiBlock *block, wmGenericCallback *post_action)
static void wm_block_file_close_save_button(uiBlock *block,
wmGenericCallback *post_action,
const bool has_forwardcompat_issues)
{
uiBut *but = uiDefIconTextBut(
block, UI_BTYPE_BUT, 0, 0, IFACE_("Save"), 0, 0, 0, UI_UNIT_Y, nullptr, 0, 0, 0, 0, "");
block,
UI_BTYPE_BUT,
0,
0,
/* Forward compatibility issues force using 'save as' operator instead of 'save' one. */
has_forwardcompat_issues ? IFACE_("Save As...") : IFACE_("Save"),
0,
0,
0,
UI_UNIT_Y,
nullptr,
0,
0,
0,
0,
"");
UI_but_func_set(but, wm_block_file_close_save, block, post_action);
UI_but_drawflag_disable(but, UI_BUT_TEXT_LEFT);
UI_but_flag_enable(but, UI_BUT_ACTIVE_DEFAULT);
@ -3849,6 +4091,8 @@ static uiBlock *block_create__close_file_dialog(bContext *C, ARegion *region, vo
uiLayout *layout = uiItemsAlertBox(block, 34, ALERT_ICON_QUESTION);
const bool has_forwardcompat_issues = bmain->has_forward_compatibility_issues;
/* Title. */
uiItemL_ex(layout, TIP_("Save changes before closing?"), ICON_NONE, true, false);
@ -3863,6 +4107,11 @@ static uiBlock *block_create__close_file_dialog(bContext *C, ARegion *region, vo
}
uiItemL(layout, filename, ICON_NONE);
/* Potential forward compatibility issues message. */
if (has_forwardcompat_issues) {
file_forwardcompat_detailed_info_show(layout, bmain);
}
/* Image Saving Warnings. */
ReportList reports;
BKE_reports_init(&reports, RPT_STORE);
@ -3969,7 +4218,7 @@ static uiBlock *block_create__close_file_dialog(bContext *C, ARegion *region, vo
uiLayoutSetScaleY(split, 1.2f);
uiLayoutColumn(split, false);
wm_block_file_close_save_button(block, post_action);
wm_block_file_close_save_button(block, post_action, has_forwardcompat_issues);
uiLayoutColumn(split, false);
wm_block_file_close_discard_button(block, post_action);
@ -3995,19 +4244,13 @@ static uiBlock *block_create__close_file_dialog(bContext *C, ARegion *region, vo
wm_block_file_close_cancel_button(block, post_action);
uiLayoutColumn(split_right, false);
wm_block_file_close_save_button(block, post_action);
wm_block_file_close_save_button(block, post_action, has_forwardcompat_issues);
}
UI_block_bounds_set_centered(block, 14 * UI_SCALE_FAC);
return block;
}
static void free_post_file_close_action(void *arg)
{
wmGenericCallback *action = (wmGenericCallback *)arg;
WM_generic_callback_free(action);
}
void wm_close_file_dialog(bContext *C, wmGenericCallback *post_action)
{
if (!UI_popup_block_name_exists(CTX_wm_screen(C), close_file_dialog_name)) {
@ -4019,12 +4262,6 @@ void wm_close_file_dialog(bContext *C, wmGenericCallback *post_action)
}
}
static void wm_free_operator_properties_callback(void *user_data)
{
IDProperty *properties = (IDProperty *)user_data;
IDP_FreeProperty(properties);
}
bool wm_operator_close_file_dialog_if_needed(bContext *C,
wmOperator *op,
wmGenericCallbackFn post_action_fn)

View File

@ -89,6 +89,13 @@ bool wm_operator_close_file_dialog_if_needed(bContext *C,
* Check if there is data that would be lost when closing the current file without saving.
*/
bool wm_file_or_session_data_has_unsaved_changes(const Main *bmain, const wmWindowManager *wm);
/**
* Confirmation dialog when user is about to save the current blend file, and it was prviously
* created by a newer version of Blender.
*
* Important to ask confirmation, as this is a very common scenario of data loss.
*/
void wm_save_file_forwardcompat_dialog(bContext *C, wmOperator *op);
void WM_OT_save_homefile(struct wmOperatorType *ot);
void WM_OT_save_userpref(struct wmOperatorType *ot);