Post-processing effects test (SSAO/HDR/Bloom)

[dvangennip]

I suspect we are talking about different 'errors' The flickering is a known issue that can be attributed to existing code in the engine. The odd error messages you got ("preposterous datagram length" and all that), however, is not. So we are curious to know if these error messages happens with the vanilla GLSL renderer or only after you have applied myconid's changes.

Ah, that was the confusion No, those 'datagram length' errors have not come up in the vanilla code. I checked the game and Atlas.

Myconid explained the Alt+Enter issue, as for Alt dragging, perhaps the OS is using that keyboard combination? (I think at least some Linux window managers use that combination for moving the window or something, so it's at least a possibility) If that's the case you can always set the keyboard combination to something else, but if it's not it sounds like an actual bug which should be solved.

I do not know of such a key combination in OS X. The behaviour only shows in 0 A.D. Just to be clear: the window is not going anywhere (fullscreen), it is the in-world view that moves similar to what happens if the cursor would reach the edges of the screen. In itself it is not bad (no need to move the cursor a lot to adjust the view a little), but it makes it difficult to only select military units. If this behaviour is not in line with how it works on Windows and Linux I think it should be adjusted (if possible). As for not being able to play the game windowed, I would expect this to be a minor issue (that is to say, not so important to me to get fixed).

[myconid]

Oh well, since I can find literally no info on any of these errors, we'll have to do the digging ourselves. This line looks particularly interesting:

Sat Jul 28 21:42:43 s030454t.local pyrogenesis[36019] <Error>: kCGErrorFailure: Set a breakpoint @ CGErrorBreakpoint() to catch errors as they are logged.

You'll need to start the game with gdb (look for a tutorial if you don't know how, it's fairly easy). Of course, things will be much easier if you edit the game's config to set fullscreen to false.

You'll need to set a breakpoint on the "CGErrorBreakpoint" symbol (with "break CGErrorBreakpoint" in the gdb console), so the engine's execution will be paused when an error occurs. Type "bt" in gdb to get a backtrace and post it here. Then type "c" to resume execution, until another error occurs. When another error occurs, do the same thing.

There seem to be at least three different messages here: kCGErrorTypeCheck, kCGErrorIllegalArgument and kCGErrorRangeCheck. You need to resume and repeat for at least three times.

[dvangennip]

Oh well, since I can find literally no info on any of these errors, we'll have to do the digging ourselves.

I will have a look at it tomorrow, it is past midnight here. Will post the results if I manage.

[feneur]

As for not being able to play the game windowed, I would expect this to be a minor issue (that is to say, not so important to me to get fixed).

You should most definitely be able to run the game in windowed mode, see http://trac.wildfiregames.com/wiki/Manual_Settings for how to set it up, the bug should just be that it's not possible to change it while running the game.

[dvangennip]

You should most definitely be able to run the game in windowed mode, see http://trac.wildfire...Manual_Settings for how to set it up, the bug should just be that it's not possible to change it while running the game.

Found it! For everyone, set windowed=false in the config file.

You'll need to set a breakpoint on the "CGErrorBreakpoint" symbol (with "break CGErrorBreakpoint" in the gdb console), so the engine's execution will be paused when an error occurs. Type "bt" in gdb to get a backtrace and post it here. Then type "c" to resume execution, until another error occurs. When another error occurs, do the same thing.

There seem to be at least three different messages here: kCGErrorTypeCheck, kCGErrorIllegalArgument and kCGErrorRangeCheck. You need to resume and repeat for at least three times.

Using gdb was easy, so I have attached a logfile. Although I was persistent in continuing more than three times, the stack trace appears very, very similar for the errors. At some point I stopped the program though. Anyway, hopefully it gives a clue where to look further. I noticed today that the errors do not come up straight away, but rather after playing a game for about 10 to 15 (?) minutes.

patch_gdb_log.txt

[myconid]

Thanks. It appears these aren't graphics-related errors, but event handling errors from the window manager. They seem to be caused by either SDL or OSX, not Pyrogenesis:

#15 0x0000000100d62dd1 in QZ_PumpEvents ()
#16 0x0000000100d3bad8 in SDL_PumpEvents ()
#17 0x0000000100d3bf61 in SDL_PollEvent ()                  <- function in SDL to access input events
#18 0x0000000100017afd in PumpEvents () at ../../../source/main.cpp:181	  <- our code calls ^

where our code just does this:

SDL_Event_ ev;
while (SDL_PollEvent(&ev.ev))   <- this line, though there's no way the engine could cause this call to fail
{...

Why are these triggered by my changes? Good question, as there's nothing in my code that relates to event handling! The main difference from an event-handling perspective is that the drawing phase takes longer than before, so maybe this forces an unexpected SDL or OSX bug to the surface.

My instinct would be to open a bug report for this and let it be for now (especially as the errors don't seem to be harmful), though maybe that wouldn't be the best way to go about it. Any other programmers have thoughts on this?

[dvangennip]

My instinct would be to open a bug report for this and let it be for now (especially as the errors don't seem to be harmful), though maybe that wouldn't be the best way to go about it. Any other programmers have thoughts on this?

I will create a ticket in Trac for this, pointing back to this discussion/post. The errors come up at some point and normally, if no breakpoints are set, stop after about 20 of them (thus, nothing continuous it seems). While I was using gdb this afternoon, it appeared there were more errors (it didn't stop before my patience did ). Perhaps your suggestion that a temporary hickup (e.g., a frame taking more time or a breakpoint interruption) causes the errors is in the right direction? Anyway, the errors are non-fatal and I haven't noticed any negative side effects. So, especially if no one else has seen these errors, let it be.

Edit: Trac ticket #1567 was created for this issue.

Edited July 31, 2012 by dvangennip

[myconid]

Okay, 3/4 patches are completed and ready for review! 4th is easy and should be done by tomorrow at the latest.

Each patch is saved as a separate commit in this branch:

https://github.com/m...e/merged-patch4

Running a bit late for Alpha 11, I guess... but if someone can step up and review this, there might still be a chance.

Edit: Ok, 4/4 patches are ready. I don't want to commit anything to SVN without it being reviewed first, as these bring rather extensive changes so there's always a possibility of silly bugs that could be caught now. Please review them in the order committed, let me know what needs to be changed and I can take care of committing to SVN now that I have access.

[k776]

Well, there is two choices here. We can wait for someone with time to review them, and possibly not have them in Alpha 11.

Or, since they only affect GLSL and not ARB, commit each commit with it's own SVN commit, and then all SVN users get a chance to opt in to the GLSL version for testing.

We talked about the latter option, and I'm leaning toward it myself. Since GLSL isn't enabled by default, it won't affect much.

Plus, if we get them in now, we have a week to fix any issues we find.

[Echelon9]

Okay, 3/4 patches are completed and ready for review! 4th is easy and should be done by tomorrow at the latest.

Each patch is saved as a separate commit in this branch:

https://github.com/m...e/merged-patch4

Running a bit late for Alpha 11, I guess... but if someone can step up and review this, there might still be a chance.

Edit: Ok, 4/4 patches are ready. I don't want to commit anything to SVN without it being reviewed first, as these bring rather extensive changes so there's always a possibility of silly bugs that could be caught now. Please review them in the order committed, let me know what needs to be changed and I can take care of committing to SVN now that I have access.

In addition to k776's comments, a means of reasonably quickly parsing over the changes would be to pull in the help of some automated source code checkers.

PVS-Studio, MSVC's Analyze etc can be run on the current trunk 0ad, then with each of these patches applied, to pick up the delta.

Some reports will be gumph, but could pick out reasonably quickly things like incorrect use of arrays, copy/paste typos and the like.

I did a parse over the code base about 6 months ago with some of these tools. Might be unlikely for me to do so again this weekend, but next would be fine (assuming that is pre A11).

Edited August 5, 2012 by Echelon9

[wraitii]

I can run Xcode analyze (clang analyzer) and the few instruments (checking for leaks, mainly) if you want, it may help find a few stuffs.

I'm in favor of getting it in. The game is still Alpha, and testing cannot really hurt at this point.

[wraitii]

Okay, compiled successfully merged-patch4 (what's the difference with the number 1?). had to do the changes for OS X 10.8 but that's not linked.

A small error in the terrain_common.vs , lines 64-66 should be

  #if DECAL
    v_tex.xy = a_uv0.xy;
  #else

I'd make it explicit in the config file that gentangents = true is needed when using preferglsl=true.

Smoothlos works independently of preferglsl.

No noticeable errors, everything seems to work in-game, and I ran the clang analyzer and it showed nothing that seems linked to the patch/useful.

[myconid]

Okay, I'll commit the patches tonight. If there are mistakes, they'll show.

k776, this may affect ARB as well, as there have been a lot of changes to how things are done in the background. I'm most apprehensive about the changes having been tested quite extensively in GLSL mode but almost not at all in ARB mode.

Echelon9, wraitii, thanks for the suggestions and for testing.

[myconid]

Okay, compiled successfully merged-patch4 (what's the difference with the number 1?). had to do the changes for OS X 10.8 but that's not linked.

I'm rebasing stuff straight into the patches, so I do that as a crude way of keeping a history. Messy, but that's what I use git for.

A small error in the terrain_common.vs , lines 64-66 should be

  #if DECAL
	v_tex.xy = a_uv0.xy;
  #else

The error in terrain_common.vs was actually a typo in the declaration of a_uv0. For some stupid reason I typed vec3 instead of vec2... for UVs!

I'd make it explicit in the config file that gentangents = true is needed when using preferglsl=true.

Done.

Smoothlos works independently of preferglsl.

Smoothlos may look like it works independently of preferglsl, but it actually depends on a GLSL shader that has no ARB equivalent yet, so it won't work if your GPU doesn't support GLSL.

No noticeable errors, everything seems to work in-game, and I ran the clang analyzer and it showed nothing that seems linked to the patch/useful.

Thanks, glad to hear that!

[wraitii]

The estimated feature freeze for Alpha 11 is in 3 days. Unless you won't be here tomorrow/the day after tomorrow, I'd recommend you wait a bit before committing, the ARB aspect of your patch should be tested a bit more thoroughly (hopefully graphical stuffs are usually noticeable when they go wrong).

Do you have a "complete" list of features to check? (like windy trees, specular…)

[myconid]

Here's a few tests you can try:

Modelmapping: Look at Roman CC and Mars Temple 2 models for AO/parallax/normal/specular/emissive mapping. Look at windy trees when in the game and in Atlas after pressing "Play".

Smoothlos: Los should move smoothly around units. It should toggle correctly from the dev console. It should not be visible at all in Atlas.

Terrainmapping: "Biome alpine/Alpine cliff" uses triplanar, normal and specular mapping. "Biome desert/Desert city tile" uses a custom alphamap. "Grass/Grass1 spring fancy" uses the special grass material. "Snow/Snow grass 2" uses normal and specular mapping.

Heightmap: In atlas, File/Import heightmap. Choose an image file (not too huge!).

In the first three, also check if ARB and Fixed modes work as expected (there should be no visible changes). Pay special attention to decals, as I've had to modify those manually and since they are stored all over the place I might have missed some (I just spotted a broken decal in Campaign Test map, fixing that now).

[wraitii]

Tried all of those (but the grass. I couldn't find the proper material, perhaps you forgot to include it?). Works OK on GLSL (the trilinear mapping in particular looks magnificent). on ARB, the custom alphamap is still there but everything works. I did not notice a problem with decals.

Atlas and in-game work.

I'll try a game later on (busy with AI/unitAI stuff right now), but as far as I can tell there are no obvious bugs.

[myconid]

I did forget to include the grass. The custom alphamaps should work in ARB mode too, come to think of it.

Grepped through all the decals again, just to make sure.

Thanks for testing!

Latest code: https://github.com/myconid/0ad/tree/merged-patch5

[Enrique]

Okay, I'll commit the patches tonight. If there are mistakes, they'll show.

k776, this may affect ARB as well, as there have been a lot of changes to how things are done in the background. I'm most apprehensive about the changes having been tested quite extensively in GLSL mode but almost not at all in ARB mode.

Echelon9, wraitii, thanks for the suggestions and for testing.

This means we can test it just updating SVN? I want to test it and help but cannot compiling patchs

It's also nice to have a list with the basic tests we can perform as you did

The only thing I would add to that basic test list is how to switch from ARB to GLSL for more in-depth review

Thanks for your hardwork guys!

[Shield Bearer]

No, I think the patch has to be applied and then committed to the SVN trunk. Lets hope somebody does that soon

[myconid]

No, I think the patch has to be applied and then committed to the SVN trunk. Lets hope somebody does that soon

Actually, I have access to the internal SVN repo now, and I'm ready to commit everything. If that's what you're referring to, then expect it to be in by tomorrow evening (GMT)! I'm delaying in case someone tests the ARB mode and finds any glitches, though hopefully there won't be any.

Not sure if that repo autobuilds Windows binaries, though. I've seen the binaries in the "system" directory, but I don't know if they're kept up to date.

[Loki1950]

Don't know for sure but auto build seems broken ATM that last few exe updates have been manual builds.

Enjoy the Choice

[k776]

@myconid Looks all good from here. As long as you're available for say, an hour after committing, we can get a bunch of people to test it quickly for any obvious issues. And then reporting any smaller issues over the next day or two.

Re: Affecting ARB, any big changes like this are bound to have some issues somewhere. Again, as long as you're around the help fix any big ones soon after committing, and smaller ones the days after, shouldn't be a problem.

Re: preferglsl and gentangents, if they both have to be enabled for things to work, remove gentangents and use the same preferglsl variable. We don't want reports from people who have enabled one but not the other. I would stick the material manager settings under the preferglsl too, and comment the entire block of those settings as "## Experimental" or something similar.

And when you do commit, just remember to keep preferglsl on false for now. These big changes should be opt in until we know they are stable enough.

You have a green light for commit :-) Awesome work.

Edit: Also, before and after you commit, can you pop on IRC so we can report issues to you there in realtime? Thanks.

Go to http://webchat.quakenet.org/ and join channel #0ad-dev

[myconid]

Ok, I'll do that tomorrow, as it's getting pretty late here.

The gentangents option does serve a purpose, so I'll leave it in but make it even clearer that both are needed to use the new effects. My rationale:

1. Technically, tangents are not needed when GLSL is enabled but the normal/parallax materials aren't being used (i.e. when the material quality options are set low enough).
   
2. Tangents use extra GPU memory. Not huge amounts, just 16 bytes per model vertex, shared between all model instances. That's around 400K extra for a fairly complex model like the Roman CC. This can add up, and may cause problems for older GPUs with very limited memory - those aren't powerful enough to use the fancy materials, anyway.
   
3. The new effects will also get ported to ARB (wraitii has already done some work on that), so the option will be relevant there as well.
   

[Gen.Kenobi]

Question: Is this already documented on Trac?

Just wondering... hehehe