personal opinion: your code is bad!

[duwangthefirst]

• hard to read

  • use decoratior and metaclass
  • 3 types of renderer, cairo/opengl/webgl, BaseRenderer/SubRenderer, Scene/Renderer/MObject, the author of the related code seems superior, using lots of fancy grammer, trying to seperate things up(trying to implement the Factory mode), but the truth is that there are lots of cross-reference. I just wonder how you gonna maintain this mess in the future.

• bad documentation

  • many popular python library use auto-generated api reference, which is based on doc-comment of each functio/class/module, BUT your api reference seems to be handwitten line by line.
  • tutorials are mostly based on example, there are no documentation introducing pipeline or best-practice or other deeper topic like how you implemented the gpu acceleration. Or maybe your team think the user just need to know how to write construct() function and put all the needed lego blocks(different MObject/Animation) together.

The result is: the potential developer/contributor interested in core function(like rendering process) may get blocked away. It seems that what contributors can do nowadays is some documentation-editing staff only, which souds sucks.

[christopher-besch]

This is comedy xD
But I totally agree that the code base is bad. When you're using a static type checker like Pyright, it flags everything. This is how most of the code looks like:

I'm already used to disabling IntelliSense in my IDE every time I work on Manim. I honestly have no idea how Manim is working at all.
The problem is that we're building on top of an old (and frankly pretty bad) code base. Today this project is community maintained with no paid devs—most devs are here mainly to have fun. And because it's no fun rewriting Manim in its entirety, few people are interested in doing so. Almost all contributions are in the frontend, adding new animations or Mobjects.

But there is some hope on the horizon. Starting in January, Manim will enter a maintenance mode where no community contributions will be merged. This allows some more radical refactors of the "middle-end", the part that connects the frontend and backend. The next step is an entirely new renderer (possibly written in C++). That will be worked on in parallel to the normal work on Manim. This doesn't require the maintenance mode, because the community doesn't touch the renderer.

[Darylgolden]

In general I agree with your assessment. However,

use decoratior

I don't think there's anything bad with using decorators.

many popular python library use auto-generated api reference, which is based on doc-comment of each functio/class/module, BUT your api reference seems to be handwitten line by line.

It's not handwritten. You may be confusing it with the .po files, which are automatically generated files to be sent for translation.

the author of the related code seems superior

No one is trying to flex their coding abilities with that code, it was just what seemed to be the best solution at the time. (We quickly realized how messy it made everything become, however.)

there are no documentation introducing pipeline or best-practice or other deeper topic like how you implemented the gpu acceleration

That's mostly a case of no one getting to doing it yet.

I also think there are better and more polite ways of phrasing your message. You make good points, but your tone borders on being rude, which would not be in line with our code of conduct.

[duwangthefirst]

@Darylgolden sorry I don't mean to be rude. It's just driving me crazy trying to seperate the opengl rendering code(for image and text) apart. And english is not my native language so it's hard for me to express my opinion clearly and politely at the same time.

1. decorator and metaclass are fine, but not easily readable. all I want to convey is that the code is "hard to read" for rookie like myself.
2. the main problem is "cross-reference", for example, a Scene holds a Renderer, and a Renderer also need to get init with a Scene. the other problem is: the Scene need to know which type of renderer to use according to cfg.renderer param, and this kind of judgement logic is making the Scene class too huge to maintain, I myself prefer to just create three library: manim_opengl, manim_cairo and manim_webgl.
3. the documentation issue: I may mix the concept of this project's "Reference Manual" with the usual "api reference", it‘s just not covering all the details systematically. I think when it comes to python coding the author of the code should be responsible for its related documentation. for example, one write a function, the doc should be like:

def transpose(a, axes=None):
    """
    Reverse or permute the axes of an array; returns the modified array.
    For an array a with two axes, transpose(a) gives the matrix transpose.
    Refer to `numpy.ndarray.transpose` for full documentation.
    Parameters
    ----------
    a : array_like
        Input array.
    axes : tuple or list of ints, optional
        If specified, it must be a tuple or list which contains a permutation of
        [0,1,..,N-1] where N is the number of axes of a.  The i'th axis of the
        returned array will correspond to the axis numbered ``axes[i]`` of the
        input.  If not specified, defaults to ``range(a.ndim)[::-1]``, which
        reverses the order of the axes.
    Returns
    -------
    p : ndarray
        `a` with its axes permuted.  A view is returned whenever
        possible.
    See Also
    --------
    ndarray.transpose : Equivalent method
    moveaxis
    argsort
    Notes
    -----
    Use `transpose(a, argsort(axes))` to invert the transposition of tensors
    when using the `axes` keyword argument.
    Transposing a 1-D array returns an unchanged view of the original array.
    Examples
    --------
    >>> x = np.arange(4).reshape((2,2))
    >>> x
    array([[0, 1],
           [2, 3]])
    >>> np.transpose(x)
    array([[0, 2],
           [1, 3]])
    >>> x = np.ones((1, 2, 3))
    >>> np.transpose(x, (1, 0, 2)).shape
    (2, 1, 3)
    >>> x = np.ones((2, 3, 4, 5))
    >>> np.transpose(x).shape
    (5, 4, 3, 2)
    """
    pass  # the code of this transpose function is ommited

