Add support for backface properties

[adeak]

This builds on top of #2941 for its Property feature. For now this PR has feat/add_composite as the base branch for CI, so before this is unset as draft the base branch has to be changed to main! This is otherwise ready for review.

The new feature itself is fairly minor, it just exposes backface properties via calls to plotter.add_mesh(). I may have gone a bit overboard and implemented the main results of a differential geometry paper in order to have a nice animation:

TODO:

• [x] change PR base to main once #2941 is merged -> draft until then
• [x] move eversion example to advanced examples section

~~maybe TODO:~~ future work:

• [ ] check if textures play nicely on the backface
• [ ] check why enabling PBR makes the backfaces disappear; perhaps backface culling is enabled automatically

[akaszynski]

Guess we need to merge #2941. I'll work on that this evening.

Been playing around with cadquery and pyvista:

[codecov[bot]]

Codecov Report

Merging #3135 (84a8d6e) into main (3a8db48) will increase coverage by 0.04%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3135      +/-   ##
==========================================
+ Coverage   95.08%   95.13%   +0.04%     
==========================================
  Files          83       83              
  Lines       18450    18468      +18     
==========================================
+ Hits        17544    17569      +25     
+ Misses        906      899       -7     

[adeak]

I didn't expect history to be mucked up this bad by my choice to open this PR on top of a feature branch first. Let me know if I should reopen a new PR with my cherrypicked changes for a less horrible merge commit message at the end.

[adeak]

Have the sides of our animations always looked this jagged? Or did we change something about anti-aliasing? Or is it just the small image size I used to reduce the size of the GIF?

[MatthewFlamm]

Have the sides of our animations always looked this jagged? Or did we change something about anti-aliasing? Or is it just the small image size I used to reduce the size of the GIF?

Could this be caused by #3168 changing the default image size for "fast builds"?

[akaszynski]

Have the sides of our animations always looked this jagged? Or did we change something about anti-aliasing? Or is it just the small image size I used to reduce the size of the GIF?

Could this be caused by #3168 changing the default image size for "fast builds"?

Yes. Reducing the image size improves the build speed at the expense of quality. Tradeoffs...

[adeak]

I've run a full build since, and I see the same behaviour. In fact just running the example in python gives me similar aliasing, although this is harder to notice on the default grey background:

Compare with my original gif from the PR comment:

Seems like the render passes PR changed the default meaning of global_theme.anti_aliasing = None. If I manually set this to 'ssaa' it looks nice again.

[adeak]

Considering my workload for the near future I don't think I'll be able to add to this for a while. It would be nice to figure out if we can do anything about PBR here, but these can go in follow-up PRs. So this is final for now in case anyone wants to take another look.

Also thanks @akaszynski for the merge.

[akaszynski]

Also thanks @akaszynski for the merge.

I'll try to take a look at the PBR issue.

[MatthewFlamm]

I read through this and I cannot really meaningfully comment on the back face property implementation, although it looks good from a naive read through. The math/implementation for sphere eversion is impressive! I also cannot meaningfully comment on it. It really showcases how complex visualizations can be accomplished with only a little bit of PyVista code. I wonder if it should go in "advanced" or perhaps a new category "showcase" (or something like this) as the math might scare off users at first glance in the plot section.

[adeak]

It really showcases how complex visualizations can be accomplished with only a little bit of PyVista code. I wonder if it should go in "advanced" or perhaps a new category "showcase" (or something like this) as the math might scare off users at first glance in the plot section.

Very good point, thanks @MatthewFlamm! It did cross my mind at one point when I last looked at https://github.com/pyvista/pyvista/pull/2994, reminding me of the "advanced" section :smile: But then I forgot to come back to this thought. I agree that this is better tucked away in advanced/ for now. Hopefully the basic backface example makes this feature discoverable enough.

[adeak]

I've moved the sphere eversion example into the advanced section. I've also updated the simple example a bit to demonstrate the new Actor class and its Actor.backface_prop property, and ended up doing some non-trivial changes while fixing numpydoc warnings. For instance, I changed all mapper types to pass the theme as a keyword parameter as that seems a bit more robust, and I defined a default of theme=None in the ABC because mypy complained that type(self)() needs a positional arg (and that's right). Please review the new changes carefully.

I'm also still getting the same warning twice:

WARNING: [numpydoc] Validation warnings while processing docstring for 'pyvista.CompositePolyDataMapper.copy':
  RT01: No Returns section found

WARNING: [numpydoc] Validation warnings while processing docstring for 'pyvista.DataSetMapper.copy':
  RT01: No Returns section found

I think I solved this by hinting the return type of _BaseMapper.copy(), but I'll have to do a full doc build to make sure of that (and that takes figuratively literally forever for some reason).

[adeak]

I think I solved this by hinting the return type of _BaseMapper.copy(), but I'll have to do a full doc build to make sure of that (and that takes figuratively literally forever for some reason).

Well that wasn't it. What I don't get is why all the other methods with no Returns section doesn't raise the same validation errors. Almost all of them are properties, could that be it? The one other method that's not a property is https://github.com/pyvista/pyvista/blob/314e6b1014207b4be9297593e90ec97bbccd67bf/pyvista/plotting/mapper.py#L278-L280

Perhaps the barely existent docstring and the lack of return statement makes this other method exempt from validation...

My only issue is that the only thing I can put in the Returns section of _BaseMapper.copy() is _BaseMapper. Is it not an issue to have a private ABC show up in the rendered docs? Or should I wrap the copy() method in child classes and copy the docstring?

[tkoyama010]

Except for the topics that have yet to be resolved.

[akaszynski]

I've raised a question regarding disappearing backfaces here, but I think this is a fundamental limitation of VTK and have made a note of it in this PR.

I also addressed textures, which I'm happy to report still works with backface properties.

[akaszynski]

Thanks for working on this, and I'd love to have this out for the next release.