Page MenuHome

Add font selection to VSE text strips
Needs ReviewPublic

Authored by Justin Moore (DrChat) on Dec 21 2016, 2:52 AM.
Tokens
"Love" token, awarded by tintwotin."Love" token, awarded by patnard."Love" token, awarded by vklidu."Yellow Medal" token, awarded by looch."Love" token, awarded by davidmcsween."Love" token, awarded by pcote."Love" token, awarded by FraYoshi."Love" token, awarded by Lucas_Pierru."Like" token, awarded by Squareys."Like" token, awarded by oxben."Mountain of Wealth" token, awarded by januz."Love" token, awarded by Bawaria."Love" token, awarded by GuillaumeAmat."Like" token, awarded by Funkster."Like" token, awarded by aditiapratama.

Details

Summary

Allows users to select a font for their text strips in the video sequence editor.

This also fixes a minor bug where the load function isn't called on effect strips with blending enabled (this patch depends on that fix)

Diff Detail

Repository
rB Blender

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
release/scripts/startup/bl_ui/space_sequencer.py
674

Duplicate line

Justin Moore (DrChat) removed rB Blender as the repository for this revision.

tooltip

Justin Moore (DrChat) set the repository for this revision to rB Blender.Dec 22 2016, 2:43 AM
Justin Moore (DrChat) marked an inline comment as done.

Text effect copy function

release/scripts/startup/bl_ui/space_sequencer.py
674

Not a duplicate?

Great to see someone tackling this at last! :D

source/blender/blenfont/intern/blf.c
332

Although we don't have an explicit style rule about this, it's better to not perform assignments during conditionals.

source/blender/blenfont/intern/blf_internal_types.h
166

Perhaps a slightly more descriptive comment would be useful here? Like "number of users" or something like that?

One small request: Can you use the Blender font loading infrastructure like in the font object (see attached image)?

One small request: Can you use the Blender font loading infrastructure like in the font object (see attached image)?

I looked into doing that at first - but those are vector font datablocks which don't have anything to do with BLF fonts.
Was gonna add a "sequencer font" datablock but gave up cause I just couldn't figure out how.

Justin Moore (DrChat) marked 2 inline comments as done.

Change text defaults to appear centered w/ font size 90

Fix some misc. issues

Any news on this? It's been a while (January)...
@Justin Moore (DrChat) Maybe you need to mark that "duplicate" comment (which I also cannot recognize as such) as "Done" so that the blender maintainers look at it again?

This would be *amazing* to have. Rendering text as images in Blender just to have them in the Sequence Editor is a little cumbersome ;) (And without being able to select a font, this hasn't been too useful yet :/ )

Justin Moore (DrChat) marked 2 inline comments as done.May 17 2017, 6:37 PM

Any news on this? It's been a while (January)...
@Justin Moore (DrChat) Maybe you need to mark that "duplicate" comment (which I also cannot recognize as such) as "Done" so that the blender maintainers look at it again?

This would be *amazing* to have. Rendering text as images in Blender just to have them in the Sequence Editor is a little cumbersome ;) (And without being able to select a font, this hasn't been too useful yet :/ )

Yeah - I'd love to see this in mainline blender. I'm not too sure how to call their attention back to this issue, though.

We definitely need that feature !

Thus after modifications and build, the font selector is indeed appearing but not working :

It doesn't change the font of the text when applied and resets to an empty state. It's also printing this on the stdout : Can't find font: //roboto/RobotoCondensed-Bold.ttf
Looks like it's struggling to find the path...

Is it working on your side ? Could someone help me with this ?

I have this patch in my local builds (Fedora) and it works fine. Admittedly my source isn't really up to date, I think it's hovering somewhere close to 2.78c. It's been a while.

Well that's surprising since I'm also using build 2.78c as the source. I redownloaded a clean 2.78c source this morning and did all the process again (manually adding the code shown below then compiling) and I still have this "Can't find font: //folder/font.ttf" error. As for now I can't figure what I did wrong.

Have you tried specifying an absolute path for the font?

@Olly Funkster (Funkster) Spot on ! :) Nevermind, it's working when I manually enter the absolute path, relative path is still buged though...

