ManimCommunity / ManimPango

Binding for Pango, to use with Manim.
https://manimpango.manim.community
MIT License
41 stars 13 forks source link

Simpler API implemented #78

Closed YishiMichael closed 2 years ago

YishiMichael commented 2 years ago

This new api mainly cleans up the code, with a new interface provided:

def markup_to_svg(
    markup_str: str,
    file_name: str,
    width: int = 1920,
    height: int = 1080,
    *, # keyword only arguments below
    justify: bool = False,
    indent: int = 0,
    alignment: int = 0,
    line_width: int = -1,
) -> str

This interface can handle Text and MarkupText simultaneously.

I notice that among the parameters @naveen521kk has provided in #2355, most can be directly handled with attributes of markup tags, e.g. font, color, strikethrough, baseline_shift, open_type_features, letter_spacing (Visit https://gnome.pages.gitlab.gnome.org/gtk/Pango/pango_markup.html to see more). Each substring of FormattedString may have different properties. The rest (like justify, indent, insert_hypens) are properties of paragraphs, so one entire FormattedString should have only one for each of them.

In such a case, IMHO I think we only need an api that looks like

def markup_to_svg(markup_str, file_name, justify, indent, insert_hypens)

or, if FormattedString is implemented,

def markup_to_svg(formatted_string, file_name)

The other properties can be simply dumped into markup_str in advance. You may see https://github.com/YishiMichael/manim/blob/master/manimlib/mobject/svg/text_mobject.py as a reference.

Tests are updated inplace (i.e. in /tests/ folder), but I don't know how to run these test files...

naveen521kk commented 2 years ago

When writing https://github.com/ManimCommunity/manim/issues/2355 I was in the plan to use PangoAttributes instead of just making Markup ourselves as you suggested. This would avoid the overhead of Pango parsing the markup to PangoAttributes and manim converting the stuff user-specified to markup; so I don't see a reason why we shouldn't use PangoAttributes.

I also noticed your changes are very similar to MarkupUtils.text2svg why not simply use it instead of creating a new API?

Tests are updated inplace (i.e. in /tests/ folder), but I don't know how to run these test files...

Installing dev-deps from requirements-dev.txt and then running pytest should do.

YishiMichael commented 2 years ago

I've just initialized manimpango/formatted_string.py, basically followed #2355. Any suggestions?

When writing https://github.com/ManimCommunity/manim/issues/2355 I was in the plan to use PangoAttributes instead of just making Markup ourselves as you suggested. This would avoid the overhead of Pango parsing the markup to PangoAttributes and manim converting the stuff user-specified to markup; so I don't see a reason why we shouldn't use PangoAttributes. Got that. Then I'll try fetching PangoAttributes from pango in the future.

YishiMichael commented 2 years ago

The current FormattedString gives the following result. Is this the desired output?

s = FormattedString("0<i>12345</i>67", color="red")
s.update_local_attrs((7, 14), {"fgcolor": "green"})
s.update_local_attrs((6, 8), {"bgcolor": "yellow"})
print(s.get_text_pieces())
print((s + "89").get_text_pieces())
print((s + FormattedString("<span font_family='monospace'>8</span>9")).get_text_pieces())
[('0', {'foreground': 'red'}), ('12', {'foreground': 'red', 'font_style': 'italic'}), ('3', {'foreground': 'red', 'font_style': 'italic', 'background': 'yellow'}), ('4', {'foreground': 'green', 'font_style': 'italic', 'background': 'yellow'}), ('5', {'foreground': 'green', 'font_style': 'italic'}), ('6', {'foreground': 'green'}), ('7', {'foreground': 'red'})]
[('0', {'foreground': 'red'}), ('12', {'foreground': 'red', 'font_style': 'italic'}), ('3', {'foreground': 'red', 'font_style': 'italic', 'background': 'yellow'}), ('4', {'foreground': 'green', 'font_style': 'italic', 'background': 'yellow'}), ('5', {'foreground': 'green', 'font_style': 'italic'}), ('6', {'foreground': 'green'}), ('789', {'foreground': 'red'})]
[('0', {'foreground': 'red'}), ('12', {'foreground': 'red', 'font_style': 'italic'}), ('3', {'foreground': 'red', 'font_style': 'italic', 'background': 'yellow'}), ('4', {'foreground': 'green', 'font_style': 'italic', 'background': 'yellow'}), ('5', {'foreground': 'green', 'font_style': 'italic'}), ('6', {'foreground': 'green'}), ('7', {'foreground': 'red'}), ('8', {'foreground': 'red', 'font_family': 'monospace'}), ('9', {'foreground': 'red'})]
naveen521kk commented 2 years ago

Why do you want to close it?

YishiMichael commented 2 years ago

I've just noticed you had already done some work on refactor branch. Honestly it's beyond my ability... I think I'll turn back to it sometime later. And probably that branch seems to be a better starting point.

naveen521kk commented 2 years ago

I've just noticed you had already done some work on refactor branch. Honestly it's beyond my ability... I think I'll turn back to it sometime later. And probably that branch seems to be a better starting point.

Fair enough. Though I would be really busy around this month because of exams and can only start working on that after that's done.