-Add back use of last_position_x and last_position_y
-last_position_x and last_position_y will be set to undefined when the window is shutdown IF the current display used is not the desired display
-last_display will be set to desired display at window shutdown if not the same
-vid_displayindex clamped using ClampDisplayIndexCvar() at startup and window shutdown
-We only need to init the display indices once in GLimp_Init
-We only need to clear the display indices once in GLimp_Shutdown
-Remove extra 'displayindex' variable
-SDL_GetNumVideoDisplays() will always remain the same after the call to SDL_Init(SDL_INIT_VIDEO), so it makes sense to set in GLimp_Init where we do this.
There's an "enable alt joy keys" command now. If a key is bound to that
command, all joystick buttons (incl. hat and triggers) are turned from
K_JOYx into K_JOYx_ALT, which allows two keybindings on the same key,
one with the altselector pressed and one without.
If there's no keybinding for K_JOYx_ALT, it will use the binding for
just K_JOYx as a fallback (if it exists).
This is especially handy to create direct bindings for all the weapons
on the (limited amount of) Joystick buttons.
Seems like AMDs Windows driver doesn't like it when we call
glBufferData() *a lot* (other drivers, incl. Intels, don't seem to
care as much).
Even on an i7-4771 with a Radeon RX 580 I couldn't get stable 60fps
on Windows without this workaround (the open source Linux driver is ok).
This workaround can be enabled/disabled with the gl3_usebigvbo cvar;
by default it's -1 which means "enable if AMD driver is detected".
Enabling it when using a nvidia GPU with their proprietary drivers
reduces the performance to 1/3 of the fps we get without it, so it
indeed needs to be conditional...
use GL3_BufferAndDraw3D() instead of glBufferData() and glDrawArrays()
in each place it's needed.
This by itself doesn't make anything faster, but it will make trying out
different ways to upload data easier.
The developers tested their maps without the fix and decided that it
looked good. Add a new cvar gl_fixsurfsky defaulting to 0 that enables
the fix if someone really want it.
The software renderer already did this, but not the GL renderers. Maybe
the logic was lost somewhere on the long way... Without this change a
fullbright lightmap is generated for SURF_SKY surfaces and without the
SURF_DRAWSKY flags the surfaces aren't skipped in RecursiveLightPoint()
and GL3_LM_CreateSurfaceLightmap(). This isn't a problem under real
skyboxes, but in cases were SURF_SKY is abused fpr interior lightning.
rmine2.bsp in rogue is a good place to see the problem
Reported by @m-x-d, fixes#393.
I guess it makes sense to apply gamma to the color, we do the same
for the standard round particles.
Also, this way the fragment shader for square particles references the
uniCommon UBO (gamma is part of it) - apparently the Intel HD4000
(from Ivy Bridge) GPU driver for Windows has a bug that uniform blocks
that exist in the shader source but aren't actually used can't be found
(with glGetUniformBlockIndex(prog, name)), which we treat as an error
in gl3_shaders.c initShader3D().
fixes#391
Apparently the lightsource for exploding rockets/grenades is very close
to the surface, so the dot-Product between surface-normal and the
vector between the light and the pixel returns 0, basically disabling
the dynamic light for that surface.
As a workaround, move the lightposition (only for that dot product)
a bit above the surface, 32*surfaceNormal looks good.
fixes#386
The real needed size can't be derived from the .bsp file size, because
* many generated structs contain pointers
* there's lots of data generated per face..
* _especially_ for warped faces that are subdivided
introduced FS_GetFilenameForHandle(fileHandle_t) for this
this helps if a map has been started with "wrong" case, which doesn't
immediately fail if it has been loaded from a pack, but will result
in invalid savegame names that (with case-sensitive FSs) will fail to
load (when going back to a formerly played level)
The r1q2 code prioritized pak files over all other files, e.g. as soon
as a pak file was requested no more file were added to the download
queue until it finished downloading. That way one could be sure that
assets included in the pak file weren't downloaded in parallel as single
files.
This is a better, bugfixed and more robust implementation of the same
logic. With this back in place we can switch back to parallel downloads
which gives a nice speedup on Windows. Maybe, just maybe some day
Microsoft will fix Windows crappy I/O...
Working with getter and setters was a good idea as long as we had one or
two quirks. Now we're at three with maybe more to come so it's easier to
use a struct to communicate quirks between the precacher and the HTTP
download code.
The stores it's text in the key_lines array which is NUM_KEY_LINES *
MAXCMDLINE chars long. The code never checked for overflows, it just
assumed that a line will never be longer then 256 chars * 8 = 2048
pixel. With modern displays we can have higher vertical resolutions,
so the array will overflow sooner or later.
Fix it by clamping the maximum line width to MAXCMDLINE - 2 chars (1
for the prompt and 1 for the terminating \0). While at it increase
MAXCMDLINE to 1024 chars * 8 = 8192 pixel, which is more then 8k
resolution and should be enough for the years to come.
This is belived tot fix at least a part of issue #368.
I'm not 100% sure if this is okay for q2pro, but at least in my simple
tests r1q2, q2pro and now yq2 generate the same URL. Nevertheless it's
somewhat inconssistent to search generic files at /moddir/... and the
filelist at /moddir.filelist
This closes issue #370.
Until now the softrenderer calculated the fov relative to a hard coded
aspect of 4/3. That's wrong, because we're supporting arbitrary aspects
and we aren't calculating a fov but just a scaling factor to the global
fov which takes the aspect into the account.
Fix this by not taking any aspect calculations into account. BUT: While
this renders the gun with a correct perspective it's positioned much
nearer to the camera / player then in the GL renderers. The GL renderers
work around that problem by enforcing a minimal Z distance of 4 units,
which can't do because we're just calculating a scaling factor...
The r1q2 URL generator was, like everything in this game, buggy. It took
cl.gamedir into account when generating the URLs, but overlooked that it
is only set when 'gamedir != BASEDIR'. So baseq2 assets ended up with
/maps/foo.bsp and mod assets with /mod/maps/foo.bsp. q2pro fixed that
to always include the gamedir...
Work around this by refactoring the HTTP -> UDP fallback logic to be
more generic: Count the number of iterations and depending on the
iteration set the gamedir to be used by the URL generator or force
UDP downloads.
Cleaning the download queue as soon as a file finished downloading leads
in combination with only one parallel download and multiple precacher
runs to an ugly problem: The generic filelist is requested several time
which can lead to cycles. Hack around this by rembering if we already
requested it and reset set reminder as soon as the precacher finished.
Yes, I'm feeling dirty.
If the server sends us an URL with trailing slash we're generating URIs
like http://example.com//maps/foo.bsp. While double // are perfectly
valid they might me rejected by some servers. So let's play save.
While at it replace the crappy Z_TagMalloc() with the standard malloc().
Z_TagMalloc() ist just a wrapper arround malloc(), there's no functional
change.
This looks easy, but is rather hacky... Downloading is implemented
through the precacher. The server sends an asset list, while loading
the map another one is generated. CL_RequestNextDownload() goes
through this list, in the order models / maps -> sounds -> images,
calls CL_CheckOrDownloadFile() for each file. CL_CheckOrDownloadFile()
checks if the file is already there, return true if it is and false
if not. If the return code is false CL_RequestNextDownload() itself
returns, it's called again by CL_ParseDownload() as soon as the just
queued file finished downloading. This way all missing files are
downloaded one after the other, when CL_RequestNextDownload() finally
reaches it's end (all files are there) it send 'begin' to the server,
thus putting the client into the game.
HTTP downloads are parallel, so CL_RequestNextDownload() cannot track
which files are there and which are missing. The work around for that is
to queue the file but have CL_CheckOrDownloadFile() return true. So
CL_RequestNextDownload() thinks the file is already there, continues
with the next one, until all missing files are queued. After that it
polls CL_PendingHTTPDownloads() and sends the 'begin' as soon as all
HTTP downloads are finished.
If a HTTP download fails we cannot just queue it as UDP download,
because the precacher things that the file is already there. And we
can't tell the precacher that it's not because the precacher tracks
files only by the number of downloaded files per asste type and not
their name. Just decreasing the number of downloaded files isn't
possible since the precacher may have progressed to the next asset
type.
So: On the HTTP side it's tracked if there was an error or not. After
CL_RequestNextDownload() has queued all files and waited for all HTTP
downloads to finish it checks the HTTP error status. If there was an
error the precacher state is reset and CL_RequestNextDownload() recurses
into itself to take another run. All files that couldn't be downloaded
are queued again, this time as UDP downloads.
My solution to this problem is somewhat hacky. A newly added function
OGG_SaveState() can be called to save the state and OGG_RecoverState()
at a later time to restore it. There's only one state and it works
only iff OGG is state playing.
This closes#347.
If we're passing file handles to libcurl to write the data into, the
game may crash under Windows due to incompatible C runtimes between cURL
and quake2. This is even mentioned in the official cURL doku.
Since the moment I took a very first look at the download code I wasn't
a friend of parallel downloads. There're several reasons for that:
- Parallel downloading needs some ugly hacks. For example downloading a
pak file has a high chance to make asset downloads running in parallel
unnecessary.
- Parallel downloads are hard to debug.
- There's just no need for them. I've tested several connection, 1
GBit/s LAN, 50 MBit/s DSL, 6 MBit/s DSL, and there wasn't a
significant difference between 1, 4 or even 16 parallel downloads.
I'm leaving the parallel download code in place. I someone really wants
parallel downloads he can bump the MAX_HTTP_HANDLES define.
Until now download queues entries were created for each file, but only
removed in the unlikely event of changed download server. This leaked
about 4200 byte per file. Fix this by:
- Create a new function CL_RemoveFromQueue() that removes a queue entry
and call it everytime when a download either finished, failed or
aborted.
- Retire the 'download done' state, just the queue entries associated
with a download handle to NULL to communicate that the handle is
unused.
- Cleanup the full queue at shutdown.
Loading libcurl at runtime instead of linking it at compile time makes
things a lot easier and more reliable on Windows. On other platform
libcurl can be installed as optional dependency instead as an hard one.
We're printing only the two relevant informations: A download was queued
and a download finished or failed. That's enough to see what's going on
and not too noisy.
...and fix the bugs, that were worked around with that crap, instead.
This removes some corner cases like cancelation of all HTTP downloads
and fallback to UDP if too many 404 errors were generated. If this is
still a problem in reality - for example HTTP servers blocking the
client after too many 404 or even crashing HTTP server - fix the server
and don't force the clients to work around that.
We aren't in 1997 anymore, todays broadband connections are fast enough
to handle multiple large files. This may download some assets twice if
the server provides both a pak file with all assets and the assets as
plain files.
There's no need to parse the HTTP header on our side of things, CURL can
do that. And the old buffer code was overcomplicated, simplify it. While
at it switch to normal malloc().
While the download speed calculations may be useful their implementation
is crappy and the integration into the console is rather fragile. If we
really want to support the progress bar with HTTP downloads this needs
to be reimplemented.
The URL generation logic was buggy, it took the local fs_gamedir into
account when determining the files path on the server. That could have
worked in r1q2 or q2dos since they assume that the executable dir is
also the config dir. But it breaks in YQ2 were the executable dir and
the config dir differ.
These are:
- CL_ResetPrecacheCheck(): Resets the precacher, forces it to reevaluate
which assets are available and what needs to be downloaded.
- FS_FileInGamedir(): Checks if a file (and only a real file, not
somthing in a pak) is available in fs_gamedir.
- FS_AddPAKFromGamedir(): Adds a pak in fs_gamedir to the search path.
This is a very first cut:
* It compiles
* It doesn't crash
What's missing:
* cmake integration
* CURL should be loaded dynamically
* Integration between download code and filesystem
* Likely UTF-8 stuff
* cl_http.c needs cleanup
* Windows support
We're taking indices and converting them to pointer relative to the
hunks base. Yes, that's dirty. Since the indices are stored as 32 bit
values and hunks are generally small using 32 bit pointers is enough,
even on 64 bit platforms. So the code took the size of void* / 2...
See the problem? Yes, that's not a good idea on 32 bit platforms. Bite
the bullet and just take the size of void*. Shouldn't be a problem,
because the indices are the first thing that's loaded and the hunk is
trimmed right after it anyways. If, and just if, we really need each and
every byte in the early stages of map loading we need two cases. One for
64 bit and one for 32 bit.
This fixes issue #346. Kudos to @ricardosdl for the analysis.
In src/backends/unix/network.c:
* line 181: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?
* line 276: The scope of the variable 'tmp' can be reduced.
* line 665: The scope of the variable 'mcast_addr' can be reduced.
* line 665: The scope of the variable 'mcast_port' can be reduced.
* line 666: The scope of the variable 'error' can be reduced.
* line 775: The scope of the variable 'i' can be reduced.
In src/backends/windows/network.c:
* line 186: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?
* line 287: The scope of the variable 'tmp' can be reduced.
* line 707: The scope of the variable 'mcast_addr' can be reduced.
* line 707: The scope of the variable 'mcast_port' can be reduced.
* line 1049: The scope of the variable 'err' can be reduced.
* line 1163: The scope of the variable 'i' can be reduced.
In src/client/menu/menu.c
arrayIndexOutOfBounds:
* line 1921: Array 'creditsIndex[256]' accessed at index 256, which is out of bounds.
variableScope:
* line 332: The scope of the variable 'item' can be reduced.
* line 533: The scope of the variable 'x' can be reduced.
* line 533: The scope of the variable 'y' can be reduced.
* line 838: The scope of the variable 'b' can be reduced.
* line 864: The scope of the variable 'b' can be reduced.
* line 1910: The scope of the variable 'n' can be reduced.
* line 2199: The scope of the variable 'str' can be reduced.
* line 2812: The scope of the variable 'length' can be reduced.
* line 2813: The scope of the variable 'i' can be reduced.
* line 3838: The scope of the variable 'c' can be reduced.
* line 4112: The scope of the variable 'scratch' can be reduced.
* line 4181: The scope of the variable 'i' can be reduced.
* line 4345: The scope of the variable 's' can be reduced.
In src/game/player/hud.c
arrayIndexOutOfBounds:
* line 132: Array itemlist[43] accessed at index 255 which is out of bounds.
Itemlist assigned only once, and has only 43 items, better ignore unexisted items.
variableScope:
* line 82: The scope of the variable 'n' can be reduced.
* line 217: The scope of the variable 'x' can be reduced.
* line 217: The scope of the variable 'y' can be reduced.
* line 218: The scope of the variable 'cl' can be reduced.
* line 583: The scope of the variable 'cl' can be reduced.
In issue #296 it was pointed out that the menu shows ogg_shuffle
always as disabled, even if it's set to 1. This was an oversight,
the menu code was still checking the ogg_squence cvar removed in
the big OGG/Vorbis refactoring. Update it to match reality.
1) Do not increment the frame rate returned by SDL by 1. Incrementing
is unnessecary, more or less up to date versions on Nvidias, AMDs
and Intels GPU driver on relevant platform return an value that's
either correct or rounded up to next integer. And SDL itself also
rounds up to the next integer. At least in current versions. In fact,
incrementing the value by one is harmfull, it messes our internal
timing up and leads to subtile miss predictions. Working around that
in frame.c would add another bunch of fragile magic... So just do
it correctly. If someone still has broken GPU drivers or SDL versions
that are rounding down the could set vid_displayrefreshrate.
2) The calculation of the 5% security margin to pfps in frame.c was
wrong. It didn't take into account that rfps can be slightly wrong
in the first place, e.g. 60 on an 59.95hz display. Correct it by
comparing against rfps including the margin and not the plain value.
After some pondering I realised that the changes was stupid. It
introduces some new subtile bugs, for example in some cases SDL
still rounds 59.95hz down to 59hz...
In the old world GLimp_GetRefreshRate() was called once at renderer
startup. Now in the new world with SDL 2.0 only it's called every frame
and thus the target framerate git increased by one every frame... That
lead to subtile timin problem in case that the vsync is enabled.
While here remove the hack added for some Windows GPU drivers by AMD.
Older versions returned 59 on 59.95hz displays, leading to small timing
problems. This is fixed in newer version so we don't need to work around
it. Removing the hack gives us somewhat more overall timing precision.
If someone really needs the hack vid_displayrefresh can be set to 60 to
get the old behaviour.
Until now we just called OGG_Stop() as soon as we read the last samples
from a Vorbis files. OGG_Stop() flushed all unplayed samples (about 12
seconds of playback) from the OpenAL playback queue... Instead just set
our internal state of STOPPED, open the next file and be done.
Make sure that the window is destroyed at gl renderer shutdown and
recreated by the soft renderer. Don't deinitialize SDL in the
softrenderer, that's done by vid.c. And make sure that we start the soft
renderer with a clean GL state.
I've chosen the minimal invasive way for this:
* Import miniz and remove -lz linker flags.
* Create a short header minizconf.h roviding everything we need
originally defined by zconf.h and not provided by miniz.
* Replace zlib.h with miniz.h and minizconf.h.
The input system backend was once used in the client and the renderers,
but for some years now it has been an integral part of the client only.
Move it there.
The last commits did some bigger changes to the interaction between the
GL renderers and the client. The code is now SDL 2.0 conformant, window
and context creation are strictly distinct operations. SDL is only
initialized when necessary. Since this broke the client <-> renderer
API, bump it's version.
There a lot of things left to do for dark and cold winter evenings:
* The software renderer implements it's own window handling and
reinitialized SDL whenever vid_restart is called. This is highly
problematic.
* vid_fullscreen is abused to communicate changes to renderer config
throughout the code. That's a very ugly, messy and potential very
problematic hack. But not easy to remove.
* Some funtion calls between the client and the renderer are
unnecessary.
The changes to the client <-> renderer interaction fixed issue #302.
* Sync both files as much as possible.
* Another round of general cleanup.
* Fix stencil tests.
* Simplify gamma handling, hardware gamma is now default.
* Support new client <-> renderer API.
* Another round of general cleanup.
* Introduce gl3_libgl cvar to force a libGL.
* Fix stencil buffer tests.
* Further untangle window <-> context stuff.
The window is now fully at client side, the context at renderer side.
This is another break of the renderer API. And at least GL1 needs to
track this, it's broken for now.
* Even more syntax and code style fixes.
* Rename functions to match their actual purpose.
* Fix comments.
* SDL initialization and shutdown is now client side only. With
SDL 1.2 finally gone there's no need to involve the renderers
in it.
This breaks the client <-> renderer API. I haven't bumped the API
version with this commit because there're likely more changes when
I'm going through the renderer side of things. The VID backend also
needs a lot of love...
It might be a good idea to move this SDL backend files into the client
and rename them. We'll decide that at a later time.
Until now, likely since we first introduced OGG/Vorbis playback 9 years
ago, in about 50% of all cases OGG_PlayTrack() was never called if
cd_shuffle was set 1, resulting in missing background music. Add the
missing call. :)
I wonder why I didn't catch this in sunday. For some reason a "make
clean ; make" cycle was necessary. Maybe a corner case that the header
dependencies didn't catch?
this happens when you just copypaste and adapt r_lefthand
also did some minor changes to R_AliasDrawModel in the soft renderer
to make sure alias[xy]scale is reset properly in the early out cases
At high 'fov' values the weapon looked quite distorted.
Now it's rendered with an independent FOV, which looks better.
Note that the 'fov' cvar sets fov_x, while this is based on fov_y
(which is calculated from fov_x), so it's indeed different values:
r_gunfov seems to correspond to fov 90.
We use r_gunfov 80 as default, because it looks better.
The old code was working only when the client was connected to a local
server. The 'newgame' executed by the menu expands to a 'map', loading
a map ends in SV_InitGame() which calls CL_Drop() on the local client.
That calls CL_Disconnect() and everything is okay.
When the client is already connected to a remote server and no local
server is running the 'map' command spawns a new local server. This
new server thinks "Hey, I'm a new local server and no one is connected
to me. Let's pull the client in!". So it pull the already connected
client onto a new server without disconnecting, smashing it's state.
And everything goes down in flames.
The correct way would be to execute a 'disconnect' right before the
'newgame'. But the 'disconnect' cmd calls CL_Disconnect_f that throws
an ERR_DROP. ERR_DROP is implememted through a longjump(), jumping
around puts the process internal state in ashes... So bite the bullet
and add another hack: Call CL_Disconnect() before executing 'newgame'.
The 'game' command was more or less functional after the last commit.
We just need to reset the initialGame (renamed to userGivenGame) so we
don't revert back to the old game at server disconnect.
When connecting to a multiplayer game that runs a different mod
("game" cvar) than you are, it didn't load the corresponging configs
from the mod, but saved your changes to the config to the mod's config.
Which is doubly useless.
Now when the "game" cvar is changed, the configs are reloaded (from
the right directories for the mod), and when disconnecting the configs
are written, so the changes you did for a mod while playing MP are saved
before game is reset to the game you started with.
The problem was that the cvars were only initialized (with CVar_Get())
if you opened the address book menu.
So if you start (and possibly run) and quit the game /without/ opening
that menu (or at least the "join network server" menu), the game will
not save those cvars to the config when it next writes it.
To prevent this, *always* initialize the cvars in M_Init().
On Unix platforms unicode is implemented through UTF-8 which is
transparent for applications. But on Windows a UTF-16 dialect is
used which needs alteration at application side. This wrapper is
another step to unicode support on Windows, now we can replace
fopen() by a function that converts our internal UTF-8 pathes to
Windows UTF-16 dialect.
This is a noop for Unix platforms. The Windows build is broken,
the compiler errors out in shared.h. This will be fixed in a
later commit.
Caveats:
* fopen() calls in 3rd party code (std_* and unzip) are not replaced.
This may become a problem. We need to check that.
* In the Unix specific code fopen() isn't replaced since it's not
necessayry.
There's no need to exclude directories from search by flags. In fact
the Unix backend has worked nicely for years without it... Sadly we
can't remove the now superfluous 'canhave' and 'musthave' attributes
from Sys_FindFirst() and Sys_FindNext() since they're defined in
shared.h and may be used from custom game DLLs.
