Hard Light Productions Forums
Modding, Mission Design, and Coding => FS2 Open Coding - The Source Code Project (SCP) => Topic started by: ShivanSpS on November 20, 2019, 03:30:28 pm
-
Hi,
im doing a little detective work trying to figure out this, this is the issue, due to only OpenGL 2.1 and no S3TC a Raspberry PI can only run 3.7.2 and uncrompressed textures.
But in some projects, like Diaspora and Wing Commander Saga attempts to load most models in the tech room ends in a crash whiout futher log info, in fact i only get a "bus error" if i run from a terminal, this is not due to textures as removing the textures also causes the game to crash.
In Diaspora for example, the Vipers load, but the Raptor crashes. On Wing Commander Saga all fighter crash, but two or 3 of the Cruicers do load. In constract, all retail FS2 models load.
As i understand, there were huge changes on 3.8.0 and mods adapted to it, thre is something in the .pof files that could cause this?
-
A list of changes can be found at the bottom of this page. (https://www.hard-light.net/forums/index.php?topic=94988.0)
But I have a suspicion.. what build are you running and/or what date of build are you building from?
-
Just a little bit of correction, i never tried to run mediavps, i really dont think the RPI can handle it, altrought i going to try 3.7.2 probably this weekend.
This is the build im running
https://www.hard-light.net/forums/index.php?topic=89597.0
Compiled from the source code export provided there directly on a RPI4. Its kind of a mess due to autogen not detecting the ARM cpu and generating the makefiles with x86 flags, so i had to change that manually.
As i said, the RPI 4 is limited to that at least to the day that both FSO and the RPI4 supports Vulkan, so it will be a while. There is SOMETHING in the .pofs of Diaspora and WCS (and i guess most of other mods) that crashes that 3.7.2 build with a "bus error", and i know those mods were updated to use FSO 3.8.0.
Other that that im lost, since the retail game can be finished on the RPI.
-
I take it this is a Source Code Project question then, add it's unrelated to the MediaVPs.
-
I dont think it is a code question, i already know that a lot of changes has been done on FSO 3.8.0 onwards, specially on the graphics department.
If FSO 3.8.0 allowed new stuff to be added on the .pofs that could cause a crash on 3.7.2 the people that should know about it are the one that work with the model themselves. And the people doing the mediavps are the one that did the most work on models ever.
There is something on those .pofs that cause the game to crash, if nothing special was added on the pofs from 3.7.2 to 3.8.0 to take advantage of some new feature them it could be something else. And i have to admit that getting a "bus error" like this is strange.
-
Sounds like a memory alignment issue to me, which arm is more finicky about than x86. Either an unaligned data read or accessing a struct that isn't aligned/padded properly. Try running it in gdb and see if that helps to locate the problem area.
-
There have been no changes at all in the pof file format or how it is interpreted between those versions as far as I can recall.
-
Don't know if it helps, but I noticed that you can't open pof files that have been safed with PCS 2.1 in 2.0.3 for some reason.
-
Quote from: taylor on November 20, 2019, 10:46:03 pm
Sounds like a memory alignment issue to me, which arm is more finicky about than x86. Either an unaligned data read or accessing a struct that isn't aligned/padded properly. Try running it in gdb and see if that helps to locate the problem area.
You right, one would think this should be a non-issue today but nope...
Thread 1 "fs2_open_3.7.2_" received signal SIGBUS, Bus error.
point_in_octant (pm=0x24a1770, oct=0x24a33c4, vert=0xac768c16)
at model/modeloctant.cpp:28
28 if ( vert->xyz.x < oct->min.xyz.x ) return 0;
Time to rust out my C skills to see if i can figure this one out, .net makes you soft.
Quick test, just comenting all the ifs in that function and make it to always to return 0 already fixes the issue and make the models load and techroom nd the mission to load as well, im sure im disabling something by doing this, so ill try to make a proper fix., good thing the issue seems to be isolated in that function and those pointers.
Update, the SIGBUS happens on these two calls on that file:
if ( point_in_octant( pm, oct, vp(p+20) ) )
at line 174 and 235, the problem seems to be "vp(p+20)"
I spooke too soon, there are more: Shield impact
Thread 1 "fs2_open_3.7.2_" received signal SIGBUS, Bus error.
fvi_ray_boundingbox (min=0x345db0d, max=0x345db19, p0=0xabc4b4 <Mc_p0>,
pdir=0xabc4d0 <Mc_direction>, hitpt=0xbeffeb0c) at math/fvi.cpp:335
335 if (p0->a1d[i] < min->a1d[i]) {
I fail to understand why none of this happens with the retail game files...
-
As far as the point_in_octant error, I found a potential lead. The call stack leads up to this bit of code:
case OP_SORTNORM: {
int frontlist = w(p+36);
int backlist = w(p+40);
int prelist = w(p+44);
int postlist = w(p+48);
int onlist = w(p+52);
if (prelist) model_octant_find_faces_sub(pm,oct,p+prelist,just_count);
if (backlist) model_octant_find_faces_sub(pm,oct,p+backlist,just_count);
if (onlist) model_octant_find_faces_sub(pm,oct,p+onlist,just_count);
if (frontlist) model_octant_find_faces_sub(pm,oct,p+frontlist,just_count);
if (postlist) model_octant_find_faces_sub(pm,oct,p+postlist,just_count);
See the documentation on bsp_tree here, particularly on sortnorms:
https://wiki.hard-light.net/index.php/BSP_data_structure
Note that these five numbers, which are read from the model file itself, specify byte offsets. In order to be properly aligned, these offsets must be multiples of 4. But there is no enforcement of this requirement in the code, and obviously no enforcement in whichever program exported the model. @ShivanSpS, try adding some logging statements to catch non-compliant offsets.
I suspect a similar problem is happening with the shield mesh.
-
mmm if thats the case this is far more extensive problem that i belived, ill keep looking into it this weekend. Also i think the reason of why this happens is that the kernel is set to fix unaligned access and with the retail files can, and with mods cant for some reason, maybe i should try to disable that just to see what happens, ARMv8-A should be able to deal with unaligned access.
-
I think the simple reason is that the retail model files all have offsets in multiples of 4, and some of the newer models do not have offsets in multiples of 4. The logging statements would confirm this.
-
So i did this ugly thing
int model_octant_find_faces_sub(polymodel * pm, model_octant * oct, void *model_ptr, int just_count )
{
ubyte *p = (ubyte *)model_ptr;
int chunk_type, chunk_size;
chunk_type = w(p);
chunk_size = w(p+4);
mprintf(( "First chunk_type %d\n", chunk_type ));
mprintf(( "First chunk_size %d\n", chunk_size ));
while (chunk_type != OP_EOF) {
switch (chunk_type) {
case OP_DEFPOINTS:
moff_defpoints(p, just_count);
break;
case OP_FLATPOLY: moff_flatpoly(p, pm, oct, just_count ); break;
case OP_TMAPPOLY: moff_tmappoly(p, pm, oct, just_count ); break;
case OP_SORTNORM: {
int frontlist = w(p+36);
int backlist = w(p+40);
int prelist = w(p+44);
int postlist = w(p+48);
int onlist = w(p+52);
if (prelist) model_octant_find_faces_sub(pm,oct,p+prelist,just_count);
if (backlist) model_octant_find_faces_sub(pm,oct,p+backlist,just_count);
if (onlist) model_octant_find_faces_sub(pm,oct,p+onlist,just_count);
if (frontlist) model_octant_find_faces_sub(pm,oct,p+frontlist,just_count);
if (postlist) model_octant_find_faces_sub(pm,oct,p+postlist,just_count);
}
break;
case OP_BOUNDBOX: break;
default:
mprintf(( "Bad chunk type %d, len=%d in model_octant_find_faces_sub\n", chunk_type, chunk_size ));
Int3(); // Bad chunk type!
return 0;
}
p += chunk_size;
chunk_type = w(p);
chunk_size = w(p+4);
mprintf(( "End chunk_type %d\n", chunk_type ));
mprintf(( "End chunk_size %d\n", chunk_size ));
void model_octant_find_faces( polymodel * pm, model_octant * oct )
{
ubyte *p;
int submodel_num = pm->detail[0];
p = pm->submodel[submodel_num].bsp_data;
mprintf(( "chunk_type OP_DEFPOINTS %d\n", OP_DEFPOINTS ));
mprintf(( "chunk_type OP_FLATPOLY %d\n", OP_FLATPOLY ));
mprintf(( "chunk_type OP_TMAPPOLY %d\n", OP_TMAPPOLY ));
mprintf(( "chunk_type OP_SORTNORM %d\n", OP_SORTNORM ));
mprintf(( "chunk_type OP_BOUNDBOX %d\n", OP_BOUNDBOX ));
oct->nverts = 0;
model_octant_find_faces_sub(pm, oct, p, 1 );
mprintf(( "%s\n", "pass" ));
if ( oct->nverts < 1 ) {
oct->nverts = 0;
oct->verts = NULL;
return;
}
oct->verts = (vec3d **)vm_malloc( sizeof(vec3d *) * oct->nverts );
Assert(oct->verts!=NULL);
oct->nverts = 0;
model_octant_find_faces_sub(pm, oct, p, 0 );
// mprintf(( "Octant has %d faces\n", oct->nfaces ));
}
Results in this
chunk_type OP_DEFPOINTS 1
chunk_type OP_FLATPOLY 2
chunk_type OP_TMAPPOLY 3
chunk_type OP_SORTNORM 4
chunk_type OP_BOUNDBOX 5
First chunk_type 1
First chunk_size 74590
End chunk_type 4
End chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 4
First chunk_size 80
First chunk_type 0
First chunk_size 0
First chunk_type 5
First chunk_size 32
End chunk_type 3
End chunk_size 80
<crash here>
74590 is not multiple of 4 and that p += chunk_size; that makes the pointer to go to unaligned memory, so i went on to check that first chunk size on retail models:
First chunk_size 16072 | 4018.000000
First chunk_size 15268 | 3817.000000
First chunk_size 9716 | 2429.000000
No point in writting them all, they are all multiple of 4.
So Goober, you are right.
Now this happens while reading the .pof into memory or when reading the data in memory? Because if this happens while reading the file... well, i not sure if it can be fixed, altrought i dont know the code to think in a workaround, maybe memcpy the entire thing and work from there. Where exacty this is done in the code?
-
The chunk size is specified as a number within the POF file itself. So the first thing to do is to find out what that chunk represents and why it was given that particular size. It is very possible that whatever program was used to create the POF file wrote incorrect data.
The best solution is to re-export the POF with the correct settings. If that is not possible, then a fallback option is to memcpy the bytes, manually align them, and then read the data from the copy. But that would be much more complicated.
-
I would like to come out with a fix that works in general rather than going to edit every pof for every mod out there.
There are two things that may work:
1) use the "__packed" atributte, this should allow for unaligned access of data, i did this with the pointer, but my attempts so far has been unsuccessfull, but i never did this before so i need to read about it a bit more, i may not be doing it right or over the right data.
2) memcpy and fix does not sound that hard, but i need to know exactly was wrong, its the BSP_Data that is not correctly aligned here right?
This data?
p = pm->submodel[submodel_num].bsp_data;
If that i think i can come out with a code that memcpy it and if the chunk size is wrong, fix it.
Anyway in both cases it will take a while as i need to read and understand how the bsp_data works, for example i do not understand why model_octant_find_faces_sub is recursive.
-
More specifically it is the chunks in the BSP data tree that are not aligned. All of the chunks are concatenated together, but if some of them are not multiples of 4, they will not be on the proper boundaries.
I will reiterate what I said before:
So the first thing to do is to find out what that chunk represents and why it was given that particular size. It is very possible that whatever program was used to create the POF file wrote incorrect data.
We don't know why the chunk did not have a proper size. It is possible that there is a bug in the POF program, which means that it is important to understand the bug before attempting a fix. There may be a deeper problem hidden beneath the superficial problem.
-
Well yes, the V models are all aligned that cant be a coincidence. Actually it makes sence since back in the 90s having unaligned access would have performance costs, today thats a non-issue. Except for these ARM processors that for some reason cant deal with it, even trough they should.
OK, taking a look at the pofs first, that would be the PCS2 and PCS1 right?
And if someone else also want to check for unligned models adding this
if((chunk_size % 4) != 0)
{
mprintf(( "Warning: Unaligned memory access: Chunk Type %d, chunk_size %d in model_octant_find_faces_sub\n", chunk_type, chunk_size ));
}
before
p += chunk_size;
chunk_type = w(p);
chunk_size = w(p+4);in model_octant_find_faces_sub on modeloctant.cpp is a easy way to detect it.
-
That looks good, but you should also check to see if the frontlist, etc. variables are divisible by 4. That could cause the same issue.
PCS2 is the main program used for editing POF data. However I don't think it's the program that's used for converting the original 3D model file to POF in the first place. I recommend asking a modeler about that.
-
The first was Modelview32 i think.
I checked ModelView32 there is no reference to anything, but i cant find the source code so i cant look into it. Opening a pof, go to edit and save it does not fix it.
PCS2 the same, i took a look in the source code https://github.com/scp-fs2open/PCS2/blob/master/src/BSPDataStructs.cpp
int BSP_DefPoints::Write(char *buffer)
There is nothing there enforcing aligment.
In the end i think it is a 20 year old oversight, V tools produced properly aligned models and Modelview and PCS do not, and no one realised because it just works on x86.
I think PCS2 can be fixed so in going to open a issue on git. Them ill start thinking in a workaround in fs2 code for these cases, that dosent mean i can do it, but ill try.
WAIT, hang on, i found this on ModelView32 readme.
Note:
~~~~~
- If you save a POF in the editor that you just extracted from the original
FreeSpace 1/2 .VP files, the file will be a bit smaller than the original
file, even though nothing changed. This is not a bug, there is no
information lost. However the original Volition editors wasted some bytes
containing nothing but null or CRLF bytes at the end of strings, which are
however totally useless. MODELVIEW in contrast cuts strings to their actual
content.
Those extra bytes that modelView is cutting were the aligment bytes, i have no other explanation. This carried on to PCS2.
-
OOF
Yet another example of the idiotic human fallacy, "I don't understand it, therefore it must be wrong."
-
That looks good, but you should also check to see if the frontlist, etc. variables are divisible by 4. That could cause the same issue.
I'm not looking at the code, but those are just offsets of p, right? In which case chunk_size alignment will determine alignment of the rest.
-
-removed-
-
-removed-
-
That looks good, but you should also check to see if the frontlist, etc. variables are divisible by 4. That could cause the same issue.
I'm not looking at the code, but those are just offsets of p, right? In which case chunk_size alignment will determine alignment of the rest.
If I understand the code correctly, they are byte offsets of p. The model_octant_find_faces_sub function is passed a pointer which is p plus a number, and p is of type ubyte.
-
-removed-
-
Err, don't replace the BSP data with the aligned version. Try just copying the bad chunk, aligning it, and passing it to the next function. Then when the function returns, free the copy. Don't touch the underlying bsp tree.
This implies the function will create and maintain recursive copies of sub-trees if the sub-trees are also unaligned, but as long as the copies aren't too large, this should be fine. The tree is finite.
-
-removed-
-
It would be better to implement a proper fix, rather than a hack. The reason Release builds still aren't working is probably because of the remaining unaligned calls.
I've asked chief1983 to tag the appropriate revision for 3.7.2.
-
-removed-
-
Wtf. Is this thread serious? Amazing.
-
Broadly speaking, x86 CPUs only incur a significant performance penalty for unaligned accesses when they straddle a cacheline boundary. That's because software using unaligned accesses is so common in the x86 world that the hardware has been optimised to accommodate it. This wasn't quite so true when Freespace was first released, but at worst you would incur a one-cycle delay per unaligned access.
Conversely, ARM CPUs are mostly still designed to be efficient first and fast second, on the grounds that an efficiently designed CPU might run fast anyway if the software is written properly. The circuitry to handle unaligned accesses is therefore considered an unnecessary complexity, given that well-written software should avoid it; unaligned executable code is outright forbidden (as ARM instructions are all 4 bytes long). This goes so far that unaligned accesses are not merely unoptimised, but totally unsupported in hardware. To work around this when an unaligned access happens anyway, it is possible to handle the exception in software by replacing the single access with multiple byte-wide accesses, but of course this is slower and requires an extra temporary register; a single 32-bit load becomes a 7-instruction sequence (load byte 0, load byte 1, combine bytes, load byte 2…) that is difficult to run in parallel. And that's ignoring the major overhead of taking the exception in the first place.
I think it would be wise to at least process all the mods in Knossos to ensure they have proper alignment, as well as patching the tools to make them produce aligned output in the first place. But to allow the apparently large corpus of misaligned mods to run on ARM, an alignment workaround does need to be added to the loader.
-
My god what have we done
As for the tag for 3.7.2, it doesn't exist because we weren't even on Git yet. The quote from the release post concerns me though: "It is an export of 3.7.2 branch r11329". I don't see a 3.7.2 branch in our git even though most of the older branches were created. Someone got too aggressive with branch pruning I think and deleted it. Either way, I found the git commit was still on my fork and pushed a branch and tag for 3.7.2 up to the main repository. Enjoy.
-
-removed-
-
-removed-
-
By all means, update the wiki where it needs updating. I have created an account for you using your HLP email.
-
Thanks Goober, i will update the wiki once im done with this.
Question: Does someone know what information is used in modelcollide.cpp, void model_collide_parse_bsp(bsp_collision_tree *tree, void *model_ptr, int version)? Because i managed to fix the pofs enoght to make them load on 3.7.2 Debug whiout any code modifications, but release still crashes on that function with a bus error, most likely on those vec3d i mentioned earlier.
-
You can browse the code on GitHub:
https://github.com/scp-fs2open/fs2open.github.com/blob/master/code/model/modelcollide.cpp#L733
EDIT: Actually you would have had to have the code to make modifications. What do you mean, does anyone know what information is used?
-
-removed-
-
¯\_(ツ)_/¯
Honestly, I think you are the person who understands this the most - certainly the person who has studied this to this level of detail in the past 10 years.
All I can say is to break down the problem into manageable tasks, take things one at a time, take baby steps, and try to squash the problems one by one. Feel free to keep posting your progress in this thread though; if we have any insight to offer, we will certainly chime in.
-
-removed-
-
Well the good news is the performance gain alone may be enough incentive for a mass effort to fix all the old models floating around.
-
That is on ARM, i dont think x86 will have any performance gain im afraid. But i will check just to be sure.
At any rate, ARM is becoming more and more powerfull and common.
-
-removed-
-
ARM is little endian so that code is unused in this case. But yeah, that 1 byte at the start is :sigh:. Even if that swapping code is unused, that 1 byte means the shield collision data is going to be unaligned.
INTEL_INT/INTEL_FLOAT are for byte swapping.
-
Then disabling SLDC data loading on archs that are not x86 is the way to go, because the workaround i came out with in fvi.cpp with all those memcpy that are done at every shild hit is in no way faster and fixing SLDC for non x86 archs would mean having to do it again, chaging that char for an int, making models incompatible.
Maybe there is some other way, but no other comes to mind right now. Dealing with it would need to do an unaligned access at some point or the other, that you can never be sure that the kernel is going to be able to fix or not.
Now if i can only figure out what bit of data is causing the release build to crash at model load on bsp_collide_tree...
-
Why would the model alignment affect FPS? Aren't models completely loaded into memory before the mission starts? So during the mission they should be referenced from their FSO data structure, not from the POF file.
-
-removed-
-
Maybe there is some other way, but no other comes to mind right now. Dealing with it would need to do an unaligned access at some point or the other, that you can never be sure that the kernel is going to be able to fix or not.
This thread triggers PTSD flashbacks from when I was trying to get a Doom sourceport working on a Sparc workstation (sparc64 doesn't take a performance hit with unaligned memory access, it just goes lolnope SIGBUS).
Unaligned POF data needs to be fixed externally, but the unaligned structs in FSO code you should be able to fix by telling gcc to enforce alignment during compile time:
https://gcc.gnu.org/onlinedocs/gcc-3.1/gcc/Type-Attributes.html
Sprinkle in "__attribute__ ((aligned (4)))"
(or 8) where needed.
I know other compilers (like the Sun CC) have command line flags to enforce that globally during compilation, but gcc seems to be missing such a feature?
-
-removed-
-
-removed-
-
-removed-
-
This is part of the scripting system. There have been several fixes to that over the years; it's entirely possible that this is related. See these PRs for example:
https://github.com/scp-fs2open/fs2open.github.com/pull/2119
https://github.com/scp-fs2open/fs2open.github.com/pull/1960
You may be stuck with coding up an alignment-fixer-loader, unless you want to be restricted to only a subset of mods. There are literally thousands of POFs that have been released for FreeSpace -- perhaps tens of thousands -- and it would be impractical to fix them all.
-
Perhaps, but starting with ones released on Knossos would be a big win and a much smaller subset.
-
-removed-
-
-removed-
-
-removed-
-
I wonder if we could integrate this conversion into knossos/nebula if it ever gets stable enough...
-
-removed-
-
-removed-
-
Just compile it default disabled on ARM maybe. Just like SLDC. Solved!
-
-removed-
-
-removed-
-
-removed-
-
Sounds reasonable.
-
-removed-
-
-removed-
-
Er, this VP writer isn't modifying VP files in-place, is it?
-
-removed-
-
-removed-
-
Would it be possible to add this to trunk for any arm processors that are in computers that have better graphics cards or better open gl support?
-
-removed-
-
The only thing that i dont know is if you guys are going to accept pof version number 2118 (and what it means) and SLC2, im waiting on that to add that information on wiki and create a issue on PCS2 git for future support.
I don't see why not. It's certainly a valid reason to bump the POF version. Just make sure that the code can load both < and >= 2118.
And by all means, submit a pull request to the main FSO repository with the align-on-load fix.
-
-removed-
-
I've been following the emails from github on the updates, it's like watching a daytime soap lol