Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New proposal for Engine::Displayable #802

Open
wants to merge 28 commits into
base: release-candidate
Choose a base branch
from

Conversation

nmellado
Copy link
Contributor

  • Please check if the PR fulfills these requirements
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • 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, ...)
    Simplify API for Engine::Displayable and inheriting classes by using Core::MultiIndexGeometry.

  • What is the current behavior? (You can also link to an open issue here)
    Mesh types are statically typed on Core Mesh types.

  • What is the new behavior (if this is a feature change)?
    Take benefit of MultiIndexGeometry to allow dynamic typing of the rendered objects: Engine::GeometryDisplayable will store a collection of geometry layers.
    Expected API (right):
    Image from iOS

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    Will impact client code explicitly working with Engine::Displayable and inhering classes.

  • Other information:

@nmellado nmellado added enhancement Type of issue/PR: enhancement Engine Related to Ra::Engine labels Jul 19, 2021
@nmellado nmellado added this to the Radium v2 - Should have milestone Jul 19, 2021
@codecov
Copy link

codecov bot commented Jul 19, 2021

Codecov Report

Merging #802 (3598b89) into release-candidate (d77ca58) will increase coverage by 0.35%.
The diff coverage is 48.11%.

❗ Current head 3598b89 differs from pull request most recent head 076ec5e. Consider uploading reports for the commit 076ec5e to get more accurate results

@@                  Coverage Diff                  @@
##           release-candidate     #802      +/-   ##
=====================================================
+ Coverage              45.65%   46.01%   +0.35%     
=====================================================
  Files                    312      311       -1     
  Lines                  23060    23101      +41     
=====================================================
+ Hits                   10529    10630     +101     
+ Misses                 12531    12471      -60     
Impacted Files Coverage Δ
src/Core/Animation/LinearBlendSkinning.cpp 0.00% <ø> (ø)
src/Core/Geometry/AbstractGeometry.hpp 66.66% <ø> (ø)
src/Core/Geometry/MeshPrimitives.cpp 33.78% <0.00%> (-1.92%) ⬇️
src/Core/Geometry/MeshPrimitives.hpp 0.00% <ø> (ø)
src/Core/Geometry/TriangleMesh.hpp 80.16% <ø> (ø)
src/Engine/Rendering/ForwardRenderer.cpp 52.28% <0.00%> (+0.01%) ⬆️
src/Engine/Scene/SkinningComponent.cpp 0.00% <0.00%> (ø)
src/Engine/Scene/SkinningComponent.hpp 0.00% <ø> (ø)
src/Engine/Data/Mesh.hpp 55.10% <20.00%> (-14.61%) ⬇️
src/Core/Geometry/IndexedGeometry.hpp 65.25% <39.28%> (-5.86%) ⬇️
... and 5 more

... and 2 files with indirect coverage changes

@nmellado nmellado force-pushed the clean_displayable branch 2 times, most recently from 5a16cbe to fa260e4 Compare July 19, 2021 13:55
@dlyr dlyr force-pushed the clean_displayable branch 2 times, most recently from 757a5e9 to 1e88873 Compare July 20, 2022 09:21
@dlyr dlyr force-pushed the clean_displayable branch 2 times, most recently from d2bf2c5 to 2d148ca Compare September 18, 2022 17:19
@dlyr dlyr added Core Related to Ra::Core WIP Work in Progress and removed Engine Related to Ra::Engine labels Sep 18, 2022
@dlyr dlyr marked this pull request as ready for review September 18, 2022 18:56
src/Core/Geometry/MeshPrimitives.cpp Show resolved Hide resolved
src/Engine/Data/Mesh.hpp Show resolved Hide resolved
src/Engine/Scene/GeometryComponent.hpp Outdated Show resolved Hide resolved
@dlyr
Copy link
Contributor

dlyr commented Dec 14, 2022

note for futur skinning https://cims.nyu.edu/gcl/papers/EG2014-Lighting.pdf

@dlyr dlyr force-pushed the clean_displayable branch 2 times, most recently from cdf9736 to f5df674 Compare April 7, 2023 06:23
dlyr and others added 5 commits April 7, 2023 10:01
[core] Expose LayerKeyHash struct

Allow to reuse layers hash function outside of MultiIndexedGeometry
[engine] Remove unused class IndexedAttribArrayDisplayable

[engine] Prepare classes attributes (do not compile)

Code does not compile as only the attributes have been defined, but the
methods have not been updated.

[core] Mark GeometryIndexLayerBase::size() as const

[engine] define all virtual function for GeometryDisplayable (will be deleted).

When transition will be complete, these accessor might not be necessary.

[engine] GeometryDisplayable updateGL (not tested).

[engine] GeometryDisplayable works, for HelloRadium.

triangulate
dlyr added 22 commits April 7, 2023 10:01
debug type clone

make clone everywhere
clean code with using

forward renderer handle poly and quad layers.
[engine] mesh override getNumVertices for GeometryDisplayable.

remove uneeded export api on template
remove forward old draw wireframe
[engine] Mesh remove debug print.

remove mesh debug message

 print mesh name on invalid layer
tmp disable rotation center skinning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Related to Ra::Core enhancement Type of issue/PR: enhancement WIP Work in Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants