Asymmetric MusicText rotation about incorrect(?) transform_origin

[Xavman42]

I'm working on a way to visualize the bounding rectangles for MusicText objects, and it's highlighted a weird phenomenon I noticed in the past when rotating these objects. What I'm trying to do is have a MusicText and a corresponding bounding box rectangle rotate about its center. It's easy to make a rectangle the same size as the bounding box and I can rotate the rectangle how I want, but the MusicText isn't quite rotating how I'd expect. Symmetric MusicTexts rotate just fine, but asymmetric MusicTexts seem to rotate about a transform_origin other than the one I specify. I can't quite tell if this is a bug or I'm misunderstanding something. The desired result is that my bounding box visualization is always correctly bounding the MusicText regardless of rotation angle. Whole notes work just fine, but treble clefs are particularly bad. Maybe there's transparent space in the SMuFL font I'm not accounting for?

Here's the code to make the above:

from neoscore.core import neoscore
from neoscore.core.brush import Brush
from neoscore.core.music_font import MusicFont
from neoscore.core.music_text import MusicText
from neoscore.core.path import Path
from neoscore.core.text_alignment import AlignmentY, AlignmentX
from neoscore.core.units import Unit, Mm

def rotate_and_box(smufl_text, font_size, y_position):
    font = MusicFont("Bravura", Mm(font_size))
    for i in range(9):
        text = MusicText((Unit(i * 80), Unit(y_position)), None, smufl_text, font)
        text.alignment_x = AlignmentX.CENTER
        text.alignment_y = AlignmentY.CENTER
        text.transform_origin = Unit(text.bounding_rect.width / 2), Unit(0)
        text.rotation = i * (360 / 8)
        rectangle = Path.rect((Unit(text.x - text.bounding_rect.width / 2),
                               Unit(text.y - text.bounding_rect.height / 2)), None,
                              Unit(text.bounding_rect.width),
                              Unit(text.bounding_rect.height), brush=Brush.no_brush())
        rectangle.transform_origin = Unit(text.bounding_rect.width / 2), Unit(text.bounding_rect.height / 2)
        rectangle.rotation = i * (360 / 8)


if __name__ == "__main__":
    neoscore.setup()
    rotate_and_box("noteheadWhole", 10, 0)
    rotate_and_box("accidentalSharp", 8, 100)
    rotate_and_box("accidentalParensLeft", 10, 200)
    rotate_and_box("restQuarter", 9, 300)
    rotate_and_box("gClef", 4, 400)
    rotate_and_box("rest128th", 5, 500)
    rotate_and_box("pluckedSnapPizzicatoAbove", 8, 600)
    neoscore.show()

[ajyoon]

Hey @Xavman42, thanks for reporting this issue in such detail, it made looking into this much easier! After messing around with this for a little bit, I think there are a couple things going on here.

First, I believe you've found a real bug. It seems like transform_origin in Text objects failed to account for text alignment offsets. I've opened a PR that I believe fixes the issue at #110; want to take a look?

Second, I think your original code failed to account for the fact that bounding rects do not necessarily start at (0, 0) - in fact, they rarely do for text objects.

Running on the branch at #110 and adjusting the code to account for this difference, I get results that make sense to me:

from neoscore.core import neoscore
from neoscore.core.brush import Brush
from neoscore.core.music_font import MusicFont
from neoscore.core.music_text import MusicText
from neoscore.core.path import Path
from neoscore.core.text_alignment import AlignmentY, AlignmentX
from neoscore.core.units import Unit, Mm, ZERO
from neoscore.core.point import Point


def rotate_and_box(smufl_text, font_size, y_position):
    font = MusicFont("Bravura", Mm(font_size))
    for i in range(9):
        pos = Point(Unit(i * 80), Unit(y_position))
        text = MusicText(pos, None, smufl_text, font)
        text.alignment_x = AlignmentX.CENTER
        text.alignment_y = AlignmentY.CENTER
        brect = text.bounding_rect
        # Place transform origin at the right edge of the bounding rect
        # and at y=0 relative to pos
        text.transform_origin = brect.width / 2, ZERO
        text.rotation = i * (360 / 8)
        # Draw green dot at specified text pos
        Path.ellipse(
            pos, None, Mm(1), Mm(1), "#0f0", "#0000"
        )
        # Draw red dot at transform origin
        Path.ellipse(
            pos + text.transform_origin, None, Mm(1), Mm(1), "#f00", "#0000"
        )
        rectangle = Path.rect(
            pos + Point(brect.x, brect.y),
            None,
            brect.width,
            brect.height,
            brush=Brush.no_brush(),
        )
        rectangle.transform_origin = (
            brect.width,
            brect.height / 2,
        )
        rectangle.rotation = i * (360 / 8)


if __name__ == "__main__":
    neoscore.setup()
    rotate_and_box("noteheadWhole", 10, 0)
    rotate_and_box("accidentalSharp", 8, 100)
    rotate_and_box("accidentalParensLeft", 10, 200)
    rotate_and_box("restQuarter", 9, 300)
    rotate_and_box("gClef", 4, 400)
    rotate_and_box("rest128th", 5, 500)
    rotate_and_box("pluckedSnapPizzicatoAbove", 8, 600)
    neoscore.show()

I've added a green dot at each Text.pos, and a red dot at each's transform_origin.

Finally, you may be interested to know that you can display object bounding rects using the environment variable NEOSCORE_DEBUG=1

[ajyoon]

btw, I noticed in your code you had a lot of redundant Unit(...) wrappers. Those are typically only needed when passing in raw integers. Math operators on units typically return other units, so there's no need to do things like Unit(text.x - text.bounding_rect.width / 2), the expression within the parens is already a unit.

[Xavman42]

Phenomenal! Thanks so much! With minimal tweaking of the numbers I was able to get exactly the output I was looking for:

I'll approve the PR momentarily!

I remembered that NEOSCORE_DEBUG=1 did what I was wanting to do, but the note in text.bouding_rect had me worried: "Note that this currently accounts for scaling, but not rotation." For context, I've been messing around with adding rigid-body physics to neoscore objects, so I assumed the debug bounding rect wouldn't cut it. If I'm wrong though, that will save me a decent bit of effort...

Ah, yep, I was being sloppy with the Unit wrapper! I was just on autopilot adding those extra wrappers - I usually find myself in a situation where I want to add an integer to a Unit like text.x + Unit(5), so I've gotten in the lazy habit of just wrapping everything rather than think about types.

[ajyoon]

That physics project looks awesome! Making bounding rects respect rotation is a bit messy, because different definitions and use-cases for bounding boxes do or don't respect rotation. The neoscore Rect class itself has no support for rotations on its own either. I'm open to suggestions for how to improve this situation.

[Xavman42]

No suggestions come immediately to mind, but I haven't spent a lot of time on this project. If you like, you can take a look at what I have here. Since I made that video, I've started visualizing the rotation physics. Treble clefs still look funny, but I'm sure that's something sloppy on my end.

Even if the Rect class doesn't rotate, Path.rect does, and that seems to be working well enough for now. Ultimately, I don't want to visualize bounding rectangles, it's just useful for making sure collisions are working properly.

[ajyoon]

Super cool! If you end up coming up with some robust way to derive simplified convex hulls from neoscore paths, I'd be open to pulling that into the library. Although there isn't currently a Text.to_path() method, implementing one would be pretty straightforward since that's what we do under the hood already at the interface layer.