mypaint/libmypaint

Backport what can be to "libmypaint-v1" branch

Jehan opened this issue · 31 comments

Jehan commented

This issue to track the task of backporting as much as possible from master to libmypaint-v1.

libmypaint-v1 is currently stuck at commit d842adc. From there, we should go in order until today by backporting any commit which improves libmypaint by not breaking API/ABI.

You probably all know, but a good comment to backport is: git cherry-pick -x (the -x option adds a small text in the git comment with the original commit hash from master. Very useful if needed for debugging and keeping history).

The point is that it will allow to release new libmypaint 1.x while still working on exciting new features on the master branch. Because right now it feels like libmypaint got stuck in time in December 2016 (no release since!) even though it is now used by several other projects.

Would it be absolutely terrible to just take the current 2.x branch and add a commit to make it 1.3 compliant? I'm pretty sure just updating the version and changing stroke_to() back to normal (remove viewzoom and viewrotation from the fuction). Then, just set viewzoom to 1.0 and viewrotation to 0.0 and it should behave exactly like 1.3. Those were just for calibration so should be fine to set them statically. I might be missing something, of course :-)

For better or worse, here's my attempt to backport all my smudge tweaks stuff from 2.0 to 1.3:
https://github.com/briend/libmypaint/tree/smudge_tweaks-v1
So the only things disabled are viewzoom, viewrotation, and the posterize brush mode.
I tested this with a very old version of mypaint gui from around the same time 1.3 was out.

Jehan commented

@briend Normally I would say it is better to cherry-pick only what is not changing the API/ABI to make sure we don't make mistakes. Yet considering the current situation and the fact that we would have more than a year of commits to backport (which is seriously a bit depressing if you think of everything needed!), your solution is probably more realistic. So I propose we rebase libmypaint-v1 branch over your branch.

If we can test thoroughly, it is ideal, then maybe we can make a new libmypaint v1 release. :-)

Jehan commented

(it is about time!)

Note my branch includes the WIP smudge tweaks but should work for testing. I can prepare a "v1" branch based on the current upstream/master as well (identical minus the posterize stuff). That would be better for official GIMP, I think. @americogobbo was interested in trying the smudge stuff in GIMP so I started with that. I'm trying to build GIMP to actually test this but looks like I need at least Debian Testing to have all the GTK requirements. I think I can try this is a chrooted env maybe.

Oh, strange thing though, when configuring GIMP it wouldn't find libmypaint 1.3, but if I changed the libmypaint version to 1.4 (via configure.ac) it seemed to find it (still failed a bunch of other dependencies of course). So I'm not sure what's going on there.

Jehan commented

I can prepare a "v1" branch based on the current upstream/master as well (identical minus the posterize stuff).

Awesome, do prepare it, give me the name and I will test.

Oh, strange thing though, when configuring GIMP it wouldn't find libmypaint 1.3, but if I changed the libmypaint version to 1.4 (via configure.ac) it seemed to find it (still failed a bunch of other dependencies of course). So I'm not sure what's going on there.

Ok. I will have a look and fix the configure if needs be. :-)

well my patch didn't work. But, I have a good build environment now so I can whittle away at this more easily. Elle's instructions worked well with some tweaks for a chrooted env

Jehan commented

@briend Any news since your last comment? :-)

Some people are starting to worry about libmypaint v1 not being buildable soon without patching (for instance because default autotools are getting updated, cf. #126, etc.), issues which are fixed on master, but not on a v1 branch.

In any case, it would be cool to be able to soon release a new stable libmypaint v1. :-)

My, time flies doesn't it? I've been very busy with the spectral mixing stuff, but I hope soon I can dive back into this. I have to wonder if maybe just backporting certain things like the autotools issues might be a more successful approach in the shorter term. Just to keep it building successfully, basically.

Jehan commented

Ok then I guess we are back to the original idea, except that we may not be as exhaustive as we hoped for.

Basically the branch already exists: libmypaint-v1
We should try to backport bug fixes, improvements and even features (when not breaking the API) one by one, checking we don't break the build.

I recommend we backport with the command git cherry-pick -x <commit numbers>. The -x adds a reference to the master commit (hence linking commits more easily).

