VAO instance cache may be invalid

[julee]

The VAO instance method uses the program id as a key to cache VertexArray internally.

However, if the original Program is released, and a new Program is created, the opengl id of the new Program may be the same as the original program.

But the vertex attributes of the new Program have a different layout. At this time, calling the VAO instance method to get the VertexArray from the cache will be wrong!

This is a potential error behavior, difficult to be discovered.

A better way might be to remember the layout of the vertices, see if they are consistent, and then decide whether to reuse the VertexArray in the cache.

Source code with potential issues:

    def instance(self, program: moderngl.Program) -> moderngl.VertexArray:
        """Obtain the ``moderngl.VertexArray`` instance for the program.

        The instance is only created once and cached internally.

        Args:
            program (moderngl.Program): The program

        Returns:
            ``moderngl.VertexArray``: instance
        """
        vao = self.vaos.get(program.glo)
        if vao:
            return vao

[einarf]

Right. I guess that is driver dependent. More data needs to be added to the key.

Potential key values

• The id() of the object
• The glo
• program members

This is a semi-hot path, so it should ideally be as light as possible.

[einarf]

It could also add some unique identifier in the Program.extra dict. That might be the best solution

[julee]

It could also add some unique identifier in the Program.extra dict. That might be the best solution

Program.extra is for user use, it seems inappropriate to use it here. Your suggestion of using "The id() of the object" may be a better idea.

[einarf]

Python's id() can have the same problem as using program.glo. I've had problems with this in the past.

[julee]

Python's id() can have the same problem as using program.glo. I've had problems with this in the past.

Yes, I forgot it.
Perhaps we can use weakref to track whether the program object still exists, if it exists and the id is the same, we can confirm that the program matches.

The use of Program.extra that you mentioned might be the simplest method, but from an interface design perspective, extra should be for the user to use, and the user could overwrite it with a new object at any time.

Similarly, adding an additional identifier member in Program might also be a way. Perhaps this is the most intuitive way. But this requires modifications in moderngl.

[einarf]

WeakValueDictionary is definitely the only solution here as far as I can see.

[einarf]

I don't know what I was thinking when suggesting storing VertexArrays in a weak structure. That will of course re-create them every render call. I think something needs to be done on the moderngl side to solve this. Maybe some creation counter or something.

Also remember that you don't have to use the VAO wrapper. It's just a convenient way to auto map buffers with different programs also supporting a sub-set of the buffers. You can just do instance(program) and throw away the VAO if possible.

An alternative is just keeping the old program around in some cache. Will have to ponder more on this one.