-
-
Notifications
You must be signed in to change notification settings - Fork 220
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
Adding preliminary Quake1 MDL plugin. #1441
base: master
Are you sure you want to change the base?
Conversation
I'm writing an extra comment to make everyone extra sure that this PR is not ready for merging with |
|
||
std::vector<unsigned char> buffer(std::istreambuf_iterator<char>(inputStream), {}); | ||
|
||
//??? What's a “splat”??? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In vtkF3DSplatReader
, the splats are actually just points with extra information (radius, color, etc...) because they will be drawn as ellipsoids later by F3D.
In your case, you just want to have points and triangles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, so does that means that I can remove all SetNumberOfTuples()
safely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, take a look at my other comment, you can just remove all the VTK buffers you have created for now.
size_t nbSplats = buffer.size() / splatSize; | ||
|
||
// identity ("IDPO"): 4 chars (4 bytes) | ||
vtkNew<vtkUnsignedCharArray> IDPOArray; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you don't really need to create VTK arrays for internal data, it's only useful for data that will be outputted down the pipeline.
I suggest we start by supporting a small subset for now (i.e. geometry only without texturing nor animation)
For now, you can just use the code in the section "Reading a MDL file" of this page: http://tfc.duke.free.fr/coding/mdl-specs-en.html
Later it would be better to clean that up to modern C++ style for readability.
When this is read, you can take a look at the section "Rendering the model", the idea is just to take the frame index 0 (we'll extend it later to support animation), and just transform vertices to a vtkFloatArray
(3 components, num_verts
tuples) and a vtkIdTypeArray
for triangles (1 component, 4*num_tris
tuples)
If you're wondering about why "4*num_tris
", that's because the buffer is containing the size of the cell before the indices to support quads or polygons with more vertices. Take a look at this code, you should do something similar.
Of course! this is fine! You can flag your PR as a draft to make that more clear if you want :) |
hey @malespiaut , need any help moving forward ? |
Hello @mwestphal, |
This pull request is here to let everyone have a clearer look at the current effort to implement support for Quake1 MDL files; and hopefully help more skilled developpers to give their insight and contribute.
This is tied to issue #1310.