[huguesdevimeux]

Hello and thanks for your opinion !
To be brutally honest with you, I think the substance of your message is somewhat true and fairly debatable, but the form is uninteresting.

That's true, codebase of Manim is improvable. Broadly speaking, the whole design with the renderer abstraction can be debated, as well as how Mobjects are declared and used through the library. The bezier handling is incredibly complex, undocumented, the frames generation is messy with Scene, renderer, and Camera calling each other in a row, with some methods that do a lot of more that one thing, there is a lack of abstraction, the code is not scalable. Nothing is immutable, there are too many methods in the Scene class, the Bézier curves handling is hand-crafted, etc.

But that works. A common joke among devs is "Why does Manim even works?"

Manim Community has been forked from the original version of 3b1b, who is not a software engineer, but has made from his hands a complete 2d animation engine. It wasn't done for being open-source, nor to be a public tool at all, it was just a personal tool, and the code can witness that. From now 1 year, our mission is to improve this codebase, and we are trying our very best for that.
That being said, I prefer action to speeches, so I feel free to fix the issues you raised. To be honest, I don't understand why you think that decorators are bad, I can see why (and agree) you don't like the meta-class approach, and I agree with the cross-references thing, and I don't see where the hell you see a Factory design pattern (there is a shy one in testing/utils/class_maker, but .. that's not really one).

I just wonder how you gonna maintain this mess in the future.

No one knows. But one thing sure, the same question is raised for every large open-source project. Take your message, copy and paste it to another project, and you will get the same replies as here. That's open source : people with different backgrounds (not software engineers) work together, you won't get a google-standard quality code, but it works, and it works fine. :D

[duwangthefirst]

I admit that manim is a nice tool to create demo videos.
Inspired by it, I want to make a lib (like a lite version of manim) focused on transfering rgba images,texts and audio(generated by TTS engine) to video(maybe to publish at tic toc).
It's the horrible structure of the core-function-code that makes me worried about that it may get too huge to maintain in the future.
I really don't mean to be affensive. :) (dog face)

[eulertour]

Regardless of how you feel about their tone, @duwangthefirst's assessment is absolutely right. And @huguesdevimeux's response illustrates well why it isn't going to be fixed.

The codebase has never been good. And while it's true that Grant admitted as much when he was writing it on his own, that was over a year ago now. Are you going to keep making that excuse a year, two, or five from now?

This idea that this is an unavoidable part of open source is wrong on its face. Manim is this way because so many people decided from the beginning that the primary goal of the project should be to have fun. That's why nobody wants to refactor anything, nobody wants to learn how rendering works, and everyone instead just wants to add their own increasingly tiny contribution to feel accomplished.

"Having fun" with a software project doesn't come for free. The cost is confusing, unscalable, inextensible, unmaintanable code and a lack of meaningful progress even after years of work. And from what it looks like, that isn't going to change any time soon.

[huguesdevimeux]

Are you going to keep making that excuse a year, two, or five from now?

I don't have any number to show, but it's likely that around 70-80% of the codebase is still from Grant. Without saying that we are in any way improving or degrading the code quality, this excuse is still coherent if we assume that Grant's codebase alters the overall quality.

Every FOSS project is built for fun. No one wants to work for free on a project without having fun. The real reason is that Manim is a mathematically oriented project with a very broad range due to the coverage made by the 3b1b YouTube channel. Due to that, many people with very different background (often not computer science as a main) come to contribute, which logically and ultimately alters code.

I feel like in your message, you are implying that people shouldn't have fun coding manim and rather be focus on productivity and quality of code. I don't want to argue with that, but I personally fundamentally disagree, as someone who took a lot from open-source and wouldn't be where I am if your way of seeing the situation was applied everywhere.

[eulertour]

There's a lot there that I don't agree with, but I'll just focus on this main point:

I feel like in your message, you are implying that people shouldn't have fun coding manim and rather be focus on productivity and quality of code.

Let me clarify. I wholeheartedly believe that having fun should always come second to productivity and code quality, not that the two are mutually exclusive. Said another way, if there's ever a conflict between what's fun for people to do and what's better for the library it's a mistake to choose to do what's fun.

[GameDungeon]

