First Stage of the KDE Rendering API

[JoaoDell]

Hello, this is a draft PR to start discussing the details of the KDE rendering API for FURY. Talking with @devmessias, some points arised regarding how this API should be done:

The way this kind of render work is different from other actors because it deals with post-processing effects. This means this render needs first to render the scene into a renderer, capture that rendered scene and pass it as a texture to another actor inside another scene assigned to another window, so the effect can be applied. This involves offscreen renderers and an implicit callback function declared when that actor is created to do that job of calling the offscreen rendering, so it is kinda more complex than a simple actor and a standard pipeline. Having that in mind, we thought in two options:

1. Create a simple actor that deals with one or more KDE renders.
2. Create a class to manage KDE actors, or maybe in a more general way, a class that manages post-processing effects.

The first one may be a more simple and user-friendly approach at first, but may present some issues regarding optimization and needs to be carefully thought. For example, it needs to be decided whether the user will pass an offscreen manager together with the onscreen one, or if this will be created inside the function. Leaving that to the user may difficult the actor usage as several pieces would need to be put together by them to work, but creating the offscreen manager inside the function may slow the rendering if the user decides to create multiple KDE actors and not multiple KDE renders inside only one KDE actor.

The second one considers the possibility of creation of another future feature in FURY, which is the post-processing feature, as the implementation of capturing the screen to a texture and pass it to be rendered in a billboard can have more general usages than only a KDE render. It also may deal with the problems pointed above internally, as a post-processing manager may reduce the probability of several managers being created, resulting in faster processing time. However, it needs to be carefully thought as well, as it needs to be simple and straightforward, existing only to avoid conflicts and diminish user misusage, even though the latter can never be extinguished, I think.

Having that in mind, we decided to take the second path, as it seems to solve issues that may arise. However, it would be important to have other opinions on that, so feel free to use this PR as a place to do that 👍

[UPDATE]

The first draft of the API is done and ready for review. Below, a brief explanation of how this works:

The main programs here are:

• effect_manager.py
• viz_kde_render.py

Those two are where the implementation of the API is done and tested, so you should be looking into that when reviewing then. Also, there is the testing script for the API:

• test_effect_manager.py

And the test for the kde actor under:

• test_actors.py (I have put it down the end)

There are also new shaders to be used:

• *_distribution.glsl
• color_mapping.glsl

The first ones are different kernels for the kde calculations, and the last one is the function to map intensities to a color map passed by the user. I have also altered some files:

• shaders/__init__.py
• shaders/base.py
• _valid_examples.toml
• window.py

With some minor changes necessary for this API to work. In case anyone got any questions, feel free to tell me down below so we can discuss about all of this. I got some doubts regarding some implementation choices, like, for example, if the "effects" name won't cause any confusion as FURY already has shader_apply_effects() function, so I would like to hear from that as well.

An important thing to say is that there are some important details missing from this API:

• This is not a KDE rendering, exactly. It still lacks the division by the number of points rendered so the density can be calculated, however this is a more complicated detail as it steps on the problem of not having enough precision as it lacks a float framebuffer, being replaced by an unsigned integer one. To understand that, you can check my week 9 blogpost on that on GSoC: Week 9 Blogpost #831. Bruno and Filipi oriented me to leave that for a later PR.
• Talking with Filipi, he hinted me it would be good to have a slider for the intensities. I tried that, however it seems to be crashing the whole program, but I didn't understand exactly why yet, so we decided to leave that for later as well.

[codecov]

Codecov Report

Merging #826 (51ae7a6) into master (2551b7f) will increase coverage by 0.04%.
Report is 103 commits behind head on master.
The diff coverage is 87.06%.

❗ Current head 51ae7a6 differs from pull request most recent head ac2ce0d. Consider uploading reports for the commit ac2ce0d to get more accurate results

@@            Coverage Diff             @@
##           master     #826      +/-   ##
==========================================
+ Coverage   84.30%   84.35%   +0.04%     
==========================================
  Files          44       45       +1     
  Lines       10353    10467     +114     
  Branches     1406     1413       +7     
==========================================
+ Hits         8728     8829     +101     
- Misses       1255     1260       +5     
- Partials      370      378       +8     

Files Changed	Coverage Δ
fury/shaders/__init__.py	100.00% <ø> (ø)
fury/window.py	78.15% <50.00%> (+0.08%)	⬆️
fury/actors/effect_manager.py	88.07% <88.07%> (ø)
fury/shaders/base.py	91.17% <100.00%> (+0.26%)	⬆️

[JoaoDell]

Hello everyone, after some weeks, I think this PR is ready for its first review. I have updated its description with some information that may help you with that.

[JoaoDell]

Also, I just noticed I may need to replace sigma name by bandwidth, to be accurate with the KDE literature:https://en.wikipedia.org/wiki/Kernel_density_estimation

[tvcastillod]

Hi @JoaoDell,
I know you have to do some refactoring, but I left a few comments on the current implementation for you to take into account for the changes you are working on.

[tvcastillod]

Not sure if this should be tested here, as you don't have a kde actor definition on actor.py, maybe take a look at actors/peak.py and peak in actor.py, to see how you could adjust that.

[JoaoDell]

Ok, I will be looking into that 👍

[skoudoro]

@ganimtron-10, can you look and review this PR?

[ganimtron-10]

Hey @JoaoDell ,
Please take a look at my below comments.

[ganimtron-10]

you can directly use size

[JoaoDell]

Hey @skoudoro, @filipinascimento and @devmessias, just finished rewriting the tests and cleaning some details on the refactor of the API, so you can review now 👍

[ganimtron-10]

While giving a brief look I found these below comments! PTAL.

[filipinascimento]

I think the PR is solid and it seems to work fine across all the platforms. I've just a couple comments listed in this review. Please make a new PR for the other changes and for the UI example. We also need to showcase these functionalities with a killer application. Lets try to converge in the next GSOC meeting.