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.

Sign up for GitHub

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

Jump to bottom

Quake MDL feature #1591

Open

Youva wants to merge 67 commits into f3d-app:master from Youva:quake-mdl-feature

Youva commented Aug 25, 2024 •

edited by mwestphal

Loading

Adds class vtkQuakeMDLImporter that reads .MDL files.

The class :

Reads vertices and triangle data from the format to display a model.
Reads a texture and texture coordinates from the format to display onto the model.
Reads animations, stored as an array of vertices in the file, and saves them as a vector of polydata.
Groups animations and displays them.
Adds light sources to the scene.

Fix: #1310

codecov bot commented Sep 24, 2024 •

edited

Loading

Codecov Report

Attention: Patch coverage is 97.35849% with 7 lines in your changes missing coverage. Please review.

Project coverage is 95.76%. Comparing base (13e81fa) to head (4bee8b4).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
plugins/native/module/vtkF3DQuakeMDLImporter.cxx	97.33%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1591      +/-   ##
==========================================
+ Coverage   95.72%   95.76%   +0.04%     
==========================================
  Files         128      130       +2     
  Lines       10293    10558     +265     
==========================================
+ Hits         9853    10111     +258     
- Misses        440      447       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mwestphal reviewed

View reviewed changes

plugins/alembic/module/vtkF3DAlembicReader.h Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/alembic/module/vtkF3DAlembicReader.h.rej Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

style.diff Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/CMakeLists.txt Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.cxx.rej Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.h Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.h Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.h Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/vtkQuakeMDLImporter.h.rej Outdated Show resolved Hide resolved

mwestphal requested changes

View reviewed changes

Contributor

mwestphal left a comment

changes needed

Meakk reviewed

View reviewed changes

Member

Meakk left a comment

Nice work @Youva !!
There is still some clean-up to do, especially memory leaks to fix, but they will appear in the CI when you will add a test.
90% of the job is done, and there's the annoying, yet important, finalization left :)

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

plugins/native/module/vtkQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

plugins/native/module/vtkQuakeMDLImporter.h Outdated Show resolved Hide resolved

Meakk mentioned this pull request

Add support of Quake 1 MDL files. #1310

Open

Contributor

mwestphal commented Nov 1, 2024

@Youva any news on this ?

1 similar comment

Contributor

mwestphal commented Nov 24, 2024

@Youva any news on this ?

Contributor

mwestphal commented Dec 17, 2024

Hi @Youva ! Looks like you are back. First you may want to rebase on the latest master :)

github-actions bot commented Dec 17, 2024

You are modifying libf3d public API! ⚠️Please update bindings accordingly⚠️!
You can find them in their respective directories: python, java, webassembly.

Contributor

mwestphal commented Dec 18, 2024

Looks like your merge did not go as expected. I'd strongly suggest using rebase instead of merge. Let me know if I can help (on discord or here).

Youva force-pushed the quake-mdl-feature branch from 70ec237 to 988186a Compare

December 18, 2024 22:51

Contributor

mwestphal commented Dec 19, 2024

Please resolve discussions you have adressed and let us know if this is ready for review.

mwestphal reviewed

View reviewed changes

plugins/native/configs/config.d/10_native.json Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/Testing/CMakeLists.txt Outdated Show resolved Hide resolved

mwestphal reviewed

View reviewed changes

plugins/native/module/Testing/TestF3DQuakeMDLImporter.cxx Outdated Show resolved Hide resolved

Youva added 27 commits

February 26, 2025 09:23


          Changed push_back to emplace_back

98a0870


          Return bool l. 432

d24665e


          Fix unusedStructMember and containerOutOfBounds

c12920e


          Coding style modification.

7f2d79d


          Added unit test

5cd40bc


          Compiler warning and indentation

d918979

Also changed test .png for quake mdl.


          Formatting

2bef09f


          Added coverage to f3dquakemdlimporter

79953f9


          Coverage, added comments

387d907


          Added asset file for coverage

4cb6286


          Changed pointer cast, formatting.

742908a


          Added constexpr variables for offset calculation

80d5116


          Test coverage for animations

4a1ffc3


          Coverage for animations.

62b1e79


          Added coverage

d5e6fd8


          Update TestQuakeMDL.png

27f82fd


          Changed animation time

559f230


          Added UpdateAtTimeValue

6593b20


          Formatting

f651dbf


          Changed some functions in quakemdlimporter

379edd8

Also changed the config and added some tests.


          Formatting

1258b22


          Changed asset file for .mdl

0057fa9


          Changed baseline for thumbnail test

41e9a6a


          Changed TestDefaultConfigFileQuakeMDL and TestThumbnailConfigFileQuak…

467a64a

…eMDL


          Formatting


          Formatting

9d3f454


          Formatting

4bee8b4

Youva force-pushed the quake-mdl-feature branch from 001a692 to 4bee8b4 Compare

February 26, 2025 14:23

Contributor

mwestphal commented Mar 1, 2025

Is this ready for review @Youva ? :)

Author

Youva commented Mar 1, 2025

I think it's ready for a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet