STORM-IRIT / Radium-Engine

Research 3D Engine for rendering, animation and processing
https://storm-irit.github.io/Radium-Engine/
Apache License 2.0
100 stars 50 forks source link

Update texture data #982

Closed hiergaut closed 1 year ago

hiergaut commented 2 years ago

Check if you branch history is PR compatible

UPDATE the form below to describe your PR.

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

codecov[bot] commented 2 years ago

Codecov Report

Merging #982 (3a73540) into release-candidate (6bdcc74) will decrease coverage by 0.08%. The diff coverage is 26.00%.

:exclamation: Current head 3a73540 differs from pull request most recent head d471254. Consider uploading reports for the commit d471254 to get more accurate results

@@                  Coverage Diff                  @@
##           release-candidate     #982      +/-   ##
=====================================================
- Coverage              43.99%   43.90%   -0.09%     
=====================================================
  Files                    335      335              
  Lines                  22364    22408      +44     
=====================================================
  Hits                    9838     9838              
- Misses                 12526    12570      +44     
Impacted Files Coverage Δ
src/Engine/Data/Texture.hpp 50.00% <0.00%> (-5.56%) :arrow_down:
src/Engine/RadiumEngine.cpp 62.29% <0.00%> (-1.20%) :arrow_down:
src/Engine/RadiumEngine.hpp 100.00% <ø> (ø)
src/Engine/Rendering/Renderer.cpp 52.19% <ø> (ø)
src/Gui/BaseApplication.cpp 0.00% <0.00%> (ø)
src/Core/Tasks/TaskQueue.cpp 27.05% <8.33%> (-4.28%) :arrow_down:
src/Engine/Data/Texture.cpp 26.77% <35.29%> (-1.54%) :arrow_down:
src/Core/Asset/Camera.cpp 96.80% <0.00%> (ø)
src/Engine/Data/TextureManager.cpp 6.66% <0.00%> (ø)
src/Engine/Data/EnvironmentTexture.cpp 72.88% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

hiergaut commented 2 years ago

Thanks for your feedback, I'll implement the mentioned points probably in the evening.

hiergaut commented 2 years ago

Thanks for your feedback, I have added the missing comments now.

nmellado commented 2 years ago

@hiergaut Can you please add tests to this PR, to cover the new classes added (can be inspired from the example). Current coverage change is -2%.

hiergaut commented 2 years ago

I have a serious problem, I can not rebase, there is a conflict that I do not understand the origin. I only added a test, I do not understand why git is so angry with me. Could someone help me to solve this problem together to solve this problem alone in the future ?

dlyr commented 1 year ago

New proposal using taskQueue to handle gpuTasks, by the engine. This new proposal misses some doc and polish. If ok with the idea, and after accepting this PR, some work can be done to move other gpu sync task to the engine task queue.

hiergaut commented 1 year ago

Please, can you make both examples ("TexturedQuad", "TexturedQuadDynamic") work without having a segmentation fault during the registerTask with the new implemented method (TaskQueue) ? I would like to do some comparison tests with the previous implem. The "TexturedQuadDynamic" example being very representative of what I can have in my application (raw data stream on different threads).

dlyr commented 1 year ago

@hiergaut my bad I forgot one file in my last commit, now should be ok, if not please report where the segfault occurs.

nmellado commented 1 year ago

@hiergaut are you fine with this proposal ? Can you confirm the final version works for you ?

hiergaut commented 1 year ago

It works on my side @nmellado, sorry for the late reply, I didn't remember receiving a notification email for your request, or I deleted the email by mistake.