I'm thinking the fix isn't 100%

PCX data is generally (always?) compressed and TGA data can be RLE compressed or might be 8-bit expanded and then expanded to 32 bit.

Then again, I might not be seeing in the code where that is addressed.

[+1 to list of things to check out ...] 

Yes, the fix was done in v0.85.7:
http://quakespasm.svn.sourceforge.net/viewvc/quakespasm?view=revision&revision=633 

I'm pretty certain that code originated with me, but memory is hazy. The comment on the code you quoted is in my style anyway, and I'm in the habit of always initializing pointers to NULL, but I note that the current QS version is a little different (different comment, uninitialized FILE *f).

Anyway, TGAs and PCXs will fall through to the second condition as external texes use offset 0: "else if (glt->source_file[0] && !glt->source_offset)" - the "read the full PAK file into memory" problem should only happen with native textures inside BSPs or MDLs, where the offset is an offset into the BSP or MDL file, not into any PAK that may contain it (COM_FOpenFile will set up the file pointer correctly for the base file whether it's a loose file or a PAKed file, then we fseek from there).

There was a hack I did for DDS files where I added a SRC_DDS format and put the full file size into width, but with hindsight and in light of the above that wasn't even necessary.

Nowadays I'd probably be more inclined to glGetTexImage into an array of system memory buffers (malloc, not hunk) and delete each texture from GPU memory as it goes in. Then glTexImage from that array and free each buffer immediately after restart. Since textures are backed up by system memory copies anyway that wouldn't be any extra overhead that wasn't there to begin with. Would be much simpler and cleaner, and should also reload faster (no file I/O). 

Possible location of issue:

sv_main.c -> SV_ConnectClient ...

strcpy (client->name, "unconnected");
client->active = true;
client->spawned = false;
client->edict = ent;
client->message.data = client->msgbuf; <---- HERE
client->message.maxsize = sizeof(client->msgbuf);
client->message.allowoverflow = true;

Because ... server.h ...

byte msgbuf[MAX_MSGLEN];

And quakedef.h ...

#define MAX_MSGLEN 32000 // max length of a reliable message //johnfitz -- was 8000

And ...

sv_main.c SV_SendClientDatagram ...

qboolean SV_SendClientDatagram (client_t *client)
{
byte buf[MAX_DATAGRAM];
sizebuf_t msg;

msg.data = buf;
msg.maxsize = sizeof(buf); <--- Here

Because server.h

#define MAX_DATAGRAM 32000 // max length of unreliable message //johnfitz -- was 1024

Maybe ... not a sure thing but looks very likely right now. Not that anyone is going to be using GLQuake ...

I also kind of wonder if sv_protocol 15 demos will play back on GLQuake.

Knowledge pile +1 

Ok this is bloody wierd because I'm pretty sure it didn't happen before...

You know mods that replace start.bsp? (e.g. czg's czg07, or honey)

Well, I'm finding now that when running these mods it only ever loads the ID1 start.bsp

Anyone else had this problem? 

ignore the above I was being being a 'tard. 

 

I was playing apsp1 in quakespasm last night and noticed the mis-positioned elevator that mfx mentioned seeing too:

http://www.quaketastic.com/files/misc/apsp1_plat.jpg
http://www.quaketastic.com/files/misc/apsp1_plat.sav.zip

The bounding box is fine, so it doesn't affect gameplay, but it's just rendered in the wrong place.

I tried some different engines, and the problem started in Fitz 0.85. Fitz 0.80 is fine, as well as glquake/winquake. The lastest Darkplaces has the problem too though.

This is the entity text, from opening the bsp in a text editor:

{
"classname" "func_train"
"angle" "-1"
"sounds" "4"
"speed" "184"
"target" "3f_lift_p1"
"dmg" "5"
"noise" "plats/plat2.wav"
"noise1" "plats/plat1.wav"
"targetname" "tllift"
"model" "*10"
}

If I put a breakpoint in R_DrawBrushModel, e->origin is (0, 0, 0) and e->angles is (0, -1.40625, 0). The second component of e->angles seems to be related to the problem, if I zero it before the R_RotateForEntity call, the plat appears in the correct position. (I don't understand why adding a rotation to the OpenGL matrix shows up visually as the plat translated diagonally.) 

I have seen this before. The root cause is that func_train should not have an "angle" set. So I could blame it on the mapper. But, under most engines the -1 is rounded to 0, causing no visible problem. Therefore it's hard to blame the mapper when the engine came out years after the map!

Fitzquake (and apparently Darkplaces) round the angle to the nearest representable value instead of flooring it towards zero, resulting in more accurate angles for rotating entities. However as you can see, -1.4 may be closer to -1 than 0 is, but -1.4 results in the mapper's harmless error now being highly visible.

As to why the small rotation appears to be a diagonal translation, remember that bmodels have their origins at the world origin, so a small rotation around the world origin would result in a large lateral move for the visible geometry of the lift.

The solution may be to disable the "improvement" of better rounding for angles. 

I remember seeing this with a lift in A Desert Dusk too, and, checking the bsp, sure enough it has a func_train with "angle" "-1". 

Rounding toward the nearest value is actually what the Quake code attempted to do, but did it wrong (due to an int cast of a float rounding toward zero, but the Quake code assumed it rounded down consistently).

So I'm both a little unnerved by the error, and inclined to blame it on the map rather than the engine, I could do a spot-fix for this specific map name but I don't normally do any spot-fixes for anything in the engine as I believe in having no map-specific or model-specific logic anywhere in the engine.

A .ent file could easily fix this...

sv_saveentfile while playing it, and then edit that one entity in there and any future loads will use the modified .ent file instead of the data in the .bsp.

A more important issue is that this angles is going to affect the MOVETYPE_PUSH directly because darkplaces supports rotated (and rotating) plats, so there is a physical difference here.

An alternative - however with unknown side effects - what if the engine chose to ignore incomplete vectors when parsing the entities? Because angles is a .vector field in the progs.dat, it is being parsed as a vector, it is an incomplete vector whose value is just "-1", right now this gets expanded to "-1 0 0" but perhaps it should expand to "0 0 0" due to incompleteness? 

i feel this is just a map bug that went unnoticed due to an engine bug.

the problem lies with the map. 

Just to clarify, this is "angle", so that second approach I mentioned won't work as this isn't an incomplete vector.

A spot fix for specific map names to ignore "angle" "-1" seems to be the safest solution but still a complete hack and I don't like hacks. 

The engine could detect the following combination:
classname == func_train
origin == 0 0 0
angles != 0 0 0

And clear the angles in that case.

Any entity with origin 0 0 0 that is using angles is suspicious, but a func_train is highly suspect. 

I believe the cause for this is a mapper who started with a func_door with angle -1, then later changed the classname to func_train/plat to get better behavior, but didn't clear out the "angle" key.

As to the question of fixing it with an engine hack, i guess testing for "func_train" or "func_plat" with angle != 0 would be a fairly targeted way to do it. It's a hack no matter what unfortunately, and I'm not 100% against hacks (for example i put in a hack for the large shell ammobox texture) but what worries me is we don't know the complete list of existing levels that have this problem. 

I believe the cause for this is a mapper who started with a func_door with angle -1, then later changed the classname to func_train/plat to get better behavior, but didn't clear out the "angle" key.

To be clear, i mean that of the 3-4 cases I know about, the cause seems to always be the same. 

but if they're not supposed to have angles, why don't you just always throw it away? 

quakec could do that. Engine doesn't know what is supposed to have what. 

In engines that support rotating platforms (which darkplaces for example does), I can't just throw away angles on func_train automatically.

But in the specific case that the func_train has origin 0 0 0 (which is the default for any brush model), the angles make no sense - as it would just orbit around the center of the map. 

That's something I could see in abstract maps though. 

fair enough. I was assuming that all mods with rotation support would use func_rotate_train isntead, but i guess that isn't guaranteed. 

How bad would the side effects be of reverting to the original rounding code? whereabouts in the code is the rounding?

The hacks do sound dangerous to me, unless they were restricted to the known filenames with this bug.

The only good news is mappers won't make this mistake on new maps, since most SP mappers probably test in a fitz0.85 derived engine and/or darkplaces. 

The rounding is only for the networking (I.E. visual only), if the engine supports rotation in the underlying sv_phys.c code, then even if the visuals are not rotated, the physics will be (leading to a rather confusing jump and fall-through in this case).

DarkPlaces supports that, so I can't really just revert my rounding change.

Furthermore, the same rounding change gives a significant improvement in aiming, where it is widely known that stock quake has kind of a "down and to the right of the crosshair" characteristic, the aiming is more accurate because of this rounding fix (although darkplaces goes further and uses 16bit angles for aiming so you can hit a pinpoint location across the room). 

It is okay to throw away negative angles for a func_train.

Even in DarkPlaces, negative angles don't make any sense (except for func_wall or func_door where -1 and -2 have special meaning) because negative angles are invalid.

Rotated brushes/entities in any map editor have positive angles from 0 to 359.999999.

[This excludes the idea that someone would write custom func_train code that supported angles, but even then the values would be positive]. 

My main reason for bumping this thread is this.

FitzQuake renders rather slow compared to other engines because of the sky. The sky draws first and in the case of a skybox, it literally draws the entire screen.

I'd like to Z-fail the sky, but I want the fog to render on the sky in the proper FitzQuake manner. And I'm not entirely sure what process sky entities does.

I always use A Roman Wilderness of Pain (arwop) map ROMAN1 as the example of an fps killer in engine testing. In Mark V, I get 20 FPS. DirectQ gets an insane 169 fps. In other engines, typically the fps is close to 30, which sounds low but is a 50% improvement over FitzQuake. (Even on a low end video card, some other engines often are hitting 600 fps with ease on id1/start.bsp and the FitzQuake renderer struggles to break 300).

FitzQuake does rendering far better than other engines, in my opinion, but the slowness is an obstacle and I think it is exclusive to the sky.

[There are other ways to increase the FitzQuake frame rate, like using 3 texture units to draw texture, fullbright and lightmap in the same pass, instead of using 2 texture units, but even if I essentially disable that with gl_fullbright 0, I still get 20 fps on ARWOP (no improvement).]