While I am not as involved in this project as much as many others, the large problem I see (not that it's the only one) is the scene and renderer dynamic. I was trying to figure something out about how an animation went from being called at self.play to being rendered. I bounced between the scene and render file about 20 times before giving up. I really like the mobject and animation system, and while it could be improved, its flexibility is why I keep coming back to manim. The other problem is the lackluster implementation on opengl.

If we could:

• Make the render / scene thing make sense
• *Make the implementation of new renders easy and make sense

Manim would be significantly improved.

*While I support the idea of creating our own renderer, I strongly disagree with locking us into only one renderer.

[BorisTheBrave]

I've reviewed the codebase myself, and offer the following observations.

1. As you said, renderer separation is not good. But imho the duplicate code for opengl classes is an order of magnitude worse in terms of code quality. There's so much code there, it's impossible to tell how those classes actually differ from the original ones. I'd prioritize fixing that. If they drift too much, it'll become unfixable.

2. Manim objects have two different sets of data. There is the python data, such as the type of an object and it's fields, and the vector data. These things can very easily get out of sync (e.g. using become) and that leads to un-intuitive behaviour. I think this one is too late to change, but I think it would have been better if you cannot morph between arbitrary things unless you explicitly convert to a path object.

3. Relatedly, manim's story for animating any property that is not vector data is terrible. Hence why ValueTracker secretely creates vector data under the hood. None of the Animation classes do much for you, you are stuck using updaters. And most python properties are read only.

4. Manim doesn't have a scene graph. Sure, objects can contain sub objects, but there are no transforms associated with them. In fact, there are no transforms stored per object in manim at all. If you want to translate an object, it literally updates every point in the vectorized shape. This leads to a lot of problems. It's not efficient to simply move stuff around. You cannot compose a lot of animations.

5. Similar to the lack of scene graph, objects don't have boundaries. Every time you do next_to etc, it scans the entire vector data to work out the bounds. This isn't the end of the world, but I do think most similar software caches the bounds.

6. Manim is not fun for beginners (though the install process was pretty good). I've already raised a PR about shoddy type annotations and weak docs, but they still fall short in a lot of places. You don't get any useful warnings if you do something wrong either. Calling missing methods or writing missing attributes just silently does nothing. There's no clear distinction between public and private methods, leading to dozens of mostly useless methods exposed to users.

Imho renderers and performance are the least of your problems. It doesn't need a new renderer, and it especially doesn't need a C++ plugin (all the heavy lifting should be done by opengl/cairo/numpy which are already plugins).

Before sounding too negative, I wanted to mention some things this project is doing well. manim is very easy to set up, and your docs often include visual examples that are amazingly useful. It was also extremely easily to onboard as a new developer, with absoltely tons of automated suites working very nicely and the core devs are active and friendly on the discord.

Fwiw I have no issues with the use of decorators. The opengl metaclass hack is not so good, but that's the specifics of what it is doing rather than metaclasses themselves.

Manim still looks salvagable to me. It doesn't need any sweeping changes, just some care to improve the code quality. If "how does manim even work" is a running gag, you should invest in more comments and docs on the internals of manim. You cannot really make large changes without understanding the original code anyway.

[christopher-besch]

I almost entirely agree with your points. Except for one:

[...] it especially doesn't need a C++ plugin [...]

I am not promoting a C++ rendering backend solely because of performance reasons.
C++ is a lot more strict than Python. Many of the things you're complaining about would strait up not be possible with C++. Python makes it very easy to write bad code. And I'm afraid Manim took that chance and ran with it, implementing whatever feature seemed cool at a time no matter the cost. That's the main reason why I'm pushing for a C++ backend, clean and compiler-enforced explicitly typed code.

all the heavy lifting should be done by opengl/cairo/numpy which are already plugins

Nothing I'd like to see written in C++ would replace NumPy, Cairo is simply not providing the features we need (e.g. 3D) so we have to replace it and OpenGL is just a rendering API. We need so much more than just OpenGL. We need a clean abstraction of different rendering APIs. After all OpenGL is pretty old itself and already deprecated on some platforms. We need the ability to easily switch to a different rendering API when the time arises.
And that's a perfect job for C++.

[BorisTheBrave]

Well, this is the vent opinions thread :D

As I gather you are having a stab at this C++ stuff soon, it'll become clear how well it works, so no need to spend time on debate when we'll have actual evidence.

[mainrs]

I didn't look that much into the code and I know that it is a huge burden, but from the feedback gathered here, wouldn't it be maybe a better idea to rewrite it from the ground up? The problem seems to lie deeply in how animations and scenes interact. There doesn't really seem to be an alternative than to rewrite it.

I am no Python expert at all, but having some more free time from March on I'll try and give it a shot. Even if it's just for me to learn something new. I'd prefer Rust tbh. but seeing that a lot of (data) science is done in Python it might be a good idea to keep the language.