Categories
Blender Stuff by other people that I helped with

I fixed a bug in Blender!

Summary: I found a bug in one of Blender’s Grease Pencil modifiers. In the process of prepping The Best Bug Report Ever, I fixed the bug instead with help and encouragement from Grease Pencil’s module owners. Read on if you’re interested in how Grease Pencil makes its lines thick and thin, how Grease Pencil’s Thickness modifier does the thing, and how my fix works (in Plain English). Big thanks to Antonio Vazquez and Mathias Mendiola for their patience, encouragement and guidance!

I wanted to test Blender’s Grease Pencil system out as an inking tool, because it allows me to go back and edit my lines a bit after I’ve drawn them. If a line is a bit wobbly or it doesn’t curve nicely or the width doesn’t taper properly, I can sculpt it and fix it after I put the line down, even going point by point if need be.

It was while I was wrestling with ways to adjust particularly lumpy stroke width with the Thickness modifier that I discovered a bug.

The bug and its habitat

The Thickness modifier for Blender’s Grease Pencil adjusts the thickness of Grease Pencil strokes. The Thickness modifier has a Normalize mode which sets the entire stroke to an even width.

Under the hood, “thickness” is a property of the strokes themselves. The strokes are made up of a series of points, and each point has a “pressure” value – the pressure of the graphics tablet for that point. Stroke thickness times point pressure determines the width of the stroke at that point.

Normalize works by setting the thickness for the stroke to whatever the modifier says it should be, then it sets the pressure for all its points to 1.0 – exactly the same pressure on each point means the width of the line becomes even. You can select a particular Vertex Group or Grease Pencil Layer in Normalize mode to target particular strokes and points for evening up while leaving the rest alone… or at least, that’s how it should have worked.

The bug was that the modifier was changing the width of all the strokes whether they had points in that vertex group or not. The effects of a modifier are not meant to leak like that.

I found Grease Pencil expert user Mathias Mendiola on blender.chat – he created the Grease Pencil Fundamentals course on Blender Cloud. He asked me to report a bug.

The best bug report ever

Now, reporting a bug to a fellow developer is not something I take lightly and I wanted to do it properly as a courtesy.

A good bug report should do the following:

  • reports a bug which hasn’t already been reported (let alone fixed), saving coders more administrative work
  • describes the problem clearly, concisely and precisely (less room for misinterpretation)
  • describes the environment and context the problem occurs in (which version of the software, e.g. stable or build hash on splash screen; which operating system, CPU and graphics card, etc)
  • describes expectations of what should happen (maybe the user’s expectations are not what developers expect them to be)
  • gives explicit instructions on how to replicate the bug with as few steps as possible, or alternatively provides data which shows the bug occurring

That last bit – how to replicate the bug – is super important. If the coder can’t replicate the bug, it’s much more difficult to fix it. If the coder is obliged to fix the bug anyway, they may have to spend a significant amount of time devising their own replication which may or may not be the same as what the user does when they trigger the bug, and even if it’s the same thing the user is doing it’s still time the coder has to spend pinpointing the source of the bug. Reproducing information which a user already knows (but hasn’t passed on) is not a good use of a coder’s time.

Since I’m a programmer myself and I build Blender from source, I wanted my bug report to go further than just instructions on how to replicate the bug – although I’d do that too. Once a coder confirms the bug is there and accepts it, the next step involves finding the bit of code that’s going wrong. I decided to find that bit of code as an exercise and to be extra helpful – one less thing for the coder to do.

Tracking down the bug

I poked around Visual Studio and eventually found the code inside source/blender/bf_gpencil_modifiers/MOD_gpencilthick.c. The function which contains the procedure for the Thickness modifier to modify thickness is called deformStroke.

By now, I wanted to pry even further and figure out what deformStroke was doing wrong so that I could even quote a particular line number in the bug report – again, one less thing for the coder to do. After hours of squinting, I figured out that even with Vertex Groups switched on, deformStroke was changing the thickness of the strokes whether they had points in the vertex group or not. That was fine if the whole stroke was getting normalised, but not if the stroke had some but not all points in the vertex group, or no points at all.. hmm..