Hello developers,

Having the possibility of customizing Font in the Video Editor is mostly welcome. Thanks for working on it!

Do you have any time estimate for the release of the current feature? I'm testing 2.79 RC1 at the moment but cannot see this feature.

Thank you very much!

P.S: Are there any plans for going a bit further and eventually add such things as Kerning and Tracking? That would rock! ^_^

Some questions this patch raises.

  • As Gottfried says, wouldn't it be better to use font data blocks instead of every user having to set the file path? This will become annoying fairly quickly if you ever want to change fonts.
  • BLF_MAX_FONT is currently set to 16, after this patch is applied its likely someone will hit the limit, although it's best to handle this in a separate patch.
source/blender/blenkernel/intern/seqeffects.c
3097

Prefer patches don't adjust defaults, again below.

3126

redundant cast.

3136

redundant cast.

source/blender/makesdna/DNA_sequence_types.h
277

Is it absolutely certain the value written into the blend is never used? - if not, it should be set to -1 in readfile.c

source/blender/makesrna/intern/rna_sequencer.c
466–467

redundant casts.

Some questions this patch raises.

  • As Gottfried says, wouldn't it be better to use font data blocks instead of every user having to set the file path? This will become annoying fairly quickly if you ever want to change fonts.
  • BLF_MAX_FONT is currently set to 16, after this patch is applied its likely someone will hit the limit, although it's best to handle this in a separate patch.

You're right - font data blocks would be much better. I've looked briefly into using the vector font data block, but as it is now, it doesn't use the BLF API.
I'm not experienced enough with blender's code to figure out a good way to add a reference to a BLF font in the vector font data block without help.

source/blender/blenkernel/intern/seqeffects.c
3097

I made this change because I feel it makes more sense that text effects should default as a centered title. As I understood this, text effects were originally hacked in as temporary subtitles.

If you insist though, I can revert these changes.

It could be a context toggle like in the viewport, when a new mesh is added. Settings are manteined if changed once.. so the text font, color, position and alignment could stay the same once these are changed at creation time.

there could be preset options like "subtitles", "title", "new preset"....

Using VFont data-blocks for sequencer would mean using the VFont only for it's filepath and still using BLF for rendering.
All things considered I think this is better than adding a different kind of font data-block, others devs may want to comment though.

source/blender/blenkernel/intern/seqeffects.c
3097

Best remove unrelated edits, its trivial to apply as a separate patch later.

Campbell Barton (campbellbarton) requested changes to this revision.Aug 25 2017, 11:44 AM

Checked and @Bastien Montagne (mont29) agrees it's best to use re-use VFont data-blocks.

This revision now requires changes to proceed.Aug 25 2017, 11:44 AM

Checked and @Bastien Montagne (mont29) agrees it's best to use re-use VFont data-blocks.

Sounds good to me. I've made most of the changes necessary, but I have a couple of questions:

  1. How do I increment the refcount for VFont while I'm using it?
  2. I've reused the UI template used by curves, but when trying to unlink the font, I'm told that it's being done in an incorrect context (editfont.c:1780).

How can I fix that?

See uses of id_us_plus, for the second question - it looks like font_unlink_exec could be made to work.
Need to investigate why UI_context_active_but_prop_get_templateID behaves differently in your case.

See uses of id_us_plus, for the second question - it looks like font_unlink_exec could be made to work.
Need to investigate why UI_context_active_but_prop_get_templateID behaves differently in your case.

I'll submit my changes so far without this working.

I've implemented refcounting for the font using id_us_plus and id_us_min, but I'm unsure if it's the correct way to go about it.
I'll definitely need a second person's opinion on that code.

source/blender/makesdna/DNA_sequence_types.h
277

As it was, I initialized text_blf_id to -1 in load_text_effect, but I can move that to readfile.c

