Upgrading from 1.4 to 1.5, results of `minimal.c` changed.

[define-private-public]

I'm working on an application that will use libmypaint in the future. The minimal.c program (written for 1.4) would product a red box:

do_stroke(surface, brush, 0, 0);
do_stroke(surface, brush, 200, 0);
do_stroke(surface, brush, 200, 200);
do_stroke(surface, brush, 0, 200);
do_stroke(surface, brush, 0, 0);

But when updating to 1.5, the same minimal.c code (for 1.4) was producing this result:

If I add an extra call right after the do_stroke(..., 0, 0);, e.g: do_stroke(..., 1, 0);, then it produces the fully sided red box like before in 1.4. I saw that the code for minimal.c changed in 1.5, adding an extra stroke call (https://github.com/mypaint/libmypaint/commit/f0f2c6f998d771789b0b65bed128e6e01d9b1314#diff-80dca24354c217c5b04ecc00a3e927b1L25)

Is there a reason why we have to do this now? While I wouldn't call this a breaking change, it is an adjustment in how the API is used.

[jplloyd]

Right, it requires an initial call to stroke_to to set up the state now...

This might actually be a breaking change, depending on how the stroke_to calls are used. For MyPaint and GIMP it's not really visible, since the time slices involved are so small, but if a program like enve assumes that the first stroke_to works the same, we'll have to fix it.

The change was introduced in this commit: 096acdacbdfdaac3cbaf60b7697aea8d1f2cd538 (load mypaint-brush.c diff, search for "res1") where the basis for the interpolation was changed to use state values instead of base values for the denominators - meaning that the first call will (usually) produce nan's, since the state values aren't set.

Just to be clear, this oversight is on me. The consequences of that change didn't really register.

Imo, instead of checking for nan's in the result, we should either:

1. Check if the state value is nan and use the base value instead for those cases.
2. Revert to just using the base value (possibly less "correct", but consistent with 1.3 / 1.4).
3. Factor out the calculation and use base values when called from stroke_to and base/state values when called from stroke_to2.

@briend Do you have any thoughts on this?

---

Edit: I have tried option 3), and it resolves this without any measurable performance degradation.

[briend]

Oops, that was me huh. Sorry! Hmm, I did this because the dabs_per_xxx settings were constant, and it can be useful to adjust those on the fly w/ other inputs. I agree the checking for NaN is kind of horrible. So what's happening is the very first stroke is getting a dabs_per_xxx settings of zero, then? Argh.

In option 3), do you mean the first call to stroke_to uses the base values, and then thereafter they use the state values?

I'm also wondering. . . maybe we should initialize the state with base values beforehand?

[jplloyd]

Oops, that was me huh. Sorry!

I think your change makes the calculation more intuitive than the way it used to be (except for the NaN part), I just missed that it was a change to the semantics when I made the 1.5.0 backport.

In option 3), do you mean the first call to stroke_to uses the base values, and then thereafter they use the state values?

Yes, specifically if count_dabs_to is called via mypaint_brush_stroke_to_2 (w. the viewrotation etc), whereas if it is called via mypaint_brush_stroke_to (the old interface) it uses the old equations.

Concrete proposal: https://gist.github.com/jplloyd/471083fed66f86cd152a00648626fee7 Note that this is not about the code in master, but in libmypaint-v1.5.x.

I'm also wondering. . . maybe we should initialize the state with base values beforehand?

If it doesn't mess with anything else, that would be much nicer than the solution above!

[jplloyd]

Fixed in v1.5.1, specifically bc3a5474069e05f7af5e7c8adc19562dcdbbfd0c

minimal.c updated accordingly.

[define-private-public]

confirming that the changes now produced the red square as expected. Thanks!