Nasty crash if scale happens to be zero.

[hugoam]

A call to textBox if the scale happens to be zero result in a terrible crash: vg-renderer keeps allocating new text atlases until the app everything explodes, basically.

The core of the problem seems to be the logic here: https://github.com/jdryg/vg-renderer/blob/master/src/vg.cpp#L1934 Fons doesn't find the glyph because size is equal to zero, and vg's reaction to fons not finding the glyph is to allocate a new atlas :/

I don't know enough about vg-renderer internals to say where a security should be added, but something needs to be done somewhere :)

Also, the fact that the scale was set to zero is problematic: I didn't set a zero scale anywhere myself, it's something related to command lists. I'll let you know if I find the cause of that.

[jdryg]

Thanks for the report. I'll take a look asap.

The easiest way to avoid a crash is to clamp the scaled font size before passing it to fonsSetSize(). But if m_FontScale is actually 0.0f, then invscale will be inf so even if it doesn't crash it will render garbage.

Can you confirm that State::m_FontScale is actually zero and not Context::m_DevicePixelRatio here https://github.com/jdryg/vg-renderer/blob/master/src/vg.cpp#L1890 ? If it's State::m_FontScale then there should be something wrong in updateState() (https://github.com/jdryg/vg-renderer/blob/master/src/vg.cpp#L4598). This can be fixed by changing bx::min() to bx::clamp() and introducing a new compile time constant for the minimum font scale (VG_CONFIG_MIN_FONT_SCALE).

Also the minimum font size test I have in ctxText() (https://github.com/jdryg/vg-renderer/blob/master/src/vg.cpp#L3881) can be added at the beginning of ctxTextBox() to avoid calling textBreakLines() and text() at all in case the scaled font size is 0. It won't solve your problem (since your font size shouldn't actually be 0) but it could avoid the crash at an earlier time.

[hugoam]

From what I could gather, it's not the font scale, but the scale in the transform itself, that was zero. I believe this happened after I called "vg::beginCommandList": the state of the command list seems to be initialized with a Zero matrix instead of an Identity matrix ?

vg::resetCommandList(m_vg, this->layer_cache(layer));
vg::beginCommandList(m_vg, this->layer_cache(layer

I fixed it by adding a call to vg::transformIdentity

vg::resetCommandList(m_vg, this->layer_cache(layer));
vg::beginCommandList(m_vg, this->layer_cache(layer));
vg::transformIdentity(m_vg);

[jdryg]

I believe this happened after I called "vg::beginCommandList": the state of the command list seems to be initialized with a Zero matrix instead of an Identity matrix ?

Command lists don't maintain a separate state. They are just that; a list of commands to execute at a later point in time. Submitting a command list affects the context state at that time. If you don't pushState()/popState() inside your cmd list and your list includes transform commands, then the context state after submitCommandList() will be different than the initial state. If you don't want this behavior, you can enable VG_CONFIG_COMMAND_LIST_PRESERVE_STATE which automatically inserts pushState()/popState() before and after executing the commands.

Does the crash happens when calling textBox() while inside beginCommandList()/endCommandList() block or when calling submitCommandList() later with this specific cmdlist?

There are 2 issues here:

1. The code should have caught the error and avoid crashing when the scaled font size becomes 0. That's a bug and will be fixed.
2. Any function that indirectly uses the current state (e.g. textBreakLines()) doesn't work correctly inside a beginCommandList()/endCommandList() block. This might be trickier to solve and it was my primary concern when you suggested begin/end API for cmd lists.

Can you put a VG_CHECK() inside updateState() to check for m_AvgScale == 0.0f and m_FontScale == 0.0f? This might help pinpoint the exact point where the transformation is "destroyed".

[hugoam]

Well, I think we are in agreement in how command lists work (unless I overlooked something in my implementation). I am creating command lists at the root level (didn't set any transform). They contain transform commands and draw commands. Then when submitting the command list, I am doing :

vg::transformIdentity(m_vg);
vg::submitCommandList(m_vg, this->layer_cache(layer));

So I am even explicitly setting the "base" transform on which the command list transforms will be applied, to Identity. When the commands inside of the list execute though, there is somehow a Zero transform matrix that slipped in there if I don't explicitly add another identity transform at the root of the command list (which I find weird).

[jdryg]

The only thing I can think of right now is that there's a bug in ctxSubmitCommandList(). If you are certain that all your transformations are valid, there must be a bug in one of the commands and the buffer ptr isn't incremented correctly.

I assume you already tried running the code with VG_CONFIG_DEBUG enabled, right?

I understand that it might be too much to ask but is it possible to dump the problematic command list to a file and send it to me for debugging? Or maybe a minimal repro case as code.

[hugoam]

Hmm yes since you confirmed that it's indeed weird, I will try to isolate that bug. It might be something I'm doing wrong, I'll try to find where the cause is. I'll report back when I have more info !

[jdryg]

Slightly unrelated to your issue: I'm looking at ctxTextBox() and I was wondering, does it work correctly for you (*)? Do you use any alignment flag other than TextAlign::Left (vertical alignment doesn't matter)?

It seems like there's a bug in there. I should test it out properly and fix it but I thought of asking about your use case, to be sure I don't break anything accidentally.

EDIT: (*) when it doesn't crash :)

[jdryg]

Pushed some changes to textBox() and related functions. Shouldn't crash anymore even if the scaling of the current transformation matrix gets to 0.0. If the zero scale is a bug in submitCommandList() it will be fixed separately.

Please report back once you find the time to test it out. Thanks :)