Allow passing color to TextSettings

[marcin-serwin]

This will allow to fix problem described in ManimCommunity/manim/pull/2341

[PhilippImhof]

Thanks for the contribution. I will have a closer look at this at latest by the end of the week. I hope you can wait until then and I apologise for probably not being able to do it sooner.

The approach you are using is similar to what MarkupText does. (Actually, that was the reason why I wrote that class roughly a year ago. I was regularly having trouble with t2c and co.) I think, it should be the final goal to merge Text and MarkupText into one single non-LaTeX text class. But I am not sure there is a consensus on that. And your contribution definitely improves the existing Text class.

[naveen521kk]

Thanks for the PR. This looks good to me. Using https://gnome.pages.gitlab.gnome.org/gtk/Pango/struct.Attribute.html seems like a better idea to me rather than using PangoMarkup. I guess I will work on it later.

[PhilippImhof]

Thanks for the PR. This looks good to me. Using https://gnome.pages.gitlab.gnome.org/gtk/Pango/struct.Attribute.html seems like a better idea to me rather than using PangoMarkup. I guess I will work on it later.

I think that PangoAttribute has its advantages. My concern is rather on the end-user side of it. I think it would be good to get rid of the archaic syntax we have inherited, also because it does not really allow overlapping formatting. Using PangoMarkup is an easy solution, because it offers a solution on the backend and on the frontend.

[naveen521kk]

I think that PangoAttribute has its advantages. My concern is rather on the end-user side of it.

Probably we should this a better API than the current one. I mentioned PangoAttribute because it would be better than creating XML kinda strings ourselves and then passing it to pango.

[marcin-serwin]

I've noticed another problem, this time with gradient. It is a problem for both Text and MarkupText, namely the following code

class Test1(Scene):
    def construct(self):
        self.add(
            Text(
                "gradiented office isn't perfect",
                t2g={"office": (GREEN, RED)},
            ).move_to(UP))
        self.add(
            MarkupText(
                'gradiented <gradient from="GREEN" to="RED">office</gradient> isn\'t perfect',
            ).move_to(DOWN))

when run with current main branch results in

I've changed the implementation of gradient in text to set colors individually on each letter but this exposed another problem, namely the same code now results in

As you can see the ff ligature is no longer present. This is again problem with ManimPango as text2svg draws each group of formatted letters individually. Hence this code:

class Test2(Scene):
    def construct(self):
        self.add(
            Text(
                "gradiented office isn't perfect",
                t2s={"f": NORMAL},
            ))

results in

because ManimPango draws

• gradiented o
• f
• f
• ice isn't per
• f
• ect

[marcin-serwin]

The tests are failing because windows is missing some fonts. I don't think it is caused by my changes, as I didn't touch fonts.

[PhilippImhof]

AFAICS, the error mentioned above (gradiented office) is explained in the docs for MarkupText: one can use <gradient from="RED" to="YELLOW" offset="2,1">example</gradient> to start the gradient two letters earlier and end it one letter earlier

I admit that this is only a workaround. However, as long as we do not have gradients built-in to Pango, we must do them at the SVG level and there just seems to be no way to reliably know how many glyphs you really get from a certain text.

[naveen521kk]

The tests are failing because windows is missing some fonts. I don't think it is caused by my changes, as I didn't touch fonts.

I think the change which deletes the SVG file on the error raised has caused the issue. I think removing that could fix the errors(at least the permission one).

[marcin-serwin]

AFAICS, the error mentioned above (gradiented office) is explained in the docs for MarkupText: one can use <gradient from="RED" to="YELLOW" offset="2,1">example</gradient> to start the gradient two letters earlier and end it one letter earlier

You're right, I didn't notice that. I think instead of requiring offsets, as a workaround we could set colors directly with markup in strings passed to pango. So for example we could replace <gradient from"WHITE" to="RED">ab</gradient> with <span color="WHITE">a</span><span color="RED">b</span> before sending it to pango. This will of course still be workaround since ligatures will be a single color (of the second letter I think), but as you said we can't do better without built-in gradients.

[naveen521kk]

I think this will still error out. The error was not FileNotExistsError it was PermissionError(which mean the file was opened but wasn't closed). I guess this part of the code can be removed.

[marcin-serwin]

The issue is that on my machine pango left an empty svg file when this occurred and it interferes with caching. That's why I've added cleaning, but it seems windows works differently.

[naveen521kk]

I don't think there exist any caching for tests/_manim.py. Also, I think it should be fine to not remove the file at all since it's just used while testing.

[marcin-serwin]

The caching problem was present when running manim. I'll remove it from _manim tests but it may cause problems later on manim repo with windows build.

[naveen521kk]

In the manim repo, you can check the number of submobjects in the parsed object. If the count is 0 it's an empty SVG(in which case delete the SVG) if not it's correct and is cached.

[marcin-serwin]

Empty svg files cannot be parsed. I get ExpatError: no element found: line 1, column 0 when I try.

[naveen521kk]

Hmm, did you mean by empty SVG file a literal empty file? In my case when I tried previously (without the checks for valid colour), I got a valid SVG file but nothing was in it.

[marcin-serwin]

I did mean literal empty file. Without validation it was a valid svg but now a ValueError is raised so ManimPango creates the file but doesn't populate it.

[naveen521kk]

Without validation it was a correct svg but now a ValueError is raised so ManimPango creates the file but doesn't populate it.

Ah, I see what's wrong. We should close the file before erroring out(which should create a valid SVG) or check before even creating a file. That is we should move the checking part before calling cairo_svg_surface_create or we should say cairo to close the file handles by calling cairo_destroy(), cairo_surface_destroy().

[marcin-serwin]

You're right, that was missing. That's probably also why windows couldn't get permission to delete the file, because it was still owned by the program.

[naveen521kk]

I will have a look at the tests failing on windows tomorrow.

[naveen521kk]

I will have a look at the tests failing on windows tomorrow.

The tests failed because the order of execution of tests has changed. There is some kind of cache in Pango that gets interfered in these tests. Can you rebase on #67 and check if that works?

[naveen521kk]

@marcin-serwin Can you rebase again with main?

[naveen521kk]

I think SVGStyleTester only tests for styles and not actual paths, so I think this can be run on win32 also.

[naveen521kk]

LGTM, thanks for your contribution.

[naveen521kk]

I will release a new version after the CI is fixed #68 . Thanks.

[naveen521kk]

I have released 0.4.0 now, it should have this change.