Improve texture management

[hiergaut]

Check if you branch history is PR compatible

• Your branch need to be up to date with origin/master AND to have linear history (i.e. no merge commit).
• Update your git repository git fetch origin if origin is this remote
• Check with the script provided in scripts/is-history-pr-compatible.sh
• You must use clang-format style These checks are enforced by github workflow actions Please refer to the corresponding log in case of failure

UPDATE the form below to describe your PR.

• Please check if the PR fulfills these requirements

• [x] The commit message follows our guidelines
• [x] Tests for the changes have been added (for bug fixes / features)
• [x] Docs have been added / updated (for bug fixes / features)

Be aware that the PR request cannot be accepted if it doesn't pass the Continuous Integration tests.

• What kind of change does this PR introduce? (Bug fix, feature, docs update, ...) Improve code, new feature, new docs

• What is the current behavior? (You can also link to an open issue here) I have problems when using the texture object alone without using the texture manager. The texture pointer management does not take into account the fact that the data is generated on a separate thread, in fact I want to modify the texel data on a different thread than the one that renders the texture. I would like to retrieve a texture by its name with the texture manager and not the textureParameters. I would like to guarantee that textureParameters of a texture is constant during the life of a texture and not give the possibility for a user to modify this attribute there giving to the texture an incoherent state. I would like the name of a texture not to be in the textureParameters but rather in the textureManager because in fact the creation of a texture does not require a name, only the textureManager needs this information. I have encountered other problems with texture management, my current goal is to display a stream on a texture, so I'll contribute to Radium in order to have the possibility of using an efficient solution.

• What is the new behavior (if this is a feature change)? I can update a texture in a separate thread the way I wanted to. To do this I added an Image class (Core) containing only the information specific to an image (resolution, format, image data). This class has been implemented with the OpenImageIO library features, it also has been implemented in another way. Today an image can be attached from an engine texture. This allows to not modify the behavior of the existing code as well as the subjacent codes (software using Radium for rendering, and using massively textures). This PR does not modify the behavior of the textures on the engine side. On the other hand, attaching an image to a texture implies a slight overhead when using these textures during the bind process (which is done several times per frame for each displayed texture). That's why it would be necessary to consider a texture engine as an image (inheritance) and therefore to manage not a texture manager but rather an image manager in the Core side. Otherwise another solution is to build a texture attached to an image, the texture is a view of the image with its own parameter OpenGL (Wrap, mignify, magnify). Several textures can watch the same image for example, with different parameters. But changing the way to build a texture means changing all the calling codes. My opinion is this PR is not ready for a merge. Before the merged, we should measure the impact of changing the constructor of a texture. In the Radium code we have to change some lines in (ForwardRenderer, Renderer), and discuss about the modification impacts. The problem comes rather from sub-adjacent applications like Painty or others that I don't know yet.

• Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

• Other information:

[MathiasPaulin]

Thanks to think about generalizing texture management. BTW, this PR makes me think you go toward more advanced CPU Texture management than we have right now. If this is the case, and as the needs should also helps other developers, perhaps we should think about using openImageIO (https://github.com/OpenImageIO/oiio) as base for our image/texture manager instead of maintaining and refactoring code, that was not developed for general texture management and will quickly become sub-optimal. I'll open an issue to discuss this.

[dlyr]

Reading this PR make me think we can move texels linearization out of Texture, but only in TextureManager. This kind of transformation should be on eventually on Core side (when Core have full Image data management, Engine should only take care of the GPU representation of the texture, agnostic to the actual data semantic)

[hiergaut]

Oiio depends of Boost lib. Should I consider boost is a lib known by all our build machines or should I compile the Boost source and therefore add this new dependency to Radium ?

[hiergaut]

Why not place the OpenImageIO lib in IO instead of Core?

[codecov[bot]]

Codecov Report

Merging #901 (ab91285) into release-candidate (171d8cb) will increase coverage by 0.05%. The diff coverage is 29.23%.

:exclamation: Current head ab91285 differs from pull request most recent head 9dce257. Consider uploading reports for the commit 9dce257 to get more accurate results

@@                  Coverage Diff                  @@
##           release-candidate     #901      +/-   ##
=====================================================
+ Coverage              27.75%   27.81%   +0.05%     
=====================================================
  Files                    337      342       +5     
  Lines                  22125    22711     +586     
=====================================================
+ Hits                    6140     6316     +176     
- Misses                 15985    16395     +410     

Impacted Files	Coverage Δ
src/Engine/Data/Texture.cpp	1.64% <0.00%> (-0.16%)	:arrow_down:
tests/ExampleApps/TexturedQuad/main.cpp	0.00% <0.00%> (ø)
src/Core/Asset/ImageSpec.cpp	64.70% <64.70%> (ø)
src/Core/Asset/Image.cpp	75.40% <75.40%> (ø)
src/Core/Asset/ImageSpec.hpp	100.00% <100.00%> (ø)
src/Engine/Data/Texture.hpp	42.85% <100.00%> (+42.85%)	:arrow_up:
tests/unittest/Core/image.cpp	100.00% <100.00%> (ø)
tests/unittest/Engine/texture.cpp	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6f2cc6c...9dce257. Read the comment docs.

[hiergaut]

I agree to talk about it, there are several ways to design this.

[hiergaut]

During the writing of this PR, I wrote a test case to validate the features I wanted to have for my application (40Hz texture update, resizing, update of a local transformation matrix to 60Hz or if possible 120Hz). It turns out that this test shows some artifacts, one of which does not depend on the implementation of this PR. It is a matter of moving an object at 120Hz, for example with a single thread I decide to update the local transformation matrix of an object to simulate a position sensor. In this test I simply perform a rotation around a main axis of a white quad without texture. And I observe an artifact (see first image below). I suppose that updating the local transformation matrix is not limited by the renderer refresh rate, indeed if I make several modifications of this matrix per frame, the renderer will take the most recent modification. I also detected another artifact on a texture that changes from white to black loop at 30Hz, I was expecting to see gray but it's not the case (see last image below). This artifact may be related to my implementation though. So before closing this PR, I ask you to take into account what is said here in order to be able to correct these problems in a future implementation of an Image class. Thanks for reading.

Rotate quad at 120Hz

Blink tetxure 30HZ (white <-> black)

[nmellado]

Superseded by #982