Add TextureFilter for adding textures to MeshVisuals

[asnt]

This PR implements texture support for MeshVisual as a filter.
Addresses #748

• Extend the MeshVisual vertex shader with a hook for passing the texture coordinates
• Extend the MeshVisual fragment shader with a hook for applying the texture
• Add a flag to toggle the texture
• Add setters for texture coordinates and texture
• Add a demo program
• Add demo data to https://github.com/vispy/demo-data/
• Use demo data from vispy/demo-data

Edit(2018-11-05): Update following #1462 enforcing unindexed buffers for meshes.

[asnt]

This was copied over from get_vertices().
Should there be support for texcoords indexed by faces? (In which case _compute_unindexed_texcoords() can be implemented as in the case of vertices.)

[djhoese]

@asnt I can't say too much to the interface for this right now. Maybe the other maintainers have stronger opinions.

Do you have a self-contained example that I can test this with? Maybe you could add a new example script to the examples directory. If you can't use the existing vispy test/demo data then we can work on adding more to the proper location.

Thanks for all your work.

[kmuehlbauer]

@asnt IIRC there has been the attempt to implement material and shading via filters. But I really can't say if what you want to achieve can be done via the filter approach.
Ping @bollu since he was very much into this back in 2015.

[larsoner]

Not sure if the implementation is optimal and if the decisions integrate well with the existing API.
...
The original shaders were duplicated.

There is machinery in VisPy to swap in GLSL code snippets for these sorts of purposes. Can you look to see how it's done with value-to-color mapping via a colormap? See e.g. this PR:

https://github.com/vispy/vispy/pull/1329/files#r116057462

[asnt]

Thank you all for your comments.

@larsoner I'll have a look at implementing this using the function placeholders and pre/post hooks.

I found some code [1] removed a while ago that seems related to the topic. Not sure yet if this is exactly what I want to do. @kmuehlbauer, is that what you were talking about?

vispy.visuals.filters.material [2] might also be related.

@davidh-ssec I'll add an example.

[1] 457657e#diff-fe7848e225c020030c1852544bc953bd
[2] https://github.com/vispy/vispy/blob/master/vispy/visuals/filters/material.py

[kmuehlbauer]

@asnt Yes, this was what I was referring to. @campagnola, who did the scenegraph-implementation and also designed the filter approach, wrote about having mesh-texture with filters in this comment.

So I would suggest to have a general discussion on implementing textured mesh via the filter-approach. Maybe if @campagnola can point us into the right direction and guidance we can get this implemented. @larsoner I didn't find anything about that texture-API which is referred to in the linked issue. Do you have any pointers?

@asnt Does that make all sense to you?

[larsoner]

I linked to that PR not because of it being related to textures specifically, but rather about swapping in GL code as necessary using snippets and $ syntax. This texture functionality could be implemented that way, or possibly also via a filter. Not sure which would be better.

[kmuehlbauer]

