Page MenuHome
Differential D7595 Diff 24433 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,778 Lines • ▼ Show 20 Linesstatic void init_text_effect(Sequence *seq)
TextVars *data; TextVars *data;
if (seq->effectdata) { if (seq->effectdata) {
MEM_freeN(seq->effectdata); MEM_freeN(seq->effectdata);
} }
data = seq->effectdata = MEM_callocN(sizeof(TextVars), "textvars"); data = seq->effectdata = MEM_callocN(sizeof(TextVars), "textvars");
data->text_font = NULL; data->text_font = NULL;
data->text_blf_id = -1; data->text_blf_id = SEQ_FONT_NOT_LOADED;
data->text_size = 30; data->text_size = 30;
copy_v4_fl(data->color, 1.0f); copy_v4_fl(data->color, 1.0f);
data->shadow_color[3] = 1.0f; data->shadow_color[3] = 1.0f;
BLI_strncpy(data->text, "Text", sizeof(data->text)); BLI_strncpy(data->text, "Text", sizeof(data->text));
data->loc[0] = 0.5f; data->loc[0] = 0.5f;
data->align = SEQ_TEXT_ALIGN_X_CENTER; data->align = SEQ_TEXT_ALIGN_X_CENTER;
data->align_y = SEQ_TEXT_ALIGN_Y_BOTTOM; data->align_y = SEQ_TEXT_ALIGN_Y_BOTTOM;
} }
void BKE_sequencer_text_font_unload(TextVars *data, const bool do_id_user) void BKE_sequencer_blf_font_unload(TextVars *data)
{ {
if (data) { if (data && data->text_blf_id < 0) {
/* Unlink the VFont */ return;
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…
if (do_id_user && data->text_font != NULL) {
id_us_min(&data->text_font->id);
data->text_font = NULL;
} }
/* Unload the BLF font. */
if (data->text_blf_id >= 0) {
BLF_unload_id(data->text_blf_id); BLF_unload_id(data->text_blf_id);
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) /* BLF font is loaded on-demand. */
void BKE_sequencer_blf_font_load(TextVars *data)
{ {
if (data->text_font != NULL) { if (data && data->text_blf_id != SEQ_FONT_NOT_LOADED) {
if (do_id_user) { return;
id_us_plus(&data->text_font->id);
} }
/* Fallback to default font. Never unload this font. */
data->text_blf_id = -1;
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)`
if (data->text_font != NULL) {
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);
} }
} }
void BKE_sequencer_text_vfont_unload(TextVars *data, const bool do_id_user)
{
if (!do_id_user || data == NULL || data->text_font == NULL) {
return;
}
/* Unlink the VFont */
id_us_min(&data->text_font->id);
data->text_font = NULL;
}
void BKE_sequencer_text_vfont_load(TextVars *data, const bool do_id_user)
{
if (!do_id_user || data == NULL || data->text_font == NULL) {
return;
}
id_us_plus(&data->text_font->id);
}
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 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_vfont_unload(data, do_id_user);
if (do_id_user) {
BKE_sequencer_blf_font_unload(data);
}
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); /* Font usercount is restored when .blend file is read, so don't increase usercount here. */
BKE_sequencer_text_vfont_load(data, false);
} }
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…
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;
TextVars *srcdata = src->effectdata;
data->text_blf_id = -1; data->text_blf_id = SEQ_FONT_NOT_LOADED;
BKE_sequencer_text_font_load(data, (flag & LIB_ID_CREATE_NO_USER_REFCOUNT) == 0); BKE_sequencer_text_vfont_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) { BKE_sequencer_blf_font_load(data);
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);
▲ Show 20 Lines • Show All 389 Lines • Show Last 20 Lines