I wonder... can this be automated? I'm fairly good with shell scripting. . . maybe we can build a loop where each commit is cherry picked, then gimp is built and somehow tested (maybe if it builds that is enough?) and automatically discard bad commits and continue with the next. . . . of course... that implies no merge conflicts. sigh...

Jehan commented

It's not impossible of course. Though doing it KISS may miss some API changes. Suppose you have the same number of parameters but different types. Then it may build (and unless we also track the number of warnings, we may miss the change). Worse case: same number of parameters and same types (but actually different meanings).
Now I don't know if there was such changes in libmypaint. You'd know better than me (just stating what could fail).

Also there is the problem of binary compatibility (ABI) only. For this, as I understand, we should not rebuild GIMP (this would work anyway as the API remained). At the opposite, GIMP should still run fine by loading the updated libmypaint (that's how I understand binary compatibility to work).

And last changes which a script would miss: same apparent API and ABI but the behavior changes! I think you had some of it in your recent changes, no?
To detect such changes, I guess compiling GIMP is unnecessary. You'd need unit tests comparing results maybe. Like you simulate some input (pressure, tilt, speed, whatever), stroke with your brushes, then compare the pixel results before and after the change. Considering no randomization, we should get the same result.

If you can track all these, then it's feasible, but I fear it's more than just a simple script.
Of course you may also do your script only to handle the simple case, and go through the resulting commit list afterwards by hand to remove the less obvious ones as well.

Jehan commented

This being said, I see only 55 commits (removing merge commits which are semantically meaningless) between libmypaint-v1 and master branches:

$ git log origin/libmypaint-v1..origin/master --oneline --no-merges  |wc -l
55

Even though boring, I feel that just going through these 55 commits by hand would just beat any scripting. :-)

Jehan commented

@briend:
I have started to cherry-pick commits and was wondering about commit 9e53545: if some software were using any of these mypaint_mapping_*() API and in particular were setting this n value, does your commit change the behavior?

For instance, say a software set mypaint_mapping_set_n (mapping, 8); should they now update to mypaint_mapping_set_n (mapping, 64);? Or would whatever this does would still behave the same.

If the former, we probably don't want to cherry-pick this one, otherwise we would.

Jehan commented

@briend: about commit 86c7208 and 2681060, these adds properties for new brushes, but don't change existing properties, right? So these can be cherry-picked, I assume?

Would this added feature break something in software which were using libmypaint without updating their code after the feature is added?

Jehan commented

Ok so I have cherry-picked a bunch of commits already in libmypaint-v1 branch. About 30 commits left (and I want also your input on the 3 commits I name above).

Hopefully I didn't make any mistake with the choice, but so far at least, GIMP still runs fine (without even rebuilding GIMP, so it's API and ABI compatible).

Wow thanks Jehan! The two brush inputs should be pretty benign. The mapping from 8 to 64, I'm not sure. I can't remember if I had to patch the gui because of error or simply to unlock the editor to allow 64 points.

I can rebase and remove that patch from the gui and see, I think that's a reasonable check. But then again I'm obviously not sure on a lot of things!

Jehan commented

@briend I had started to cherry-pick more commits, starting with the 2 new brush inputs. But then there are further commits which also modify these commits mentionning viewrotation (which was an input we didn't want to cherry-pick since it modified the API), and even changes the computation of the direction input, based on the computation of viewrotation (see commit 6505237).

And the more I went up the list, the more I had to resolve merge conflicts and wondering whether this or that change wasn't going to break some existing usage. Basically I just don't know enough about the internal of libmypaint and I think you should finish the cherry-picking, if you don't mind.

I think the one rule is: if someone was using libmypaint 1.3 on a software, one should be able to update to libmypaint 0.4, without rebuilding the program (hence ABI-compatibility), and the behavior would still be good even if the developer doesn't update anything in one's code.
New features and new API are ok (and of course, a developer may have to update the code to take these into accounts) but not behavior change (well unless old behavior was a bug!). :-)

So if that's good to you, I'll let you to it? ;-)

I was worried about this. . . :-D. We could either strip all the viewzoom/viewrotation stuff (kind of a pain). . . OR, statically assign them to 1.0 and 0.0 respectively. That would effectively nullify the setting without changing much code. Do you think it's ok to leave cruft in there? Well, it might end up a part of v2.0 so I guess it's not really cruft :-P

Jehan commented

Do you think it's ok to leave cruft in there?

As long as it's not wrong and making development difficult, I think it's ok.

