Planned rendering engine rewrite

[Argh]

From what I can tell, upspring currently chokes miserably when trying to load a 3ds. The model comes in as a single piece and is rotated incorrectly.

The model's rotated, because 3DS and OBJ use a different coordinate system- that problem happens with OBJ, too, at least over here- I always have to rotate the model, because "forwards" in Rhino is "down" to UpSpring. This happens in other situations for me, so I don't really care about it.

But 3DS comes in with all uniquely-named meshes separate (if you select option 2). You just literally cut / paste to build your hierarchy from there, and then set up your offsets.

Maybe you've just exporting from Wings without giving the Pieces unique names or something. Or maybe I need to write a tutorial- I thought that was easy, but maybe it's mysterious.

[SpliFF]

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?

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.

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).

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.

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.

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?

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.

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;

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.

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.

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.

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.

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.

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...

[Forboding Angel]

Well to be honest, I think I would be perfectly happy with loading spring and pressing B to get the hitsphere visualized (b doesn't show the unit height tho... could that be added perhaps?) because it would still save me literally hours of work from having to screw around with upspring... I really really hate upspring lol.

[SpliFF]

Ok, Here's a basic outline of what I have in mind for a rewrite of the rendering system.

New Rendering System Proposal (30KB PDF)

Some highlights are:

* A buffer object to abstract displaylists (GL1.0) and VBO/PBOs (GL2.0)
* A state object to better organise common GL setup/cleanup
* A mesh object for general purpose rendering
* An abstract vertex class (currently tied to specific formats)
* More overridden contructors for creating/cloning objects from different data types
* Support for access to bones/lights/animations in models (even though they aren't currently used)
* Features don't call into unit drawing classes.
* Some existing classes and functions renamed for clarity or consistency
* Every class will get a Dump() method. The purpose of this method will be to puke a bunch of debug messages to the log that loosely describe the current state of the object (like python's repr()).

None of this really adds new features. It's not even that different from what we have now. It's more about refactoring and cleanup so future development is easier and the code is easier to read. I will also be writing full doxygen comments for every class, member and method so we can easily generate documentation like this:

http://assimp.sourceforge.net/lib_html/ ... scene.html

In the PDF diagram base classes are red, derived classes are pink. Constructors are named Create just because it's cleaner in the diagram, destructors aren't shown. It's an overview only and shouldn't be considered correct or complete.

The second page shows a bit of pseudo-code to setup and render a couple of units. It's not too far from what the actual C++ source might be like but it's over-simplified and a bit contrived (it's just an example).

The state object could be problematic. I'll decide whether to keep or remove it later after some real-world testing.

[CarRepairer]

Doing it by altering Lua text files would be incredibly slow and cumbersome, and would not be welcomed by non-coders.

I find it dreadful to have to use upspring to set up the hierarchy. Slow and cumbersome is using upspring to cut and paste pieces, make empty nodes, swap things around like an idiot, then for some reason the swapped pieces move way off to lala-land if their origins aren't the same. It's a huge pain. And if the model source gets changed and I need to import it to upspring again, gotta start over!

[AF]

The lua is executed only once on load, and atm is no more complex than what we have in TDF from what I can see

[FLOZi]

The lua is executed only once on load, and atm is no more complex than what we have in TDF from what I can see

Hi, learn to parse thread. kthxbai!

[jcnossen]

I especially like there is a seperate class to handle ass models...
May I also propose a Boundary Object Orientated Bouncing System?

Call me immature, I couldn't resist anymore.. seriously someone needs to rename stuff. People have complained about less obvious words in the source comments... anyone remember Hugh Perkins?

[smoth]

keep that up and team ninja will be calling you.

[AF]

Oh sorry I assumed the blatantly obvious other bits were well... blatantly obvious, sarcastic post was sarcastic

[SpliFF]

I especially like there is a seperate class to handle ass models...
May I also propose a Boundary Object Orientated Bouncing System?

Consider it implemented :)

Call me immature, I couldn't resist anymore.. seriously someone needs to rename stuff. People have complained about less obvious words in the source comments... anyone remember Hugh Perkins?

Sure it wasn't "Hugh Gass"? (say that quickly).

Anyway people complain about lots of things (religions, non-believers, hot days, cold days, offensive words, political correctness) I just stopped caring. If I called the class CGetFuckedYouGreatHairyPrick I'd probably accept there was an issue. The library is called Assimp (short for Asset Importer) and I'm too lazy to type even that. Most sane people would find the name funny. I'm not changing it.

PS. Yeah, I know you're being sarcastic.

[Forboding Angel]

Ass Importer... Hmm, it's got a ring to it. Hopefully the name isn't indicative of code quality ;p

[SpliFF]

I'm tossing up whether animations and bones should be synced or unsynced objects:

synced
pros:
* deformations / animations affect game world / collisions.
* easier to sync with sim-side COB / Lua
* easier to use from synced gadget
cons:
* animations constrained by sim speed (could be a pro?)
* increased chance of desyncs

unsynced
pros:
* animation can be offloaded to GPU, major speedups
* non-physical objects (UI, decoration) easier to animate
* easier access from unsynced Lua widgets/gadgets
* can disable anims on low-spec machines without desync
cons:
* what player sees is different to what's really happening. projectiles may appear to explode before reaching target, objects more likely to clip through each other.
* harder to sync model anims with cob/lua script anims

both
pros:
* more flexibility
cons:
* more code, harder to implement
* mod makers need to know the difference and not get confused

Personally I'm leaning towards unsynced, but I'm open to other opinions.

[imbaczek]

unsynced is better for artists, but how do you want to sync piece positions with animations?

[Neddie]

Unsynced seems better on the whole. Having deformations/animations effect the game directly would be cool, surely, but not practical - and if somebody really wants the appearance of that in a particular situation they can fake it using a gadget. We have enough impractical cool things.

[SpliFF]

unsynced is better for artists, but how do you want to sync piece positions with animations?

Rotate the piece first (synced) then apply the animation to the transformed piece (synced or unsynced). This could mean that the piece is turning left and the anim turning right and they cancel out, but presumably that would be the desired behaviour.

If anims are unsynced COB would only see where the sim thinks the piece is pointing which could result, for example, in projectiles appearing to leave the barrel at the wrong position and angle. However that opens the question of why you animated the barrel in the first place instead of using COB / Lua.

I suppose you might want a situation where the barrel sways, however that's entirely doable in COB already and the muzzle alignment and aiming will still work.

[jK]

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.

11.24.09: We changed the orientation of our quaternions to the most common convention to avoid confusion. However, if you're a previous user of Assimp and you update the library to revisions beyond SVNREV 502, you have to adapt your animation loading code to match the new quaternion orientation.

Also Assimp is pretty young so there can be added a lot of new features + bugfixes in the future, so updates won't be rarely.
_______________________________________________________________________________

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).

So why do you want to drop s3o and 3do then?
_______________________________________________________________________________

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.

and how do you want to create the texture atlas for 3do models? (also I know there is some duplicated code in the 3do and s3o parser and it annoys me too, but this is not a reason to drop their whole support)
_______________________________________________________________________________

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?

... 1. you don't listen 2. you don't even know what the current code does, but you want to replace it with something other ...
_______________________________________________________________________________

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.

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.

All of those functions are called extremely rarely, you don't reduce GL state changes at all. Also 1/3 of those functions is for FFP-support and the 1/3 for lua's unit rendering system (trepan wrote his own on the top of the current one), which can be modularized pretty easily (but should be integrated in the engine render queue with a rewrite of the whole model-renderer).
_______________________________________________________________________________

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.

check the DoDrawUnit it's the most called function in there (so it causes ~99% of the cpu usage):

Code: Select all

if (unit == excludeUnit) {}
if (unit->noDraw) {}
if (camera->InView(unit->drawMidPos, unit->radius + 30)) {
	if ((losStatus & LOS_INLOS) || gu->spectatingFullView) {
		if (drawReflection) {
			if (unit->drawMidPos.y < 0.0f) {}
			if (ground->GetApproximateHeight(zeroPos.x, zeroPos.z) > unit->radius) {}
		} else if (drawRefraction) {
			if (unit->pos.y > 0.0f) {}
		}
		if (DrawAsIcon(*unit, sqDist)) {
		} else {
			if (sqDist > farLength) {
				if (unit->lodCount > 0) {}
				if (unit->model->type == MODELTYPE_S3O) {
					if (unit->isCloaked) {}
				} else {
					if (unit->isCloaked) {}
				}
			}
			if (showHealthBars && (sqDist < (unitDrawDistSqr * 500))) {}
		}
	} else if (losStatus & LOS_PREVLOS) {
		if ((!gameSetup || gameSetup->ghostedBuildings) && !(unit->mobility)) {
			if (!DrawAsIcon(*unit, sqDist)) {
				if (unit->unitDef->decoyDef) {}
				if (model->type==MODELTYPE_S3O) {}
			}
		}
		if (losStatus & LOS_INRADAR) {
			if (!(losStatus & LOS_CONTRADAR)) {
			} else if (unit->isIcon) {
			}
		}
	} else if (losStatus & LOS_INRADAR) {
	}
}

24 if's and only 2 are modeltype related ... (I presume in compiled code it will be even more if-checks)
_______________________________________________________________________________

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.

Still your argument is wrong (-> your motives). And check my next post for a solution to clean those if-clauses up (which btw don't slowdown the cpu, it's just hard to maintain code with that many branches).
_______________________________________________________________________________

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.

You will need more GL state changes!

Code: Select all

SetupModelType1() 
for i=1 to ... do
  DrawUnitType1()
end
CleanupModelType1()
SetupModelType2() 
for i=1 to ... do
  DrawUnitType2()
end
CleanupModelType2()
...

versus

Code: Select all

for i=1 to ... do
  if (lastType != curType) {
     CleanupLastType()
     SetupCurrentType()
     lastType = curType;
  }
  DrawCurrentUnit()
end

Still it's worth to put all models/objects in one queue, but this one needs to be runtime sorted to reduce GL state changes.
_______________________________________________________________________________

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.

Has nothing to do with a Lua interface for the modelparser -> Lua would call the draw functions directly w/o the featurehandler etc. (just the unitdrawer is needed to setup the default render states/shaders - if the lua dev don't want to use custom ones)
_______________________________________________________________________________

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!?
//FIXME: abstract this too
// should not be here
// FIXME MAKE S3O ONLY

etc...

IIRC 3 of those 4 are added by me

[SpliFF]

Ok, I'm not going to respond to each point individually because I've heard your view, and I still don't see the value in maintaining 3 pipelines / formats when 2 aren't needed.

The rest is just arguing over optimisations and penalties we can't easily test. I suppose if the classes were abstracted more cleanly and more consistently (getters/setters for everything, internal differences stored as private members) I might be more inclined to agree - but they aren't - and I don't.

You're quite right that any future system should still be abstract enough to allow new formats, including ones that assimp doesn't support. On the other hand I'm not in a rush to personally implement those classes for 2 formats that have no redeeming features beyond the fact they're still in use. A window of opportunity exists here to move these legacy assets across to formats that are more widely supported by 3D tools outside of our closed community. That window might close if we keep dragging s3o / 3do along behind us until the ONE tool that can create and convert these assets stops working on whatever abomination Microsoft try to pass off as an OS next time around.

IF I fail to provide a working, easy and reliable tool to break our dependence on these formats then feel free to be the first to say you told me so and rally the troops to stop the new engine being merged into the master branch. I won't even fight about it because I don't expect anybody to be happy about having mods with no models.

As it stands now though the work required to create (and use) the necessary converters as a once-off or occasional action appears to pale beside the work required to maintain or create assets, code and tools (aka UpSpring) to support these formats for the next 5-10 years.

Yes I intend to keep UpSpring alive for now (and fix some of its bugs) but ONLY as a stop-gap measure during the transition period to convert old assets to better formats. Once the majority of Spring 0.80+ compatible mods are converted and once better tools/methods exist for setting up heights and radius then UpSpring can die a warriors death knowing it didn't outlive its usefulness like some smelly old guy who talks to cats.

I don't know what else to say. This disagreement, if that's what it is, can only be resolved by me either putting my promises into effect, or failing. It won't be solved in this thread unless somebody puts their hand up to maintain the s3o / 3do code paths in Spring, or modders declare their undying love for these formats, or everyone decides the rewrite should just not happen.

I respect your GL skills a lot, and I'm not ignoring your arguments lightly, but on this occasion I just can't agree. At least I certainly won't volunteer to implement what you're asking.

For better or worse the time has come for S3O and 3DO to die.

[Neddie]

Uh... it would be bad to kill s3o, virtually all our free assets are in s3o and porting them en masse is not trivial. I suppose somebody will say similar of 3do, but I really don't care about all those models.

So, either s3o needs to be supported, or a quick flawless batch converter needs to be included. I understand that it may not appear, from a coding standpoint, really that important but from a content development standpoint s3o is crucial.

[smoth]

For better or worse the time has come for S3O and 3DO to die.

S3o can die if you give me a tool that can convert all my s3o files correctly to whatever the new format is. I am A-OK with that as long as there is something to do the conversion. Otherwise I will have to rebuild 100s of models and that is asking a lot.

also will there be an upspring like too or are you going to further complicate the workflow?