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

with modern graphics cards, fitzquake could draw skyboxes using a cubemap (directly rendering the sky faces instead of drawing an actual giant cube.) And for the classic scrolling sky, i think you could do it with a pixel shader. 

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

I don't think engines should make this kind of assumption based on the mods that exist today. Suppose someone writes a mod that unifies func_train and func_rotate_train under the former name. Then a negative angle is a deliberate signal to the mod - make the rotation towards the next waypoint move in the positive direction. 

Baker, I'm surprised sky is a bottleneck.

If I make Sky_DrawSky return immediately without drawing anything, I get essentially no change in fps. This is with a fairly fast cpu (2.3ghz i7), 2 year old laptop gpu (nvidia gt 650m).

For me over half the time in the roman1.bsp starting position is spent drawing entities (mostly on alias models), a bit less than half drawing the world. In Mark V I get 90fps, 205fps with "r_drawentities 0" and 135fps with "r_drawworld 0" (but r_drawentities 1). Try setting those to draw only the world / only entities, to see relatively how much time is spent on each. 

sky is expensive in terms of overdraw - if its drawn fullscreen anyway.
the vanilla code is ugly as heck, it warps in a horrible way as you move around.
the skybox code that seems to be fairly abundant has a few issues with certain situations, like side windows, that increase overdraw unnecessarily, which can be expensive. a good example is dm2, where the side windows (that noone remembers are there) result in the sky basically covering the entire screen.

it can be significant when you're framerate limited, but at least the worst-case is limited to (less than) only half the framerate. 

@preach

I suppose so, I hadn't considered that. The traditional func_train would never move up/down like a door or plat. ;)

@ericw

Load up a skybox and instead of not drawing the sky, don't draw the world.

Now I understand what you say in regards to speed, I'm not blaming the fps in arwop on the sky (make no mistake) -- if you load up ARWOP and in Mark V and type in "cl_shownet 1" and then type "sv_cullentities 2" (which is DarkPlaces/FTE anti-wallhack level of culling server-side) --- you will watch the network traffic drop by 2/3rds --- ARWOP is a prime example of a beneficiary of network optimization (DarkPlaces and FTE probably breeze by it).

But I think the sky may be costing 100 fps on a map like id1\start.

I'll be finding out ... 

Make no mistake, I love the FitzQuake renderer, obviously.

The sky has been bothering me for years! And it reared up yet again.

The way the rest of the rendering works, I have never been able to determine or intent of the sky drawing the way it does --- everything else in the engine is clearly a clockwork and quite deliberate.

My only guess is to render fog on it consistently. And I intend to find another way to do that, if so. 

Goals of the fitzquake sky rendering:

* sky should only be visible when looking at a sky brush, not void
* if a sky brush is in front of some other geometry in the PVS, you should see sky, not the geometry behind it.
* brush entities with sky texture on them should look like sky too
* sky should be rendered as if it was a giant cube, and should not be distorted by the actual geometry of the sky brushes or the movement of the camera

So what i do is clear the zbuffer, then draw all the sky surfaces (brush faces) untextured (to write to the zbuffer), then draw the skybox with the depth-test reversed so that only farther objects pass the test. This means the giant, distant box of the sky will only appear on portions of the screen where the sky brushes were already rendered. Then I draw the rest of the world, and since the sky surfaces have correct depth values, they occlude any polygons farther away.

Even when drawing the scrolling cloud sky, i use a giant cube that has been subdivided enough to approximate the dome shape (r_sky_quality controls the subdivisions)

There's also some code to attempt to draw less than a full box, when only part of the screen shows sky. This was disabled for skyboxes (couldn't get it right at the time, due to tjunctions), but enabled for the cloudy sky (since it's subdivided into a grid anyway, there are no tjunctions.)