#5346 Fix GC issue in skyline rotations

[David Kastrup]

Fix GC issue in skyline rotations

This was issue 5341 initially and is a fix for a problem introduced
during issue 5319. The problem basically is that a rotated Stencil is
created and its stencil expression is getting stored on the heap
without retaining any pointer to it in areas scanned during garbage
collections.

This fix is a minimal one that merely avoids creating a stencil and
instead simulates the effect on the transform matrix to be expected
from using a rotated stencil.

http://codereview.appspot.com/344960043

[Anonymous]

Passes make, make check and a full make doc.

[Anonymous]

Patch on countdown for June 19th.

[Torsten Hämmerle]

Sorry for reporting so late, but there's a problem with the rotation centre, see attached image:

[David Kastrup]

Could you please keep tests like that not private but add them to the regtests? Particularly when you are working on that area? This could have been caught by either me or James then. I doubt I'll be able to fix this before tomorrow as I have to leave for a family celebration soonish.

[Torsten Hämmerle]

This wasn't private at all since I actually had added this very example to the regtests:
input/regression/skyline-grob-rotation.ly
It should have shown up in the standard regtest procedures, I'll check this.

By the way: grob rotation (the printed stencil) uses rotate_degrees, i.e. the offset co-ordinates are "measured in terms of the extent of the stencil, so -1 = LEFT/DOWN edge, 1 = RIGHT/UP edge." (cited from a comment in the coding).

This was the reason that drove me into rotating a copy of the stencil using rotate_degrees as an easy way to get the exact same behaviour for skylines and the printed stencil later on. Unfortunately, I was totally ignorant of garbage collection.

[David Kastrup]

It should have shown up in the standard regtest procedures, I'll check this.

Sorry for fuming off here: I searched for the file name of your regtest image and you named it different from the test so I thought it wasn't there and I had no time looking for it. I think the problem is that our regtests prefilter on differences in bounding boxes of grobs and those are entirely unimpressed by skylines. Maybe it would help to stack several systems into one markup so that the system skyline distance has some effect on some grob.

I just uploaded a new patch: I'll check it with this regtest.

[Torsten Hämmerle]

No problem with "fuming off", but what bothers me a bit is that these positioning differences actually do not show up in the standard regtests. I didn't expect that at all.

But when using the png comparison regtests (make-regtest-png.sh), the differences will become visible - even the slight shift in the much older stencil-color-rotation.
Please find a PNG file containing all six (old, new, diff) as an attachment.

[David Kastrup]

but what bothers me a bit is that these positioning differences actually do not show up in the standard regtests. I didn't expect that at all.

Well, me neither. Hence the fume. I needed hitting in the face with it to even start looking for an explanation. It's likely a potential problem with all skyline regtests, not just rotation.

[Anonymous]

Patch counted down - please push.

[David Kastrup]

Uh, well. Pushed a few days ago (and subsequently fixed) as
commit fdab8053c4fbfadf38867f7d14769d227ce26749
Author: David Kastrup dak@gnu.org
Date: Fri Jun 15 17:27:30 2018 +0200

Issue 5346: Fix GC issue in skyline rotations

This was issue 5341 initially and is a fix for a problem introduced
during issue 5319.  The problem basically is that a rotated Stencil is
created and its stencil expression is getting stored on the heap
without retaining any pointer to it in areas scanned during garbage
collections.

This fix is a minimal one that merely avoids creating a stencil and
instead simulates the effect on the transform matrix to be expected
from using a rotated stencil.