xournalpp
xournalpp copied to clipboard
[WIP] Add support for spline approximation of strokes
Hi every one, This PR fixes #2578. In the current state of things, any new stroke is approximated by a spline and stored as such. The eraser does not yet fully work (well, apparently it does, but under the hood, it falls back to strokes as a bunch of points), and splines cannot be saved as such yet.
As you write and strokes are created, you will get interesting statistics (e.i. nb of points vs nb of spline segments) in the terminal.
There is yet a lot of things to do:
- [x] Adapt the eraser tool
- [ ] Add spline to the file format. Add a "Save without splines" option for backward compatibility.
- [x] Refactor
Spline
,PartialSplineSegment
andSplineSegment
for optimization (e.g.Spline = std::vector<Point>
and the segments are obtained by recasting) - [x] Adapt ~~
Stroke::rotate
Stroke::scale
Stroke::calcSize
Stroke::isInSelection
~~Stroke::intersects
(with gap) - [ ] Create classes
StrokeWithPressure
andStrokeWithoutPressure
to avoid testing at every iteration and reduce the memory footprint of the latter. - [x] Fix pressure variations (see this comment)
- ~~[ ] Add cusp detection (to get a smaller number of spline segments, see for instance the article referenced here)~~
- [x] Benchmark all that
- [x] Fix Load/Save mechanism
- [ ] Adapt SplineHandler
- [ ] Adapt StrokeRecognizer
- [x] Return splines or PL path
- [ ] Recognize from a spline
- [x] Live spline approximation like @redweasel suggested below
- [x] Remove flicker after stroke completion
- [x] Fix: Single-click stroke mishandled (edit: fixed for standard strokes. Remains dashed/filled)
- [x] Fix: Filled strokes don't work
- ~~[ ] Implement binary tree search based on chord length instead of trying to fit for every new point~~
- [x] Fix dashed strokes
- [x] Fix graphical glitch when using Highlighter and Live approximation
- [x] Fix high zoom lag on MacOs (Catalina & BigSur). See this post and this post.
- [x] Fix high zoom lag on Linux and Windows
- [x] Implement mode selection (no spline, after the fact approximation, live approximation)
- [x] Fix changing stroke size (after selecting a stroke)
- [x] Fix MacOs BigSur SegFault's (see this post)
- [x] Fix eraser
Improbable case 2
bug (see this post) - [x] Add tests
- [x] Polynomial solver + benchmark
- [x] Path intersections with rectangles (repair once #3532 is merged and rebase is done)
- [x] Spline live approximation (how ?)
- [x] Spline post approximation (how ?)
- [x] Merge #3532 and rebase
You are of course more than welcome to give your opinion on or even tackle any of the above tasks, or to either add other tasks I forgot.
(Edit: updated todo's)
I made progress on the eraser tool. It now works fine when no pressure is involved. It partially fixes #997 (the highligher is displayed as it should while erasing). For strokes with pressure, they are displayed without pressure while erasing. The pressure is back once we stop erasing. See here (besides the poor quality and flickers of the gif)
@rolandlo: your long winded strokes issue should be fixed too. If you have a minute to try it out. [Note that fixing the pressure issues may reintroduce some lag later. Hopefully not]
@rolandlo: your long winded strokes issue should be fixed too. If you have a minute to try it out. [Note that fixing the pressure issues may reintroduce some lag later. Hopefully not]
The eraser works very nicely with (long winded) strokes that I draw with the version from this PR. If I take the long winded stroke that I drew with the current master there is some strange behavior when erasing: The eraser acts like it were pretty thick and after a short while the whole stroke disappears completely.
If I take the long winded stroke that I drew with the current master there is some strange behavior when erasing: The eraser acts like it were pretty thick and after a short while the whole stroke disappears completely.
Yes, that's expected at this stage. The new eraser only works (for now) with strokes that are splines, and stroke are not converted to splines on file load. I did not do anything in the save/load direction. This actually raises the interesting question of how to handle backward compatibility.
I would like to raise a concern here. I HATE if the stroke would slightly move every time I release the pen after a stroke! A lot of Software packages do this and i absolutly hate it!
On the other hand I LOVE bezier curves and would like to see more of them in xournalpp. The reduced memory consumption is also a bonus, that I don't wanna miss out on.
Here is what I think you can do: (correct me if I'm wrong) (Assuming your splines are of degree 3 always, so 2 controlpoints between the real points
- Approximate spline until a second real point is created
- Fix the first one in place and only approximate the stroke from there.
- Draw approximated spline segment instead of the original points
This would help increase performance on usual handwriting. But more importantly, the jitter only happens very close to the tip of the pen. I wonder how the error tolerance will look like with this approach. It might need to be set to a smaller value, depending on the implementation.
The question of backwards compatibility is a very critical one here as well! Since from my understanding the spline tool which is already in, is converting the spline into points and saving it like that, there is no way to save on the memory and have full backwards compatibility. If you fix yourself to only use 3. order bezier curves (2 control points) you could write the points and control points out in the usual point list for each stroke. Then loading it from the new version would understand to draw the bezier curves and loading it from an old version would draw a polyline, which might get close to the real shape, depending on the approximation error value. That's the best I came up with. Sorry if that wasn't helpful.
I HATE if the stroke would slightly move every time I release the pen after a stroke!
I don't like it either, but I don't know how to avoid it in a satisfactory manner. We kinda had this discussion in #2578. One possible idea (other than yours) is to differ the replacement of the raw stroke by the spline until the next full repaint (zoom in/out, scroll, etc), so that it becomes somewhat invisible to the user. I'm not sure I like this solution, but it's on the table, I think.
Here is what I think you can do: (correct me if I'm wrong) (Assuming your splines are of degree 3 always, so 2 controlpoints between the real points
- Approximate spline until a second real point is created
- Fix the first one in place and only approximate the stroke from there.
- Draw approximated spline segment instead of the original points
I don't really understand what you mean by "a second real point". When is an input move
event a real point? There seem to be "on the fly" approximation algorithms out there (e.g. a patented one that we mentionned in #2578), but we could not find an open source one. (Note that any on-the-fly approximation will be less efficient than an after-the-fact algorithm).
I could try to implement something along your lines (I think, tell me if this was not what you had in mind):
Apply a Schneider-like least squares method to fit a Bézier curve to the sample (a queue of move
events). As events are triggered and added to the sample, this curve will change. Whenever the best fitted curve is to far from the data points, store the last best fitted curve and flush all the corresponding events from the sample (i.e. all but the last in).
All in all, I think this would cost significantly more. This could be worth try, especially if we can have an efficient answer to the question: how to compute the distance between the Bézier curve and the piecewise-linear path (reterred to as the path in the following) drawn by the raw events? In Schneider's algorithm, it works as follows.
- Compute the parameter of each point in the path. The parameter is the cumulated length of all segments between the starting point and the current point, over the total length of the path. You get some
double t
between 0 and 1. Compute the point on the spline corresponding to that parametert
and compute the distance between the spline point and the path point. - If the maximum
maxDistance
of those distances is lower than your fixed error tolerancemaxError
, you're fine. IfmaxError < maxDistance < 4maxError
, then it may be worth reparametrizing. This means adapting the parametert
associated to each point of the path (and thus the corresponding point on the spline) by some Newton-Raphson method. Recompute the distance and try again. IfmaxDistance
is still bigger thanmaxError
after a couple (often 4) iteration of this process, then the Bézier curve is not considered a good fit.
The point is that the second step is crucial for Schneider's algorithm to give a small number of Bézier segments in the end. It comes with a cost though. Doing all that at every move
event seems to costful.
The question of backwards compatibility is a very critical one here as well! Since from my understanding the spline tool which is already in, is converting the spline into points and saving it like that, there is no way to save on the memory and have full backwards compatibility. If you fix yourself to only use 3. order bezier curves (2 control points) you could write the points and control points out in the usual point list for each stroke. Then loading it from the new version would understand to draw the bezier curves and loading it from an old version would draw a polyline, which might get close to the real shape, depending on the approximation error value. That's the best I came up with. Sorry if that wasn't helpful.
I don't think this can work. The spline can be fairly far from its control points and it would distord the handwrittings to much. For instance, a circle can be approximated (up to e-4) by a spline with 12 points. Drawing as you suggest would render an ugly octogon. In my mind, the files would not end up being backwards compatible: the splines would be stored as such. An option to "export to pre-1.1.1" format would be available (and of course a tool to convert). This probably means a 2-phased release: first we can read spline files and only save stroke as splines if the user asks for it, second writting as spline becomes the default. I don't really know what else can be done.
@bhennion I like how you propose to handle backwards compatibility (though it will potentially leave legacy code in the future codebase)
I'm not sure about the idea to just hide the change. I feel like that would cause some invalid bug reports... A similar solution would be to approximate on save. Again not any different really.
I can see you did get my point. Thanks for taking the time to explain some details. I have also looked a little at your code now, but there is a lot. I will not pretend to have understood everything at this late evening (it's impressive). One point you didn't mention in your comment, can you cache the last result and use newton method from there or is it part of the curve fitting and needs to be done often? That way only one newton iteration would be needed as it is done on every move event. You can also introduce a delay between the approximation calls, like 50ms or something like that and batch process very small batches. In the meantime the usual stroke points would be shown. After all I don't know how this would affect the approximation quality (the reduced file size goal) as Schneider's algorithm works on the entire path usually.
Anyway I would love to see an incremental construction instead of the jump at the end. I don't think I can be much of a help right now. Let me finish with this link to a shader of a signed distance function for bezier curves that I found https://www.shadertoy.com/view/4sKyzW (it is using newton iteration as far as I can tell)
Status update: the last commit removed Stroke::points
and adapted almost all geometric shape tools. The new and shiny eraser also works for piecewise-linear strokes (you can try it out on rectangles/arrows/lines from the dedicated tools) as well as for splines.
In effect, the pressure values are no longer used when drawing and save and load operations now result in crashes (hence the pipeline tests failing). It won't stay that way, obviously. I'll also update the todo-list in the first message above.
https://github.com/xournalpp/xournalpp/pull/2729/commits/f5ba9266636aee0942aaf5db9bec3ac083692176 seems to be an unrelated issue. Can you make a Pull Request for that one. It might fix some render issues.
Also about your error function. One alternative to the method you described, would be to calculate the (approximated) intersection points of the spline with the stroke, and then calculate the sum of the area in the enclosed regions as an error value. I think that might be faster, as you only need to sample the bezier curve once (avoiding too many samples on straight lines if possible), then find intersection points using sweep and prune (quite fast), then calulcate the areas.
f5ba926 seems to be an unrelated issue. Can you make a Pull Request for that one. It might fix some render issues.
Done: see #2846.
Also about your error function. One alternative to the method you described, would be to calculate the (approximated) intersection points of the spline with the stroke, and then calculate the sum of the area in the enclosed regions as an error value. I think that might be faster, as you only need to sample the bezier curve once (avoiding too many samples on straight lines if possible), then find intersection points using sweep and prune (quite fast), then calulcate the areas.
This idea crossed my mind too. To compute the intersections, I imagined something involving one Newton-Raphson per segments in the stroke. I don't know much about Sweep and Prune, but it seems to depend on a reliable time parameter (close to the parametrization issue we have with the other method). Am I wrong?
Another option is to sample the Bézier curve and compute the distance between each sampled spline point and the strokes segments. This seems quite efficient.
sweep and prune is just a good/fast scanline algorithm to quickly detect which line segments could intersect (broad phase collision detection). I can see problems with this approach though, when there is self-intersections or when there is geometry which does not have area like a line going back on itself, it could create wrong results and think it matched it perfectly.
Your last Idea is also pretty good, you just need a lot of evenly spaced samples (maybe up to twice the amount of the stroke? But you can go one at a time in a spacially efficient way and abort when one is outside the tolerance). But to compute the distance of the bezier points to the polyline efficiently, you need a fancy algorithm. Just brute force, checking every edge of the polyline is O(n^2). If you want to try this, go for the brute force way first and check if this approach is good in practice. A fancier algorithm for the distance check would be to do a broad phase with a very simple binary BVH tree with spheres or AABBs around the edges (3D spheres/AABBs since you need a 3D solution for pressure right?), You can use that BVH tree to get rid of a lot of checks, so you will get O(n log n) time for BVH construction and approx. O(log n) time per point distance check, giving you a total of O(n log n) for your error function calculation. If you want to go this route and need an explanation of the BVH stuff, I can give you a better one if you ask me. I have implemented such things already in the past. One advantage of the BVH Tree is also, since the stroke points never move, you can keep a lot of the structure if you do incremental construction.
But in that sense, using https://www.shadertoy.com/view/4sKyzW would not be a bad idea to since you can get your distance of the stroke points to the bezier curve in basically constant time, so the check would be O(n). Your current check is also O(n) AFAICT. (these are only valid if there is only one bezier segment, otherwise it would both be O(n*m) with m the number of bezier segments, I guess. Correct if I'm wrong on this one)
Your last Idea is also pretty good, you just need a lot of evenly spaced samples (maybe up to twice the amount of the stroke? But you can go one at a time in a spacially efficient way and abort when one is outside the tolerance). But to compute the distance of the bezier points to the polyline efficiently, you need a fancy algorithm. Just brute force, checking every edge of the polyline is O(n^2). If you want to try this, go for the brute force way first and check if this approach is good in practice. A fancier algorithm for the distance check would be to do a broad phase with a very simple binary BVH tree with spheres or AABBs around the edges (3D spheres/AABBs since you need a 3D solution for pressure right?), You can use that BVH tree to get rid of a lot of checks, so you will get O(n log n) time for BVH construction and approx. O(log n) time per point distance check, giving you a total of O(n log n) for your error function calculation. If you want to go this route and need an explanation of the BVH stuff, I can give you a better one if you ask me. I have implemented such things already in the past. One advantage of the BVH Tree is also, since the stroke points never move, you can keep a lot of the structure if you do incremental construction.
But in that sense, using https://www.shadertoy.com/view/4sKyzW would not be a bad idea to since you can get your distance of the stroke points to the bezier curve in basically constant time, so the check would be O(n). Your current check is also O(n) AFAICT. (these are only valid if there is only one bezier segment, otherwise it would both be O(n*m) with m the number of bezier segments, I guess. Correct if I'm wrong on this one)
Giving it more thougths, it is no longer clear that a distance function like this is enough for this feature (live spline-approximation near the stylus tip). Let me reflect on this a little bit.
Anyway, having an efficient distance function would still be useful elsewere (for detecting clicks on/near the stroke, for implementing a circular eraser, probably other things). More precisely, it would often be about finding the parameter (along the spline) corresponding to the point of spline closest to a given point. This is similar to the shadertoy @redweasel pointed out. IMO, this should be achieved with a good polynomial roots finding library (open source and cross platforms, ofc). To my surprise, I could not find any. Does anyone know of such a library?
Eigen is an open source library for matrix operations. It can do very efficient matrix/vector operations and also solve for eigenvalues. That way you can use the https://en.wikipedia.org/wiki/Companion_matrix to calculate the roots of a polynomial. Otherwise there is also https://eigen.tuxfamily.org/dox/unsupported/group__Polynomials__Module.html to solve polynomials. The library also contains optimizers and other useful things and is very popular as one can see here https://eigen.tuxfamily.org/index.php?title=Main_Page. Krita uses it as well.
Thanks! I had seen the unsupported Polynomials module of Eigen in my research. I didn't look under its hood, but I thought matrix based algorithm were more expensive: without clever optimisations, each iteration is in O(n²), for n the degree. With something like what you found here https://www.shadertoy.com/view/4sKyzW, the complexity is also O(n^2) (because we need to solve the derivatives to isolate the roots), but we can drop some roots we don't care about (i.e. not between 0 and 1) earlier in the process (once we know the roots of the derivative).
Of course, we will only need small degrees (say n <= 6), so those considerations may not be so relevant. I can't tell.
Edit: I guess we should benchmark those methods...
Hi everyone! I made some progress here. The eraser is now fully functional, pressure-sensitive strokes are back, Save/Load handlers are fixed (but do not yet save a spline properly, nor do they convert a loaded old-school stroke to a spline).
I must say I am quite happy about how smooth the eraser became, with somewhat simpler code!
I guess we should benchmark those methods...
Alright, I benchmarked two polynomial solvers. The first one is Eigen's (unsupported) PolynomialSolver
that relies on the QR-algorithm to compute the eigen-values of the companion matrix. The second one is handmade, based on Halley's method.
The stats are staggering... Here for finding the roots in (0,1) of 100,000 degree 6 polynomials
Eigen Loop: 99999
Found 44825 roots in 121586747 microseconds.
Handmade Loop: 99999
Found 44890 roots in 2200424 microseconds. Maximal error: 0.000001
I also compared the results when feeding them the same polynomial, ofc. They match.
The big difference here is that the matrix (Eigen) method computes every complex root, and then only keep the real ones between 0 and 1. The handmade one only ever computes real roots between 0 and 1. This makes the number of application of Halley's method drastically drop (to a handful per polynomial).
I'll commit and push the handmade solver once I've run some more test and tried some more possible optimisations.
Hey everyone! I finely took the time to improve the polynomial solver I cooked up. This lead to significant (~80%) performance improvement. This is now used to compute the distance between a point and a spline (for instance when clicking to select or play the audio associated to a stroke). It all seems to work pretty fine.
May I request that whether a spline (or another smoothing algorithm) is used or not be introduced as an option that can be toggled on and off per stroke? Or if doing that per stroke is cumbersome, at the very least keep the option to save without splines not just for backward compatibility, but keep it always as an option?
I tried all kinds of note-taking and drawing tools for my lectures, and the "feature" to smooth strokes that is present in practically all of them always ended up making those tools useless for me. One of the main reasons I ended up using (and loving!) xournalpp is precisely because it doesn't do any stroke smoothing. I need that for the physics classes I teach when drawing diagrams as then I need precision: I want the stroke to follow exactly the movements of the stylus (the discretization coming from the linear interpolation between the points was never an issue for me), and not veer off because of a smoothing algorithm. That's why I'd like a way to switch off splines (or any other stroke smoothing) if those end up in xournalpp.
Thank you for all your amazing work!
May I request that whether a spline (or another smoothing algorithm) is used or not be introduced as an option that can be toggled on and off per stroke? Or if doing that per stroke is cumbersome, at the very least keep the option to save without splines not just for backward compatibility, but keep it always as an option?
Having it as an option is certainly possible. Some strokes will anyway always be kept as piecewise linear (just a sequence of points linked by line segments). Typically, strokes created by the Rectangle tool or things like that. So piecewise linear stroke will not disappear. It then won't be hard to make spline approximation optional.
On a side note: I'm sorry I've been quite busy otherwise lately. I hope I can get back on it soon.
Hi everyone! So, I've rebased onto master and cleaned up a little the commit list to see things more clearly. I should have some time in the next month or so to work on this PR a little bit.
The biggest step yet to make is the adaptation of the file format. As @redweasel mentionned in #3021, it'd be best to minimize the number of file format changes. I don't know how realistic this is, but ideally, this would be merged at the same time as @LittleHuba's work on a file format (see also #1515). @LittleHuba, has there been any progress with this? (see also #2233)
Should this PR be put on hold, some parts of it may be retrofitted and merged independently. I am thinking about the erasure rework that improves the current one (see #2767 for a list of erasure related issues).
I just pushed a first attempt at a "live approximator", meaning that the splines are found on the fly (as opposed to after the stroke is finished).
@redweasel it should correspond to the suggestion you made above. Does it feel like you expected ?
If you look at your terminal, you'll see that: it actually gives better splines (i.e. less segments per stroke), which is good, but it costs significantly more (~4x more cpu time than the simple implementation of Schneider's algorithm).
You can also see that there are some flickers and lags (especially when making a long stroke). Most of this is due to the dirtiness of the patch and can most likely be avoided.
Still, there will most likely remain severe lags on lower end devices with such an algorithm. I'll try to improve this, but I'm running low on ideas here. I'll describe the whole algorithm in another post in case someone has an idea.
Agreed, it's currently too laggy when using the stylus. With the mouse (where less input samples are generated) it feels OK (on a reasonably fast laptop, on Linux). I can try the previous commit on MacOS Catalina, where performance is worse in general, but still acceptable (without spline approximation).
By the way, when the fill
option is on, the app crashes as soon as releasing the pen.
Error: signal 11
[bt]: (0) build/src/xournalpp(+0x22f2eb) [0x559c103ee2eb]
[bt]: (1) /lib/x86_64-linux-gnu/libc.so.6(+0x41040) [0x7f4dbe0b7040]
[bt]: (2) build/src/xournalpp(_ZNK5Point14relativeLineToERKS_d+0x38) [0x559c103b1288]
[bt]: (3) build/src/xournalpp(_ZN6Spline14addLineSegmentERK5Point+0x49) [0x559c103b2e49]
[bt]: (4) build/src/xournalpp(_ZN6Spline16LiveApproximator8finalizeEv+0x195) [0x559c103b7075]
[bt]: (5) build/src/xournalpp(_ZN13StrokeHandler20onButtonReleaseEventERK17PositionInputData+0x25f) [0x559c1031195f]
[bt]: (6) build/src/xournalpp(_ZN11XojPageView20onButtonReleaseEventERK17PositionInputData+0x4c) [0x559c1033fbfc]
[bt]: (7) build/src/xournalpp(_ZN15PenInputHandler9actionEndERK10InputEvent+0xd0) [0x559c10379640]
[bt]: (8) build/src/xournalpp(_ZN17MouseInputHandler10handleImplERK10InputEvent+0x8e) [0x559c103786ce]
[bt]: (9) build/src/xournalpp(_ZN12InputContext6handleEP9_GdkEvent+0x4ac) [0x559c1037781c]
[bt]: (10) /lib/x86_64-linux-gnu/libgtk-3.so.0(+0x3e94a8) [0x7f4dbf2584a8]
I have tested your PR without live approximation (commit 9377348648d96c3600eb2a8fcad86d5f304c62ce) on MacOS Catalina (on a 7 year old MacBook Pro). To my big surprise while drawing (before releasing the pen and triggering the spline approximation) the experience was much more fluid than on current master. Have you found a way to avoid full redraws on MacOS while drawing?
The difference can be measured by the number of input points that are collected while drawing a screen-big circle in normal speed (about 1 sec) with my Wacom Intuos Pro M tablet.
- With your PR I get around 130 input points (and the shape doesn't look edgy),
- On current master I only get around 30 input points (and the shape looks roughly like a polygon with about 15 edges).
On Linux (on a faster laptop)I don't see this difference (probably because there are no full redraws). I get around 250-300 input points.
~I will test now on BigSur, where the lag on current master is even bigger (about 12 input points for a circle).~
On MacOS BigSur the performance is also improved. Still with toolbar on the lag is pretty big, but without toolbar I get around 100 input points for a big circle (and drawing feels smooth). After releasing the pen, there is some blinking. In any case, you must have got some real performance improvements independent when neglecting the spline approximation part.
Edit: Added results on BigSur
Have you found a way to avoid full redraws on MacOS while drawing?
If so, that's an accident (I don't have any Mac). The lines most likely to have changed something like that are in StrokeHandler.cpp
. I'll make a review of the file highlighting what I think you could try out directly on master.
The changes you highlighted applied to current master didn't have a noticeable impact on performance on MacOS Catalina. There must be something else that makes your code more performant.
I improved quite a bit the live approximator. It is now fluid on my laptop (but it's quite high end). I quite like the feeling of it now!
In terms of statistic, it gives an average of 1 spline segment (~3 points) for 25 points with this laptop's pen. It means something like a 88% in data size in the end!
I have another lead to yet optimize the live approximator, but it'd be based on how many points are merged into on spline segment on average. This of course depends on the device (from 5 on my touchpad to 25 on my pen), so I need to think about how to infer this. If you have an idea, shoot!
Edit: corrected the statistics I had miscalculated
SplineLiveApproximator2.h
is missing. It doesn't compile.
SplineLiveApproximator2.h
is missing. It doesn't compile.
Yes of course I forgot to add the new files... Repushed an amended commit. Sorry about that!
It's indeed amazingly fluent on Linux! Well done! :muscle: Let me try on MacOS.
By the way, the eraser doesn't work on horizontal and vertical strokes, it seems.
By the way, the eraser doesn't work on horizontal and vertical strokes, it seems.
Could you describe the bug more precisely? I can't reproduce. For instance when I draw a rectangle, I can erase anything I want.
There is another bug though: when doing a single-click stroke. This case needs to be handles separatly.
When doing a single click, a horizontal stroke is generated (at least that's how it appears on the screen) and these strokes can't be deleted, neither with the standard eraser mode nor with the delete stroke eraser mode.
On MacOS the performance was also very fluid. I got a (seemingly random) crash (segfault) in Document::drawElement
once when drawing for a long time (although not particularly long strokes).
One thing that could possibly be improved is the blinking when the stroke is finished. While drawing that doesn't happen, just at the end when releasing the pen. I can observe it on Linux as well.