@larsoner Sorry, my bad. I meant not the issue that you linked but this one here (campagnola#10 (comment)). There @campagnola and @bollu were talking about texture API in context of filter.

[larsoner]

It looks like the API hasn't really been hashed out there, and I don't recall it happening elsewhere. So I think it remains an open question. You could look at what glumpy does for inspiration, assuming it has texture mapping.

[asnt]

@larsoner @kmuehlbauer The filter and the modular shader program approaches make sense. I'll have a try at some implementation.

[kmuehlbauer]

@asnt Awesome!

[campagnola]

@asnt I still lurk here now and then; let me know if anything needs explaining (and I will try to remember). There is one example that might help: examples/basics/visuals/visual_filters.py.

[asnt]

The last commits define a TextureFilter to be used on the MeshVisual. Only the fragment shader is modified by the filter. The texcoords are currently part of MeshVisual by default because I couldn't find a way to adapt the main vertex/fragment programs from the filter to pass the texcoords attributes. I am not sure this is the desired interface. Any advice welcome.

Note: Shading is skipped in MeshVisual at the moment for the sake of testing.

An example was added examples/basics/scene/mesh_texture.py with accompanying data in examples/basics/scene/data.
Note: Model 'spot' in the public domain taken from http://www.cs.cmu.edu/~kmcrane/Projects/ModelRepository/

[asnt]

texcoords are passed through the vertex shader only if texcoords are defined in the MeshVisual

[asnt]

Improving on the previous tentative implementation, 697db1f adds the texcoords to the shaders on demande. MeshVisual exposes a varying_texcoord attribute that is used by the TextureFilter to bind the varying in the fragment shader.
Let me know if that makes sense @kmuehlbauer @larsoner @campagnola

[kmuehlbauer]

@asnt I did try the filter approach using your data and code snippets.

This works on the latest vispy master without your changes to Meshvisual and MeshaData. It and attaches and removes the TextureFilter in an endless loop. This works only for shading=None, but I'm sure we can add the shading stuff too.

The TextureFilter:

from vispy.gloo import Texture2D, VertexBuffer
from vispy.visuals.shaders import Function, Varying
import numpy as np


class TextureFilter(object):

    def __init__(self, texture, texcoords):
        self.texture = texture
        self.texcoords = texcoords
        self._texcoords = VertexBuffer(np.zeros((0, 2), dtype=np.float32))
        self._texcoord_varying = Varying('v_texcoord', 'vec2')
        self.apply_coords = Function("""
            void apply_coords() {
                $v_texcoords = $texcoords;
            }
        """)

        self.apply_texture = Function("""
            void apply_texture() {
                gl_FragColor *= texture2D($u_texture, $texcoord);
            }
        """)

        self.coords_expr = self.apply_coords()
        self.texture_expr = self.apply_texture()

    def _attach(self, visual):
        # vertex shader
        vert_pre = visual._get_hook('vert', 'pre')
        vert_pre.add(self.coords_expr)
        tc = self.texcoords[visual.mesh_data.get_faces()]
        self._texcoords.set_data(tc, convert=True)
        self.apply_coords['texcoords'] = self._texcoords
        self.apply_coords['v_texcoords'] = self._texcoord_varying

        # fragment shader
        frag_post = visual._get_hook('frag', 'post')
        frag_post.add(self.texture_expr)
        self.apply_texture['texcoord'] = self._texcoord_varying
        self.apply_texture['u_texture'] = Texture2D(self.texture)

    def _detach(self, visual):
        visual._get_hook('vert', 'pre').remove(self.coords_expr)
        visual._get_hook('frag', 'post').remove(self.texture_expr)

The example program:

import argparse

import numpy as np
from vispy import app, scene
from vispy.io import imread, read_mesh
from vispy.scene.visuals import Mesh
from vispy.visuals.filters import TextureFilter



parser = argparse.ArgumentParser()
parser.add_argument('--mesh', default='./data/spot.obj')
args = parser.parse_args()

vertices, faces, normals, texcoords = read_mesh(args.mesh)
texture = np.flipud(imread(args.mesh.replace(".obj", ".png")))

canvas = scene.SceneCanvas(keys='interactive', bgcolor='white', size=(800, 600))
view = canvas.central_widget.add_view()

view.camera = 'arcball'

shading = None
mesh = Mesh(vertices, faces,
            shading=shading,
            color='lightgreen')

tex = TextureFilter(texture, texcoords)
view.add(mesh)

canvas.show()

# attaching texture filter via timer
def on_timer1(event):
    mesh.attach(tex)
    timer2.start()
    canvas.update()

# detaching texture filter via timer
def on_timer2(event):
    mesh.detach(tex)
    timer1.start()
    canvas.update()

# one second oneshot-timer
timer1 = app.Timer(2., iterations=1, connect=on_timer1, start=True)

# one second oneshot-timer
timer2 = app.Timer(2., iterations=1, connect=on_timer2, start=False)

if __name__ == "__main__":
    app.run()

[asnt]

Thank you @kmuehlbauer
I adapted your snippet slightly to make it work with flat and smooth shading in another branch there https://github.com/asnt/vispy/tree/mesh-texture2
MeshVisual._update_data() had to be adapted to always follow the same branching path for a given mesh. If the indexing of the vertices changes during execution for a given mesh object, we need to reflect this in the indexing of the texcoords in the TextureFilter, which is currently not possible as far as I understand.

Is the timers loop merely used for validating the attaching/detaching of the filter?

[kmuehlbauer]

@asnt This looks really great. Now I have some questions. Is this what you wanted to achieve? Would the filter approach suit your needs totally?

I see the indexing problem too. Can you imagine a machinery to get this straight? Or does you current implementation work around this? I need to think about this. There is also my blunt use of visual.mesh_data.get_faces() for the indexing in the filter. I think we need more opinions here, @larsoner, @davidh-ssec ?

@asnt As you are already very familiar now with filter, 'MeshVisual and 'shading, do you see a possibility to transform the MeshVisual-shading into a filter? We can open a new issue for discussion about this, if there is enough interest.

Yes, the timer loop, was just for checking correct attach/detach mechanism.

[asnt]

@kmuehlbauer Yes, this filter approach achieves what I wanted.
A suggestion, though: maybe the filter could use a boolean flag to toggle the display of the texture filter.show_texture = True/False. This would be a boolean uniform in the fragment shader:

void apply_texture() {
    if (u_show_texture) {
        gl_FragColor *= texture2D($u_texture, $texcoord);
    }
}

This would be used to temporarily hide the texture without detaching the filter. What do you think?

Regarding the indexing, if I get it right, the problem seems to be that, on the first call to _update_data(), the vertices are not face-indexed, and on the subsequent calls, they are. So the indexing of the mesh changes after the filter has set up the non-face-indexed texcoords, and the texcoords buffer thus becomes incompatible in dimensions.
I am not sure why the indexing has to change for a single mesh.
Where are the face-indexed buffers required? Is it for the flat shading?
One way to make the filter reflect the state of the mesh would be to notify the filter whenever the data of the mesh are updated, i.e. when _update_data() is called in the mesh.

I'll have a look at moving the shading code into their own filters.

[kmuehlbauer]

@asnt Yes, sure, the show_texture flag is a good idea IMHO. We can also think about adding setters for texture and texcoords. Would come in handy, if someone wants to change the texture or coords.

To the indexing, I really can't say why the indexing is done, without having a deeper look. But let's see what other opinions are out there.

[larsoner]

I haven't really worked with textures so I don't have any ideas here

[campagnola]

Should be a little more clever about finding the data; maybe:
data_path = os.path.join(os.path.dirname(__file__), 'data')

[kmuehlbauer]

@asnt I would suggest to put the data into http://github.com/vispy/demo-data/. We can retrieve it from there. Have a look how this is done in https://github.com/vispy/vispy/blob/master/examples/basics/scene/volume.py

[asnt]

@kmuehlbauer Thank you. I'll do this change.

[campagnola]

The indexing issue could be fixed by adding an event like MeshVisual.vertices_changed and having the filter connect when attached. Definitely agree that there should be setters for texture and texcoords.

[asnt]

@campagnola Thank you for the tips. In 33db5c8, MeshVisual._update_data() raises a data_updated event that allows the filter to adapt the texcoords indexing. Does that look OK or should the event be more specific like indexing_changed?

What is reason behind having the indexing of vertices by faces? In the code there is this comment:

# It might actually be slower to prefer the indexed='faces' mode.
# It certainly adds some complexity, and I'm not sure what the
# benefits are, or if they justify this additional complexity.

Moreover, from my experience implementing this filter, it seems that all meshes are drawn with face-indexed vertices after the first few draw events. (I'd have to double-check this, though.)
So if face-indexing is indeed less efficient, does that have an impact on displaying larger meshes? Should we not adopt another strategy?
Are the face-indexed vertices used for picking visuals? For other things?

[kmuehlbauer]

Moreover, from my experience implementing this filter, it seems that all meshes are drawn with face-indexed vertices after the first few draw events. (I'd have to double-check this, though.)
So if face-indexing is indeed less efficient, does that have an impact on displaying larger meshes? Should we not adopt another strategy?
Are the face-indexed vertices used for picking visuals? For other things?

Unfortunately I can't say much on that, so we would need @campagnola's expertise here. We should implement the easiest or fastest approach, sometimes this is the same.

[djhoese]

Travis tests are passing. Something is messed up with the integration so it isn't indicating that on the PR.

@kmuehlbauer If you can look at the example script and try to figure out what it is supposed to do that'd be great.

[kmuehlbauer]

@djhoese I'll have a closer look the next day.

[asnt]

Ah I see transform.map is always using initial_light_dir. Changing it to use event.pos gives a better result but I'm still not sure that's what you were hoping for. Let me know and I'll do what I can to fix it.

I fixed the input to imap (the translation component had to be 0 because only interested in the rotation of the camera, not the global shift).

For the record, the idea is to simulate a "headlamp" so that the mesh is always illuminated from the viewer's perspective. So the mesh.light_dir must be in sync with the camera orientation and direction.
view.camera.transform contains the current camera transform of the scene.
The intention is thus:

• to obtain the current camera orientation when it changes
• and apply it to the mesh.light_dir.

I get the current orientation by applying transform.map to the initial camera orientation (maybe confusingly named initial_light_dir). Not sure if there's a better/simpler/clearer solution.

BTW, maybe there is a more appropriate event than mouse_move that would be triggered when the camera changes without mouse interaction.

[asnt]

There is a relatively new vispy.visuals.filters.base_filter:Filter base class that this new filter should be based on. I can probably update the code to handle that if you don't have time @asnt.

I won't have the possibility immediately so feel free to adapt it. Otherwise, I'll definitely have a look if it's still on later on. Just ping me for any questions.

[djhoese]

Nice, thanks. That makes sense. I updated it now to use the changed event from the transform. This should be the transform that is updated by the camera so it shouldn't do updates when not moving the camera and should keep the same amount of updates when moving the camera.

I'll look at updating the filter tonight if I have time.

[djhoese]

I think I've updated the TextureFilter to make this work. I'm sure there is a better way to implement this that is more flexible, but I'm not sure this PR needs to be the place for it to become that flexible. @asnt @kmuehlbauer reviews are appreciated.

[kmuehlbauer]

@djhoese Just a quick note, the example works smoothly. I've skimmed the code briefly and found nothing suspicious. So I can't immediately see, why this shouldn't go in, finally!

Thanks @asnt! This is a nice enhancement!

[djhoese]

Aw I liked the green cow 😉

White sounds good too I guess. @asnt Do you see anything else that needs to change with this? Ready to merge when the tests pass?

[asnt]

Aw I liked the green cow wink

:-) It was fun, indeed, but I thought for the demo it was better to let the texture intact. Not sure if really relevant, actually. Feel free to reset.

White sounds good too I guess. @asnt Do you see anything else that needs to change with this? Ready to merge when the tests pass?

@djhoese This looks all good to me! Thanks for your support! @djhoese @kmuehlbauer @larsoner @campagnola

[asnt]

Aw I liked the green cow wink

I think it should be clear from the demo how to display a texture without altering its colour. Maybe a comment in the code would make this explicit: "white" = unaltered, "non-white" = blending of colour and texture.

[djhoese]

Agreed that it's good for the example to show this. If you want to update the docs to mention this and have it make sense then go for it.

[djhoese]

I added a little bit of documentation about the color. If we need more then let's add it in another PR. I've wasted enough of your time not merging these PRs.

[os-gabe]

Wow this works great. It would be awesome to get this into a released version.

[djhoese]

The plan is for 0.7 this week.

[os-gabe]

Oh, perfect. Great news. Thanks