jK wrote:Be straightforward and say the truth that you just want to drop the s3o/3do loading in the engine cause you want to use assimps structs&classes directly
I'm pretty sure I said exactly that, why would you say I'm not telling the truth?
jK wrote:THIS IS FAIL. assimp is everything else than a `stable` library, its ABI can change with each revision! Also there is none advantage in doing so, it won't be faster, it won't be cleaner.
I'm planning to write for assimps API, not ABI. I only said that earlier because I was discussing implementing a parser INSIDE assimp (an assimp plugin) for TA/Spring formats. I don't think that's a good idea either. The API may not be set in concrete either however since I'm linking and building assimp as a static library in Spring it's only going to change if and when we decide to pull updates. If we can't change our code then we don't upgrade assimp and nothing breaks.
jK wrote:The modelparser code is already modularized and can be extended easily, it's the render code that needs to be modularized.
Yes and yes. I've already finished 95% of the loader and it works for all formats bar MD5. It's primarily the renderer I want to change now.
The issue is that in the current design the renderer calls back into the parser class from drawing classes. Apart from being an obvious misuse of the term "parser" this is part of the 3 rendering paths I was discussing. Each time you add a new parser class you're also adding an additional rendering path which requires a lot of state changes if the formats are handled differently (which they are).
jK wrote:0.2.) Isn't this contrary to your main reason of adding assimp support? I thought it was the aim to remove any spring-specific tools to create a model.
Yes it is, however I can see no way around the fact that spring requires additional metadata and some people will want a GUI to create the data in. You or I would be comfortable setting properties in a Lua file, even if that meant trial and error. I am focusing on UpSpring primarily because it already exists and does most of the things required to convert S3O / 3DO to assimp-supported formats.
SpliFF wrote:1.)several hundred lines of 3DO / S3O specific code can be removed entirely.
This is true, though they aren't all in the rendering code. Most is in the parsers and texture handlers. I can easily find nearly 1000 lines between 3DOTextureHandler, S3OTextureHandler, 3DOParser and S3OParser that would become obsolete under a new system.
SpliFF wrote:2.)Optimisations that couldn't be done because they'd break the 3DO pipeline can be added.
This is true. Assimp will even do many of them for free on loading. I can't speak so much for the renderer though but it certainly looks that way. Does the 3DO pipeline even support shaders?
SpliFF wrote:10.) A *LOT* of GL state changes can be removed. The engine will no longer need to setup and cleanup different contexts for rendering 3DO, S3O and assimp.
jK wrote:3do and s3o share already 99.9% of their GL states, so this just untrue.
This IS true. There are at least 15 format-specific setup/cleanup functions (not including conditional branches inside functions) I could point to right now and some of them appear to be called quite regularly. I don't know the actual performance costs but that doesn't make the statement untrue. It could be an exaggeration though.
Code: Select all
void SetupFor3DO() const;
void CleanUp3DO() const;
void DrawUnit(CUnit*);
void DrawUnitS3O(CUnit*);
void DrawUnitAss(CUnit*);
void DrawQuedS3O(void);
void DrawQuedAss(void);
void SetupBasicS3OTexture0(void) const;
void SetupBasicS3OTexture1(void) const;
void CleanupBasicS3OTexture1(void) const;
void CleanupBasicS3OTexture0(void) const;
SetupOpaque3DO;
ResetOpaque3DO;
SetupAlphaS3O;
ResetAlphaS3O;
SetupShadowAss;
ResetShadowAss;
jK wrote:2.) 99.99999999% of all conditional statements are linked to IsInView, ShowHealthbar, IsLua...
Now you're exaggerating. The conditional statements appear frequently in the UnitDrawer class.
jK wrote:So you can't optimize anything there, yeah you would even need to add more if-flags, cuz you have to dynamically switch textures bindings and other GL states for an assimp based renderer.
I'm removing format-specific conditionals. Assimp scenes, as seen by the renderer (particularly after post-processing) are all 1 format. The materials and textures are 1 format as well. If assimp needs conditionals it needs them regardless of 3DO / S3O support so the argument is bogus anyway.
jK wrote:5.) What is a texture map/queue and why does the current one needs 3 of them?
All models to be rendered are put in a map based on their texture type and texture id so all models that share a texture are rendered together (to avoid flogging the texture cache). Because s3o / 3do and assimp textures are handled differently 3 maps are needed instead of 1 and the texture and state change more often than required. Performance impact unknown, but 1 queue is still less code and less state changes.
jK wrote:8.) Adding a Lua interface for the current modelparser is easy, and doesn't need any changes in it.
Does not mean it can't be made easier and cleaner. Currently some model rendering (like features) goes through the UnitDrawer class which is just confusing and unclean.
jK wrote:11.) This is one reason why you can't ever transform 3do in a diff format (easily), 3do sometimes define faces with 1 or 2 vertices. You can't triangulate those! and the engine needs those to calculate the facing dir of a piece.
I'll look into that. If there are models that don't convert well I'll look into workarounds but I doubt this will be a difficult issue to solve.
jK wrote:As already said I plan a model rendering rewrite already for a very long time myself and got a lot of ideas how it should be realized, so I might be a bit harsh. Still I think your motives are wrong. Not to forget that you didn't talked anything how your renderer should look like, you just talked about of much fail the current one is to your opinion.
I accept I've only outlined goals, rather than strategies. The purpose of the OP was to discuss those goals. I have some flowcharts I've been working on to organise the classes better but they are not complete yet. Happy to share when they are. Happy to hear your views (or find your old threads).
Finally, I NEVER said the engine was FAIL or that the people who wrote it suck. The current system is the result of a long process of gradual evolution which has clearly changed directions several times in its history. It it is the result of limitations in hardware, opengl, model formats, old standards, old problems, etc that may no longer exist. In short just because it needs an overhaul is no offense to the people who wrote it. I couldn't even begin to rewrite it or implement the loader without the work they did.
Having said that even the authors admit everything is not as it should be, which is apparent when you see things like this in the code:
Code: Select all
//FIXME redundant struct!?
struct LocalModel {...}
//FIXME: abstract this too
//s3o
void FindMinMax(SS3O *object);
//3do
void FindCenter(S3DO* object);
// should not be here
void DrawS3O(void);
int textureType; // FIXME MAKE S3O ONLY
etc...