Justin Moore (DrChat) edited edge metadata.

Piggybacking off of the VFont datablock instead of string font paths.

Justin Moore (DrChat) marked 2 inline comments as done.Aug 28 2017, 6:09 AM
Campbell Barton (campbellbarton) requested changes to this revision.Aug 30 2017, 5:01 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/makesrna/intern/rna_sequencer.c
484

The ID should be cleared even if the value is NULL.

484–488

There is no need to load the font here. Just invalidate the ID and allow the sequencer to load when it needs.

Also, returning if the font isn't found isn't a good convention, and not done anywhere else in Blender.

Just assign the font, if its not able to load, the user can correct the path of the font.

This revision now requires changes to proceed.Aug 30 2017, 5:01 AM
Justin Moore (DrChat) marked 5 inline comments as done.Dec 14 2017, 6:06 AM

Load BLF fonts when required rather than when selected.

Justin Moore (DrChat) marked 3 inline comments as done.Dec 14 2017, 7:51 AM

Still have the issue with unlinking fonts - incorrect context.

@Justin Moore (DrChat) I too am anxiously awaiting this feature. How's development coming along? Any chance we'll see this feature in 2.8?

I am testing this diff on 2.79.4
When I close and open blend file, VFont reference of text seq data is loaded with old memory address, but VFont block from blend file is loaded to new location. Sometimes on the "old memory address" there is random data, which will cause blender to crash.

I am not really familiar with blender code and my C skills are even worse :)
This should be possible to fix easily, but I don't know how exactly.

VFont linking solved this by adding newlibadr lookup in function lib_link_scene in file readfile.c

			Sequence *seq;
			SEQ_BEGIN (sce->ed, seq)
			{
				...

				if (seq->type == SEQ_TYPE_TEXT) {
					TextVars *t = seq->effectdata;
					t->text_font = newlibadr(fd, sce->id.lib, t->text_font);
				}

				...
			}
			SEQ_END

To accept relative path blf_dir.c was modified:

this uses global object, so I don't know if it is OK. If it's not, then there are multiple similar functions in blender code using this approach...
I guess, that when multiple blend files are linked together, this may be an issue...

...
#include "BKE_global.h"
#include "BKE_main.h"
...
char *blf_dir_search(const char *file)
{
	DirBLF *dir;
	char full_path[FILE_MAX];
	char *s = NULL;

	char temp[FILE_MAX];
	strcpy(temp, file);
	BLI_cleanup_path(G.main->name, temp);

	for (dir = global_font_dir.first; dir; dir = dir->next) {
		BLI_join_dirfile(full_path, sizeof(full_path), dir->path, file);
		if (BLI_exists(full_path)) {
			s = BLI_strdup(full_path);
			break;
		}
	}

	if (!s) {
		if (BLI_exists(temp))
			s = BLI_strdup(temp);
	}

	return s;
}
...

Incorrect context issue - in file wm_event_system.c in function wm_operator_call_internal() following code changes area of context.

...
				if (!(ar && ar->regiontype == type) && area) {
					ARegion *ar1;
					if (type == RGN_TYPE_WINDOW) {
						ar1 = BKE_area_find_region_active_win(area);
					}
					else {
						ar1 = BKE_area_find_region_type(area, type);
					}

					if (ar1)
						CTX_wm_region_set(C, ar1);
				}

				retval = wm_operator_invoke(C, ot, event, properties, reports, poll_only);
...

I don't know why (and if) this is needed, but I commented it out and everything seems to be working for me. But I do only video "editing"...

Anyway, I guess now I am a little bit familiar with blender source :)

@Campbell Barton (campbellbarton) @Justin Moore (DrChat)

I didn't want to spam, so I edited my post.
Can you please take a look at suggested modifications?

EDIT: attaching diff

@Richard Antalik (ISS) I would create a new differential as this one has messed up permissions. Unless an admin like @Campbell Barton (campbellbarton) can fix it.