manim icon indicating copy to clipboard operation
manim copied to clipboard

Fix bad 3D overlapping

Open MathItYT opened this issue 1 year ago • 5 comments

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:

  • Before:
...
for mobject in mobjects:
            mobject.render(self.ctx, self.uniforms)
...
  • Now:
for mobject in extract_mobject_family_members(mobjects, True):
            mobject.render(self.ctx, self.uniforms)

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.

MathItYT avatar Oct 23 '24 23:10 MathItYT

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()).

3b1b avatar Oct 24 '24 14:10 3b1b

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

MathItYT avatar Oct 24 '24 15:10 MathItYT

I'm not sure I understand your question

3b1b avatar Oct 24 '24 15:10 3b1b

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.

MathItYT avatar Oct 24 '24 15:10 MathItYT

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.

3b1b avatar Oct 24 '24 15:10 3b1b

@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.

MathItYT avatar Oct 27 '24 17:10 MathItYT

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

3b1b avatar Oct 27 '24 18:10 3b1b