Page MenuHome
Differential D7595 Diff 24285 source/blender/blenkernel/intern/seqeffects.c

Changeset View

Standalone View

View Options

source/blender/blenkernel/intern/seqeffects.c

Show First 20 Lines • Show All 3,798 Lines • ▼ Show 20 Lines
void BKE_sequencer_text_font_unload(TextVars *data, const bool do_id_user) void BKE_sequencer_text_font_unload(TextVars *data, const bool do_id_user)
{ {
if (data) { if (data) {
/* Unlink the VFont */ /* Unlink the VFont */
if (do_id_user && data->text_font != NULL) { if (do_id_user && data->text_font != NULL) {
id_us_min(&data->text_font->id); id_us_min(&data->text_font->id);
data->text_font = NULL; data->text_font = NULL;
}
/* Unload the BLF font. */ /* Unload the BLF font. */
if (data->text_blf_id >= 0) { if (data->text_blf_id >= 0) {
BLF_unload_id(data->text_blf_id); BLF_unload_id(data->text_blf_id);
} }
} }
sergeyUnsubmitted
Done
Inline Actions

If data can indeed be NULL then it should be if (data == NULL || data->text_blf_id < 0). Otherwise you have NULL-pointer dereference few lines below.

Is also weird to have checks < 0 in some cases and comparison with SEQ_FONT_NOT_LOADED in others.

sergey: If `data` can indeed be `NULL` then it should be `if (data == NULL || data->text_blf_id < 0)`.
brechtUnsubmitted
Done
Inline Actions

I guess having a #define SEQ_FONT_DEFAULT -1 could help here?

Then the test would be (data == NULL || ELEME(data->text_blf_id, SEQ_FONT_NOT_LOADED, SEQ_FONT_DEFAULT)

brecht: I guess having a `#define SEQ_FONT_DEFAULT -1` could help here? Then the test would be `(data…
} }
}
brechtUnsubmitted
Done
Inline Actions

I would expect data->text_blf_id = SEQ_FONT_NOT_LOADED; here.

brecht: I would expect `data->text_blf_id = SEQ_FONT_NOT_LOADED;` here.
void BKE_sequencer_text_font_load(TextVars *data, const bool do_id_user) void BKE_sequencer_text_font_load(TextVars *data, const bool do_id_user)
{ {
if (data->text_font != NULL) { if (data->text_font != NULL) {
if (do_id_user) { if (do_id_user) {
id_us_plus(&data->text_font->id); id_us_plus(&data->text_font->id);
}
brechtUnsubmitted
Done
Inline Actions

This should be (data == NULL || data->text_blf_id != SEQ_FONT_NOT_LOADED)

brecht: This should be `(data == NULL || data->text_blf_id != SEQ_FONT_NOT_LOADED)`
char path[FILE_MAX]; char path[FILE_MAX];
STRNCPY(path, data->text_font->name); STRNCPY(path, data->text_font->name);
BLI_assert(BLI_thread_is_main()); BLI_assert(BLI_thread_is_main());
BLI_path_abs(path, ID_BLEND_PATH_FROM_GLOBAL(&data->text_font->id)); BLI_path_abs(path, ID_BLEND_PATH_FROM_GLOBAL(&data->text_font->id));
data->text_blf_id = BLF_load(path); data->text_blf_id = BLF_load(path);
} }
} }
}
static void free_text_effect(Sequence *seq, const bool do_id_user) static void free_text_effect(Sequence *seq, const bool do_id_user)
{ {
TextVars *data = seq->effectdata; TextVars *data = seq->effectdata;
BKE_sequencer_text_font_unload(data, do_id_user); BKE_sequencer_text_font_unload(data, do_id_user);
if (data) { if (data) {
MEM_freeN(data); MEM_freeN(data);
seq->effectdata = NULL; seq->effectdata = NULL;
} }
} }
static void load_text_effect(Sequence *seq) static void load_text_effect(Sequence *seq)
{ {
TextVars *data = seq->effectdata; TextVars *data = seq->effectdata;
BKE_sequencer_text_font_load(data, false); BKE_sequencer_text_font_load(data, true);
/* Last used ID will have usercount recovered on file read. */
id_us_min(&data->text_font->id);
sergeyUnsubmitted
Done
Inline Actions

This seems weird to call BKE_sequencer_text_font_load() with do_id_user=true and then effectively cancel if out doing id_us_min.

sergey: This seems weird to call `BKE_sequencer_text_font_load()` with `do_id_user=true` and then…
} }
brechtUnsubmitted
Done
Inline Actions

I think these vfont load/unload functions should be removed, the name is really confusing since it does not actually load, unload.

Instead it should just do id_us_min/plus wherever these are called now.

brecht: I think these vfont load/unload functions should be removed, the name is really confusing since…
static void copy_text_effect(Sequence *dst, Sequence *src, const int flag) static void copy_text_effect(Sequence *dst, Sequence *src, const int flag)
{ {
dst->effectdata = MEM_dupallocN(src->effectdata); dst->effectdata = MEM_dupallocN(src->effectdata);
TextVars *data = dst->effectdata; TextVars *data = dst->effectdata;
data->text_blf_id = -1;
BKE_sequencer_text_font_load(data, (flag & LIB_ID_CREATE_NO_USER_REFCOUNT) == 0); BKE_sequencer_text_font_load(data, (flag & LIB_ID_CREATE_NO_USER_REFCOUNT) == 0);
sergeyUnsubmitted
Done
Inline Actions

Would say that BKE_sequencer_text_font_load() is to be avoided if doing copy for depsgraph.

sergey: Would say that `BKE_sequencer_text_font_load()` is to be avoided if doing copy for depsgraph.
ISSAuthorUnsubmitted
Done
Inline Actions

I need to copy data from original strip, but not increase usercount when doing copy for depsgraph.
But in case of doing real copy by duplicating strips I have to increase usercounts.

ISS: I need to copy data from original strip, but not increase usercount when doing copy for…
} }
static int num_inputs_text(void) static int num_inputs_text(void)
{ {
return 0; return 0;
} }
static int early_out_text(Sequence *seq, float UNUSED(facf0), float UNUSED(facf1)) static int early_out_text(Sequence *seq, float UNUSED(facf0), float UNUSED(facf1))
Show All 22 Linesstatic ImBuf *do_text_effect(const SeqRenderData *context,
int height = out->y; int height = out->y;
struct ColorManagedDisplay *display; struct ColorManagedDisplay *display;
const char *display_device; const char *display_device;
int font = blf_mono_font_render; int font = blf_mono_font_render;
int line_height; int line_height;
int y_ofs, x, y; int y_ofs, x, y;
double proxy_size_comp; double proxy_size_comp;
if (data->text_blf_id == SEQ_FONT_NOT_LOADED) {
data->text_blf_id = -1;
if (data->text_font) {
data->text_blf_id = BLF_load(data->text_font->name);
}
}
if (data->text_blf_id >= 0) { if (data->text_blf_id >= 0) {
font = data->text_blf_id; font = data->text_blf_id;
} }
display_device = context->scene->display_settings.display_device; display_device = context->scene->display_settings.display_device;
display = IMB_colormanagement_display_get_named(display_device); display = IMB_colormanagement_display_get_named(display_device);
/* Compensate text size for preview render size. */ /* Compensate text size for preview render size. */
▲ Show 20 Lines • Show All 388 Lines • Show Last 20 Lines