3b1b / manim

Animation engine for explanatory math videos
MIT License
71.13k stars 6.25k forks source link

Fix bad 3D overlapping #2231

Closed MathItYT closed 3 weeks ago

MathItYT commented 1 month ago

Motivation

I was rendering a Scene where I noticed an ugly video result, which is attached below:

https://github.com/user-attachments/assets/ea3ded8d-976a-4714-8091-4b9ea5ecdb23

With the new changes, it renders like below:

https://github.com/user-attachments/assets/0e4b871b-481b-4f64-8311-5366be1bc186

Proposed changes

I changed mobject.py and camera.py . Before, Mobject.get_shader_wrapper_list returned all shader wrappers from the family so that they're not pointless. Now, it returns an empty list if the Mobject has no points and a list with a single shader wrapper if it indeed has points, which corresponds to self. And the Camera.capture method changed these lines:

The problem was that it badly handled mobjects with recursive submobjects. It rendered submobjects in the same shaders, but now, it render each family member separately, and that fixes my issue.

3b1b commented 4 weeks ago

This is definitely a bug, but I'd be hesitant to fix it this way.

By default, manim will do its best to group together items to render all at once, since otherwise it can become very inefficient for large groups. What's happening here is that when it renders a VMobject, it first does the fill then the stroke, but here when it groups them all together, you don't actually want the stroke rendered on top of all the fill. Your fix works because it prevents that grouping, so each object is rendered one at a time. However, for groups with many submobjects, e.g. text, this will start to slow things down quickly.

When depth matters like this, the right way to handle it should be to enable depth testing via mobject.apply_depth_test(). However, it seems like that has a bug at the moment with respect to filled-VMobjects.

In the meantime, for your particular scene, a hack that would make it work is to add some non-visible non-VMobject in between the panels of your slides in 3d, e.g. loop through them and call self.add(panel, Point()).

MathItYT commented 4 weeks ago

I see, do you think it would be a nice way to mathematically blend colors if there was some color in that specific pixel?

3b1b commented 4 weeks ago

I'm not sure I understand your question

MathItYT commented 4 weeks ago

I read a Wikipedia article https://en.wikipedia.org/wiki/Alpha_compositing. It says that you can mathematically compose alphas so that if an object is fully opaque and it is over another object, it will cover it, and if it has some transparency, it will "mix" the colors.

So, I think it would be nice to edit GLSL shaders at quadratic_bezier and use color variable together with frag_color to fix the bug. My problem is that stroke and fill shaders are treated separately.

3b1b commented 4 weeks ago

It certainly does alpha blending already, that's all very standard. Note, though, that the way colors blend is dependent on the order in which they are rendered, so the bug you're highlighting really is about the order of rendering and not a blending issue.

As I said, the right way to address it will be to fix the fact that depth testing does work with fill at the moment, but I can look into that later.

MathItYT commented 3 weeks ago

@3b1b I reverted changes at camera.py and mobject.py and modified scene.py in the following way:

...
def assemble_render_groups(self):
        """
        Rendering can be more efficient when mobjects of the
        same type are grouped together, so this function creates
        Groups of all clusters of adjacent Mobjects in the scene
        """
        batches = batch_by_property(
            self.mobjects,
            lambda m: str(type(m)) + str(m.get_shader_wrapper(self.camera.ctx).get_id()) + str(m.z_index)
        )

        for group in self.render_groups:
            group.clear()
        self.render_groups = [
            batch[0].get_group_class()(*batch)
            for batch, key in batches
        ]
...

I added z_index requirement to group mobjects so that if you set z_index, the problem will be solved.

3b1b commented 3 weeks ago

Ah, perfect, that’s a good way to fix it.