Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mypaint-mapping: redundant interpolation #99

Closed
briend opened this issue Jul 16, 2017 · 3 comments · Fixed by #102
Closed

mypaint-mapping: redundant interpolation #99

briend opened this issue Jul 16, 2017 · 3 comments · Fixed by #102
Labels

Comments

@briend
Copy link
Contributor

briend commented Jul 16, 2017

Testing the Gridmap settings/inputs might have uncovered a couple bugs. If I set Opacity to use a gridmap input and then, using the liveupdate feature, drag the GridMap Scale setting left and right, eventually mypaint crashes:

python: tilemap.c:62: tile_map_get: Assertion `offset < 2self->size2*self->size' failed.
Aborted

Here's where it gets weird. Even if I click the eraser for the GridMap X, and GridMap Y on the Opacity setting, Mypaint will continue to crash. I have to hand-edit the brush file and actually remove these input lines even though they shouldn't be doing anything (after all I clicked the eraser and all). So this might be a bug in MyPaint gui (I'll make a separate issue for that) that leaves these dangling definitions:

            "gridmap_x": [
                [
                    0.0, 
                    0.0
                ], 
                [
                    256.0, 
                    0.0
                ]
            ], 

So I decided to look at the https://github.com/mypaint/libmypaint/blob/master/mypaint-mapping.c#L177 and noticed it seems to try to skip interpolation if the x0 and x1 values are the same, but it doesn't check y0 and y1. In the above example the x values are different (unless I"m really reading this all wrong), so it tries to do the interpolation anyway:

    if (x0 == x1) {
      y = y0;
    } else {
      // linear interpolation
      y = (y1*(x - x0) + y0*(x1 - x)) / (x1 - x0);
    }

I made a branch here that just adds an extra check for y0 and y1:

if (x0 == x1 || y0 == y1) {

https://github.com/briend/libmypaint/tree/mappingFix

This actually seems to fix reduces the crashing issue I was having with GridMap, but I really don't know the root cause of the assertion error. I've only ever seen it crash when doing LiveUpdate in the brush editor, not when actually drawing. Can someone validate the logic change? I'm really not quite sure what's going on.

achadwick added a commit to mypaint/mypaint that referenced this issue Jul 16, 2017
When the brush dynamics are reset for a given input in the editor UI,
the graph was reset to some spurious zero entries. This was the cause
of some annoying knock-on effects in the brush engine, and since the
meaning of "reset" is kinda obvious, we should probably do the least
surprising thing.

Note that setting the dynamic's ±y slider to *exactly* zero does the
same graph reset (but see #826 for a discussion of
whether that's a good idea or not)

Closes #840.
Addresses mypaint/libmypaint#99 and mypaint/libmypaint#100. Sorta.
@briend
Copy link
Contributor Author

briend commented Jul 17, 2017

For a square-wave type of input mapping (or anything with "flat" areas) this patch might improve performance a bit. There is no reason, for instance, to interpolate the data between points like (0.5, 125), (1.0, 125). The result is going to be 125 no matter what, right? So adding the y0 = y1 test seems to be a good idea.

@briend
Copy link
Contributor Author

briend commented Jul 26, 2017

Ok I feel a bit better that this isn't necessary a problem with the GridMap code, although GridMap seems to amplify the problem.

Tested on a fresh config of plain master (libmypaint and mypaint) and was able to reproduce this memory issue (to a lesser extent) with just a plain deevad 2b pencil and jitter assigned to random input.
Steps to reproduce:
0. run mypaint -c /tmp/temporary_profile
1.edit devaad 2b brush and assign jitter setting a random input all the way to +25 (default slope)
2. scribble around and observe memory usage. It should be pretty low and static
3. enable live update and begin dragging the random input left and right and observer the live changes on the canvas. You may see memory usage creep up a bit, but dragging the slider skips a lot of values.
4. using the arrow keys, move the slider continuously through all the values -25 thru +25 on the random input
5. Observer memory usage continuously grow beyond 1GB

The memory does not seem to be released until you quit mypaint. Also, after the memory growth it seems like mypaint peformance lags considerably with complex brushes for a few minutes and then speeds up again, although the memory consumption remains high. Changing the brush can also make the performance go back to normal.

screenshot from 2017-07-25 22-48-26

@briend
Copy link
Contributor Author

briend commented Jul 26, 2017

Verified this behavior with Windows 10 build 1.3.0-alpha+git.f03e5ec5
I'm actually now convinced this is a MyPaint issue, not a libmypaint issue. Actually I have no idea still. My reasoning is that it only affects Live Update. Even if you attach the same setting to "Stroke" you won't see the memory issue. I would think a stroke input w/ a long stroke length would be very similar to moving the sliders through all the values, but no, Live Update behaves totally differently than an actual brush

@briend briend changed the title mypaint-mapping: possible mapping interpolation issue and Live Update mypaint-mapping: redundant interpolation Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

1 participant