Aberations in graphics

[abs1nth]

btw the call to remove the SDL_Free(surface) was removed because the documentation says we shouldn't do it

(The surface returned is freed by SDL_Quit and should not be freed by the caller. Note: This rule includes consecutive calls to SDL_SetVideoMode (i.e. resize or rez change) - the pre-existing surface will be released automatically.)

[joevenzon]

p.s.: Is there a way to have the build number built into the binary and displayable? Maybe stick it under the FPS counter in a small font or maybe in the menus somewhere? Again, JAT.

This is an excellent idea. I'll put it on my TODO list (although if someone else gets around to it before me that'd be great too).

btw the call to remove the SDL_Free(surface) was removed because the documentation says we shouldn't do it

Good catch. Thanks for fixing that.

SDL on Mac OS X is the same as on Windows and neccisitates texture reloads.

This is my understanding as well. It's odd that the second reload is the one that seems to break things... if one reload doesn't cause odd display issues, then it sounds like we're reloading all of our textures properly. I wonder if this happens on Windows, also...?

[rookie1]

p.s.: Is there a way to have the build number built into the binary and displayable? Maybe stick it under the FPS counter in a small font or maybe in the menus somewhere? Again, JAT.

This is an excellent idea. I'll put it on my TODO list (although if someone else gets around to it before me that'd be great too).

Slightly OT - I've changed the scons script to include the svn revision number. Now the revision number is displayed at bottom right corner of the menu screen.

[rookie1]

Slightly OT - I've changed the scons script to include the svn revision number. Now the revision number is displayed at bottom right corner of the menu screen.

Just noticed that with the change I made to scons script, everytime there is a revision change, all the source files are getting recompiled. I think this is due to the -DREVISION the script added. In fact, REVISION is only need to be defined for main.cpp. Is there way to specify in the scons script that -DREVISION should only be added when compiling main.cpp?

[thelusiv]

This is why I made it so that when release=0 then the version is "development" instead of the date. It caused the game to be fully recompiled every 24 hours.

I think I might know how to code a fix with an extra header file generated at compile time. I'll play around with it and see if I can make it work...

[thelusiv]

I checked something (r1561) in that sort of fixes this. Actually it's more of a trade-off. I set up an automatically generated header file "definitions.h" that now contains all the #define statements that used to be -D flags for the compiler. Now, fewer files have to be recompiled every time something changes in this file. It still is a lot...but it's not quite all of them. Let me know if you guys like this better than recompiling all the files every time a define changes.

[joevenzon]

I noticed in the latest SVN version, the display menu doesn't reload the graphics at all. I realize that people were having throuble reloading twice, but could we still reload after hitting okay at the graphics menu (but not the advanced menu)? Or does this cause errors for mac/windows people too?

[thelusiv]

Nathan (FFuser) has told me in IRC that reloading textures in Windows is the same problem as in OS X - OK the first time, but messed up the second time.

Rather than try to guess which menu to reload textures in, etc., we need to just make the game restart after changing display settings once (except for in Linux where this isn't necessary). In order to make the game restart itself, we'd need to change a lot...like make main into a class, that can be re-initialized. That's a lot of stuff I'd like to save for the next release cycle, so for now we could just do as we have before, and force the user to quit after changing display options once.

Also the ChangeDisplay function could be smarter, and only reload textures when it's necessary for some reason (I noticed the other day that this isn't quite the case - it doesn't seem to reload textures when I change the texure size in the advanced menu). We can't assume that users won't press OK in the Advanced Display Options menu, and then press Cancel in the Display Options menu. This is perfectly valid, and in the case where someone's only changing texture size on the Advanced menu, then pressed OK, and then Cancel, the changes would not be applied.

[joevenzon]

Rather than try to guess which menu to reload textures in, etc., we need to just make the game restart after changing display settings once (except for in Linux where this isn't necessary). In order to make the game restart itself, we'd need to change a lot...like make main into a class, that can be re-initialized.

I don't quite follow you here. The program shouldn't need to restart itself, it just needs to re-init everything, which is what it used to do whenever you clicked OK in the graphics options.

We can't assume that users won't press OK in the Advanced Display Options menu, and then press Cancel in the Display Options menu.

I realize this would be a band-aid, but it seems better than just doing nothing. Your idea of having a "game must be restarted before changes are applied" dialog pop up works, too.

I think the ideal would be to not re-init things when you leave the advanced screen and only re-init things when you leave the graphics options screen according to the following logic:

if OK on graphics screen: re-init
if CANCEL on graphics screen: re-init if OK was pressed on the advanced screen

[thelusiv]

Rather than try to guess which menu to reload textures in, etc., we need to just make the game restart after changing display settings once (except for in Linux where this isn't necessary). In order to make the game restart itself, we'd need to change a lot...like make main into a class, that can be re-initialized.

I don't quite follow you here. The program shouldn't need to restart itself, it just needs to re-init everything, which is what it used to do whenever you clicked OK in the graphics options.

What I mean, is that on Windows and OS X, if you change the resolution twice, the graphics get messed up. If you restart the game things are better again. So if we had a way to destroy our SDL Window, and create a new one, that would be (I think) the same thing as a complete restart. This would be pretty easily possible if we abstracted most of the stuff in main.cpp into a manager class that could be reinitialized itself. Does that make more sense?

I realize this would be a band-aid, but it seems better than just doing nothing. Your idea of having a "game must be restarted before changes are applied" dialog pop up works, too.

I'll set up one of those for now. This is how we have solved the problem in the past and I think it works fairly well.

[joevenzon]

So if we had a way to destroy our SDL Window, and create a new one, that would be (I think) the same thing as a complete restart.

That's the purpose of the ChangeDisplay() function in main.cpp. We can experiment with completely de-initting SDL there also... but I don't want to do anything too drastic before the next release.

As a side note, ChangeDisplay used to destroy the main SDL surface, but this was changed in R1534 by abs1nth (see above posts), and I think I'm going to change it back to destroying the surface. Although the SDL docs say not to free the surface because it gets freed by SDL_Quit, I think only the surface from the last SetVideoMode call gets freed, and I think we'll end up with a memory leak and other problems if we don't free the surface before replacing it with a new SetVideoMode call. If this makes anything worse for you on your platform, abs1nth, then let me know, but otherwise I'm going to keep it this way.

[abs1nth]

[quote="joevenzon"][quote]Although the SDL docs say not to free the surface because it gets freed by SDL_Quit, I think only the surface from the last SetVideoMode call gets freed, and I think we'll end up with a memory leak and other problems if we don't free the surface before replacing it with a new SetVideoMode call.[/quote]

are you sure? "Note: This rule includes consecutive calls to SDL_SetVideoMode (i.e. resize or rez change) - the pre-existing surface will be released automatically."

also the source (http://www.libsdl.org/cgi/viewvc.cgi/tag...iew=markup) indicates that the documentation is right.

[quote="joevenzon"][quote]
If this makes anything worse for you on your platform, abs1nth, then let me know, but otherwise I'm going to keep it this way.[/quote]

ok
i doubt it will make anything worse since it was the default until i changed it a few days ago.

[joevenzon]

Alright, alright, you're right. I'll change it back.