Since I was on holidays and had no obligation to be awake and ready for work the next day, and I had Visual Studio open anyway, I decided to have a crack at fixing it myself – without any expectations of success or failure. If I couldn’t fix it, at least I put what I’d already tried in the bug report. Yet again, one less thing for the coder to do.

Failing to report the bug

After a few more hours of tinkering, headscratching, reading code, compiling, swearing and recompiling, I didn’t have a bug report – I had a fix for the bug. It was only a partial fix – for cases where a stroke has a mixture of affected and unaffected points, my original patch left that stroke alone – but in Normalize mode, the modifier had stopped affecting points and strokes outside the Vertex Group and that’s what I wanted it to do.

The fix

My change was to make the function scan over each point in the stroke. When it finds a point within the vertex group, it changes a variable called gps_has_affected_points from FALSE to TRUE. (gps stands for “Grease Pencil stroke”.) When it finds a point outside the vertex group, it sets another variable called gps_has_unaffected_points from FALSE to TRUE. Once both of these are set to TRUE, it stops scanning – neither of them can go back to being false.

What comes next depends on whether one or both of those variables are TRUE or FALSE.

If the stroke doesn’t have any unaffected points (
gps_has_affected_points is TRUE and
gps_has_unaffected_points remains FALSE), it’s safe to normalise the stroke by changing its the width and set all the points to 1.0. If it had both affected and unaffected points (both variables TRUE), originally I skipped handling it. (I tried to experimentally iterate a formula but then gave up and coded around it.) I left the code factored in such a way so that if need be, someone who knew the Grease Pencil system better than me could add the unhandled case in with the right maths. Something an expert coder could do in a second, probably.

I submitted patch D5483 at developer.blender.org at around 2:00am my time. Then I went to bed.

Getting the fix accepted

Now, submitting a patch doesn’t mean there was nothing for the coders to do, only that they had a code submission which fixes a bug. But was it readable code? Was it efficient code? Did it follow the appropriate formatting standards? Did the code change cover everything it needed to cover without introducing any regressions? For a module owner to accept code, it means they accept responsibility for what the code does and how it does it.

The next day, Grease Pencil module owner Antonio Vazquez had reviewed my patch. He thanked me for the code and encouraged me to handle the partially affected stroke case. I agreed it shouldn’t be left half done and did not make excuses that I was too tired to want to finish it all in one hit, and that it would be better for users if it didn’t just ignore certain cases (which were too hard for me to work out at two o’clock in the morning).

I spent a couple of extra hours on Friday reading other bits of Grease Pencil code to figure out the maths to convert stroke thickness to point pressure given a particular stroke thickness. After having no luck tracing through the code with Visual Studio for an answer, I did a brute force text search for where “thickness” and “pressure” occurred on the same line. It turned out to be simple algebra – target stroke thickness (from the modifier) divided by current stroke thickness results in the right pressure value.

Easy maths, right? Kind of.

Fun with numbers, C style

I knew enough about C to know it’s quite particular about the way it represents particular kinds of number. For instance, point pressure is a floating point (fractional) number and stroke thickness is an integer (whole number). If you divide two integers in C, you don’t get the remainder back. So if you want the remainder (which I did), you need to cast (convert) the integers to a floating point number instead. Then you can divide them to get a point pressure. (This converting may seem like a massive pain in the arse, and friendlier higher-level languages are happy to do those conversions under the hood, but the trade-off is that C runs at speeds which leave higher-level languages in the dust by comparison.)

I got the partial stroke normalisation case working on Friday evening and submitted an updated patch.

Antonio’s follow-up review on Saturday noted that Blender doesn’t do same-line comments, but that he was OK with the code of the patch. He added an extra line to clamp the width to a safe minimum – something an expert coder would know to do – and committed my updated patch to Blender’s master branch as a bugfix on Saturday.

My first Blender commit

The commit hash for my fix is a477c2e0e079 and I’m listed as the contributor. It’s not the first commit I’ve been named in (I helped fix a build bug a little while ago) but it’s the first commit where I’ve written actual code to fix something and been identified as a contributor. Yay!

Fixing that bug took me about six and a half hours, give or take. Much of that time was spent reading through unfamiliar code in a language I haven’t used before. And even if it was a small obscure bug, something very few people would notice or care about that much, it’s fun to see my name come up in Blender’s commit logs and nice to know I helped Blender work a little better than it did before.