On GIMP, we are trying to keep the gimp-2-10 and master branch as close as possible, backporting as much as possible. The more branches diverge, the more it becomes difficult to backport further commits on the same pieces of code.

Of course, this is specific to us, because of our new release policy as we are now trying to backport as many cool features as possible to the stable branch. Some other projects just want the "stable" branch to stay as still as possible, hence only backport what really has to be (i.e. bug fixes only). This was our former policy in 2.8.
So in the end, it all depends on how you want the libmypaint-v1 branch to be. :-)

But if you want it moving and getting new features, I guess leaving cruft, as long as it doesn't make the code messy, is not a big issue.

just about caught up I think. I stripped the viewzoom/viewrotation stuff instead of trying to leave it. I'm not sure I haven't screwed up something, but it compiles! I'll try to test GIMP against it soon

Jehan commented

Nice!

Maybe once it all looks good and stable, it might be time for a release. :-)

Jehan commented

@briend Any news? A new libmypaint v1 is getting really needed. Some people seem to get annoyed and scared that libmypaint would end up deprecated on various distributions (some already have to patch the build files or play with environment variable to keep libmypaint buildable).

Cf. https://gitlab.gnome.org/GNOME/gimp/issues/2202

I got really frustrated trying to compile git gimp dependencies, and got distracted with other stuff. Do you have an easy way to check it? I know Elle's instructions don't seem to work anymore and she was having problems too. I'm looking for a better build guide, recommendations welcome! I'm using debian unstable if that helps. I'm going to take another stab at it.

Oh cool I think I got it working. Turns out just not following directions works better. I'm testing now to see if I can actually use some of the new features :-)
Or maybe not just realized I had the debian package libmypaint 1.3 installed so I think gimp was using that :-P

image

Sweet, it works! I even got my brushes mostly working. I had to massage them to remove a bunch of bogus inputs to avoid crashing (libmypaint bug ;-), but it worked!

Next I'd like to finalize the smudge tweak I've made and backport those. I think the only thing that would need to be stripped from that would be the posterize brush mode. Not sure. It's a brush mode and not just a setting so I think it changed the ABI

Jehan commented

Cool! So we can soon expect a libmypaint 1.4 (or i it 1.3.1? 1.3.2? Not sure what's the versionning scheme)! :-)

Jehan commented

@briend By the way, unless you want to test last development code, you don't have to compile GIMP. Actually GIMP 2.10 compiled to work with libmypaint 1.3 (from a distribution package for instance) should still work when replacing with updated libmypaint shared library, without rebuilding.

This is actually even a good way to make sure that your changes are ABI-compatible (which is harder to see than API-compatible changes). A very dirty (don't tell anyone I told you this! ;p) trick could be to install GIMP 2.10 from a debian package then force-remove the libmypaint from package without removing GIMP (according to the web, that should be done with dpkg -r --force-depends libmypaint).

Of course, at this point, GIMP will be broken and won't run, but this is to make sure that it can only use the self-compiled libmypaint later when you will make tests.

Then install only libmypaint from source (from the dev branch libmypaint-v1) to $PREFIX, and set up the environment variable LD_LIBRARY_PATH:

export LD_LIBRARY_PATH=$PREFIX/lib64:$PREFIX/lib:$LD_LIBRARY_PATH

This way, not only you don't have to build GIMP, but you can even make sure that your changes are ABI compatible.

Of course building GIMP is good too if ever you wanted to contribute code though (we would be happy! For instance we still can't create MyPaint brushes within GIMP! :P).

Good tips! I forgot about just leaving gimp 2.10 installed (normally I run debian stable so I didn't realize 2.10 was in my debian unstable vm). I think libmypaint will be 1.4 in line with semantic versioning. It'd be great to have a brush editor. . . hmm maybe as a python module?

Jehan commented

It'd be great to have a brush editor. . . hmm maybe as a python module?

Why not! Right now, it would have to get its own GUI window though, since plug-in in GIMP cannot (yet! I want to make it possible some day) create new docks. Anyway this would definitely be possible in Python. :-)

Well, everything is actually backported now, though the 1.5.0 release lives in the libmypaint-v1.5.x branch, to not confuse the history too much (libmypaint-v1.5.x does not build upon the libmypaint-v1 branch, but (mostly) directly on the master branch).