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

Providing a Stable API #28

Open
naveen521kk opened this issue Feb 26, 2021 · 18 comments
Open

Providing a Stable API #28

naveen521kk opened this issue Feb 26, 2021 · 18 comments
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@naveen521kk
Copy link
Member

naveen521kk commented Feb 26, 2021

Currently, this project is just for the use of Manim, and mostly it can't be used outside of manim, with the functions it provides.

Having a stable API so that it can be used outside of Manim would be super cool, for example, I recently read python-pillow/Pillow#2255 and found that Pillow can't render text correctly, especially for languages with more ligatures, at least it requires raqm and isn't available by default in the wheels, and by if we had a stable API Pillow could use this to render text.

I would like to hear some ideas on how this can be done and how the API should look like.

Thoughts @PhilippImhof?

@PhilippImhof
Copy link
Member

That's a tough one. On one hand, it would obviously good to offer this toolkit to others. On the other hand, it means much more work. Currently, we only have to meet Manim's needs; there will certainly be other needs once it is used in other projects. Are we sure we can handle the additional workload?

@naveen521kk
Copy link
Member Author

Are we sure we can handle the additional workload?

I think there shouldn't be much of workload though. Even if there is, I think there should be other developers taking interest to develop this project. And, yes this is a cool project, which I, personally, love to work on.

I haven't seen any python project, even if its a binding, provide text rendering as this project does and we have wheels which pretty much work on any platform, that makes me think, why other projects, say pillow, can't use it.

Also, on the other hand I think the current implementation of Text isn't that great, and requires a major refactor. Making a stable API first and then making Manim's Text use it seems to be a better idea to me. First, though we should plan about how the API should look and how it is expected to use...

Also, currently we seem to duplicate code in Text and MarkupText which needs to be fixed.

@PhilippImhof
Copy link
Member

I think there shouldn't be much of workload though. Even if there is, I think there should be other developers taking interest to develop this project. And, yes this is a cool project, which I, personally, love to work on.

True.

Also, on the other hand I think the current implementation of Text isn't that great, and requires a major refactor. Making a stable API first and then making Manim's Text use it seems to be a better idea to me. First, though we should plan about how the API should look and how it is expected to use...

Also, currently we seem to duplicate code in Text and MarkupText which needs to be fixed.

IMHO, MarkupText is the way to go for the future, because it offers a clear and well-structured syntax; formatting is done in the text rather than with obscure dicts given as parameters. It already avoids most of the problems that Text has (e.g. bold+italic) and will avoid all of them, as soon as we handle gradients with Cairo instead of applying them manually onto the SVGMobject. (I do not say that because I wrote it; rather, I wrote it because I felt there was a need for it.)

We could probably keep Text as a simple class with just a few options, like color, font and size. Of course, we would then also need to rewrite the CodeBlock class.

@naveen521kk
Copy link
Member Author

I had a look over PangoAttribute, which should be used ideally instead of the dicts which are currently in place, like t2g and t2c. I had previously worked on them in this branch but had stopped it because it required a lot of work to do.

Essentially what I thought when I was doing that was, having a TextAttribute object, which will provide access to the attributes which Pango allows us to set, each representing a single attribute, like for example bold from the characters 6 to 10, and finally inserted in a PangoAttrList like structure which will be passed to Pango while rendering, it also allows to have overlap so blod+italic should be possible.

This API looks good to me, but still, I was wondering if there is a better way to do it, suggestions @PhilippImhof?

@PhilippImhof
Copy link
Member

I am not sure I understand the question.

As the user will still have to define style from char X to Y, I think that using some sort of markup would be the easiest interface. This also has the advantage that they do not have to actually count characters. And as Pango offers to do the whole parsing (thus translating markup to Pango attributes), using https://developer.gnome.org/pango/stable/pango-Markup.html seems straightforward. Especially, because with MarkupText we already have a working implementation for this.

@naveen521kk
Copy link
Member Author

Yeah, markup is great.
Obviously, using markup should be the simplest for the users, but for the Code we have in Manim, it should be difficult for us converting the information provided by pygments to markup. Instead, these attributes can be used. Also, user's don't need to learn that markup, if we implemented a Pythonic API for achieving the same. Also, I think when it comes to Gradients, we would only be able to support linear ones, using Markup, if we had an API we can support other types of gradients which Cairo supports, and that's one of the main reason Pango doesn't support it.

@PhilippImhof
Copy link
Member

Pygments supports HTML output. I think I might be able to write a Pango formatter, it seems to be more or less straightforward to do so; but that's what I suppose after having skimmed the docs.

@naveen521kk
Copy link
Member Author

I tried that, but it seems too difficult to convert them because pygments uses CSS which Pango doesn't support at all. The best is to convert them using PangoAttributes, and I will soon be working on it anyhow.

@PhilippImhof
Copy link
Member

I have just given it a quick try and it seems to work. There is some fine-tuning and intensive testing left to do, of course.
(And a PR to file to pygments.)

@PhilippImhof
Copy link
Member

from pygments import highlight
from pygments.lexers import JavascriptLexer
from pygments.formatters import PangoMarkupFormatter

code = """
function bla(n, m) {
   console.log(n*m);
}
"""
print(highlight(code, JavascriptLexer(), PangoMarkupFormatter()))

yields

<tt><span fgcolor="#008000"><b>function</b></span> bla(n, m) {
   console.log(n<span fgcolor="#666666">*</span>m);
}
</tt>

and with this I can do

from manim import *
from manimpango import *

class TestCode(Scene):
    def construct(self):
        text = MarkupText(
            """<tt><span fgcolor="#008000"><b>function</b></span> bla(n, m) {
   console.log(n<span fgcolor="#666666">*</span>m);
}
</tt>""")
        self.play(Write(text))
        self.wait()

to get:

TestCode.mp4

@naveen521kk
Copy link
Member Author

This looks really cool. 👍

@naveen521kk
Copy link
Member Author

Now, I think the first thing we should do is provide an API to access the image other than SVG. So, the first step I think would be to clean some repetitive code, and organize it as such a .pxd file for Pango, Cairo and Glib. I will make a new issue on that.

@naveen521kk naveen521kk added enhancement New feature or request help wanted Extra attention is needed labels Mar 1, 2021
@naveen521kk
Copy link
Member Author

@PhilippImhof Can you list down what all we should refactor? ManimCommunity/manim#1102 (comment)

@naveen521kk
Copy link
Member Author

Also, we should add an API to return the buffer so that outside of Manim, as we can't expect them to parse those SVG files. We should kinda use buffer API from CPython, where we can get the image data from Cairo when we are using Image Surface using cairo_image_surface_get_data, but we are currently using SVG surfaces everywhere so it would need a different implementation. And this would mean there would be a complete refactor soon ™️.

@naveen521kk
Copy link
Member Author

I am working on this next :)

@naveen521kk naveen521kk added this to the v1.0.0 milestone Mar 30, 2021
@naveen521kk
Copy link
Member Author

Idea's which I have in mind currently.
manimpangodev
Let's see how things goes.

@PhilippImhof
Copy link
Member

IMHO this is well thought and a good way to go!

@naveen521kk
Copy link
Member Author

Now that I'm planning for ManimCommunity/manim#2355 I think this is the time to fix this also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants