Work Blog - JeffR

456 posts Page 38 of 46
Posts: 87
Joined: Sat Feb 07, 2015 1:29 am
by HeadClot » Tue Jun 11, 2019 5:01 pm
Will have an update tomorrow, both on this workblog and the preview build!
Looking forward to it :D
Posts: 957
Joined: Tue Feb 03, 2015 9:49 pm
by JeffR » Wed Jun 12, 2019 8:15 am
Time for a work blog update!

This one isn't going to be QUITE as dense as the norm for reasons I'll get into shortly, but I can show off some nice images to compensate, so hopefully that'll all tide things over.

Preview Build Status
You guys have been finding a deluge of issues, bugs and suggestions for improvements, and it's been a gigantic help in refining/pinning all the problem spots down. I ran into a bug I need to fix before I can push the new build, so that'll actually happen tomorrow, but luckily that's just stupid UI shenanigans, versus the big bug me and az had been chasing for the past 2 weeks or so.

A lot of you noted an inconsistent, but not-one-off crash that would happen upon mission load. It didn't always happen, and - joy of joys - it only happened in release or optimized debug builds. We pretty quickly pegged it down to being related to the Skylight object, but unfortunately that would be the last definitive mark we had until just this last sunday. What followed was a nightly effort of walking back through the commit history for a few months, seeing if any obvious change would have an impact, building test cases, tossing in feedback comments into the code and dissassembling the entire probe renderer and shader piece by piece to try and isolate it down.

In the end, the entirety of the problem can be conveyed with this code, which will not merely 'crash', but actually triggers undefined behavior in the hlsl compiler, usually - but not always - resulting in a DEVICE_HUNG error from D3D itself, causing the render context to abort, killing the program AND potentially causing rendering instabilities for other applications on the desktop
float numProbes = 0;
for(int i=0; i < numProbes; i++)
   //do stuff
There ya go. Feel free to try and spot the error that would cause such horrible results. I'll wait.

Done looking? If you didn't find it, don't worry, as just covered it took me and az a few weeks to pin down, so here it is:

Turns out, the HLSL compiler really, really, really dislikes the conditional in a loop causing an early out of the loop

That's it. If we changed numProbes to literally anything, it ran right as rain. It being 0 (in the event that you had a skylight - so the system would render the probes, but there was only the skylight and no normal probes, thus numProbes = 0) then it would hit the loops where we normally process the regular probes, the hlsl would wig out, and cause undefined behavior. Sometimes it worked, other times it caused a device hung crash.

So.....that's cool. Naturally this didn't happen in OpenGL either, which was only slightly helpful in the debug hunt as it told us the rest of our code was probably fine.

Still, that ended up being ever-so-slightly maddening until we had a " couldn't be that dumb, could it?" But oh, it could indeed.

So the good news is, that's fixed, the next build should be crash free, and we even made various parts of the code cleaner and more streamlined while chasing stuff down, so at least it wasn't a waste. Pushed the intended build iteration schedule back a good bit, but it's done and we never have to deal with that again :)

In addition to that being fixed, I put together a number of fixes, tweaks and refinements to the asset pipeline stuff and with PBR working proper again and the refinements to the pipeline stuff, I started working out a workflow for PBR-ification of existing, non-PBR assets. We have a gigantic backlog of old resources, art content and the like we can bring up to spec for the new stuff, and this process could also be useful for people to convert their old project's art to work with PBR too.

Not fully locked in documentation-wise just yet, but I've got a decent feel of it now, so I've been testing on a range of different art stuffs. I've got some screens below to show how that looks with surprisingly minimal effort(though in fairness, I'm using the Allegorithmic Substance tools, which do like 90% of the work for basic conversion stuffs)

So for some beauty shots:

This was me importing an old, classicly made(ie has all the shadows/highlights built in, no normal or PBR information, pure diffuse) texture, resolution 256x256:


Took only a couple seconds - just dropped it in, it guessed a pretty good estimation of the roughness/metalness, generated the normal and even upscaled the images a bit. A few minor tweaks to roughness and it was a pretty good representation of rough concrete, given it's humble origins.

So converting tileable environment textures is easy, lets get a better worst-case scenario: character art.

I've got 3 examples. The first is an old WW2 soldier, again, classically made, no normals, PBR mats and all lighting was originally painted in the diffuse:


Slightly high resolution textures, but the main thing is because it's not one uniform material, a slightly different approach was needed. I loaded it up into Substance Painter, and did layers over the original diffuse, merely changing the brushes with appropriate materials and setting them so you only paint the roughness/metalness values to the material. This leaves the original colors intact.

The results actually look surprisingly decent, again, considering what it was. I'd still need to generate the normals for him though.

Next, you'll likely recognize these two handsome devils:


They got a similar treatment of merely painting over with PBR values. The normal maps and color maps were otherwise untouched, and I think it shows here a little more strongly. They look really soft, and Space Orc's normal maps are especially soft and poorly defined edges just makes them look overly soft and almost bloomed/blurry.

But the materials are a decent representation, so if we redid the normals, I think they'd honestly look pretty good. You can definitely tell how much more realistic lighting sells the art, even with minimal changes to it.

Leaning on this, I'm going to work at a new tester map using these old converted textures and models(partially because they're already on-hand) and build out a new, proper all-encompassing test level).

Also going to work on PBR-ifying a few more solid example cases, likely one of Ron's old art packs, and the Chinatown demo to really show much much good lighting can contribute to otherwise 'old' assets.

Beyond that, one thing that'd been bugging me something fierce is editor settings. I'd come up adding to/expanding on the editor settings as well as having a 'project settings' to inform common paths and other default values that are really important for the game to load/run right, but avoiding them being shoved into a random global var in a random script file you have to try and hunt down.

So I took a stab at updating that end of things from a UI perspective and I think the results are promising so far:

Old editor setting window(which you guys may not even realize existed)

And the updated:

It's utilizing the Variable Inspector so it standardizies the entire layout and makes everything much, much, muuuuuuch cleaner and easier to work with - no more half-dozen custom guiControls to fill out the settings editor.

I'll also be moving the asset import configuration logic over to this setup as well, so that'll cut the code related to THAT down to about a third of it's current size, so it'll be leaner, cleaner and less duplications meaning less places it can break.

The Editor/Project settings stuff will, going forward, be able to act as a proper central point for adjustments of their respective settings, and we can shift a lot of the random global vars over to be actual settings that get tracked and saved in a consistent way.
Plus, we'll be able to add a number of cool things to it going forward that should just be awesome for quality of life. I've already done a few test runs at having the editor theme adjustable via settings in this editor, as well as plotting out keybind editing so we could have a list of changable keybinds for various editor actions instead of them being hardcoded off in some random file somewhere.

So yeah, less than the norm due to hellbeast bug chases, but I'll fix that UI issue and get a new build punted up tomorrow(hopefully with the mac build as well since the windows build won't be bogging things down now) so we can get back to refining the crap out of this bad boy :)

Posts: 378
Joined: Thu Feb 05, 2015 3:20 am
by Timmy » Wed Jun 12, 2019 8:58 am
That HLSL example above would compile out if hard coded to 0 like that, i take it you actually mean passing in numProbes as a shader uniform right?

Is there anything else you are doing funky with the uniform? I know our PBR versions branched off in different directions quite awhile ago now, but i use a very similar for loop and it operates just fine passing in 0 for the probe looping.

Actually there is one difference, i pass my probe count in as an integer not a float, i wouldn't be surprised if something is going on with that. Using a float you will be causing it do some sort of conversion on it to be able to compare to an interger, check the compiled assembly, it might even go the other other and convert the integer to a float for comparison?
Last edited by Timmy on Wed Jun 12, 2019 9:33 am, edited 1 time in total.
Posts: 497
Joined: Tue Feb 03, 2015 9:50 pm
by Azaezel » Wed Jun 12, 2019 9:20 am ... r.cpp#L782
mProbeArrayEffect->setShaderConst("$numProbes", (float)mMax(mEffectiveProbeCount,1.0));
with otherwise identical code did indeed serve to bypass the flaw. which is still.. bizarre as <insert string of swearing here>

Edit: should note this *only* cropped up in release/releasewdebuginfo mode, not debug, which initially led us to believe it may have been something with initialization. Still might, but couldn't for the life of me see how.
Posts: 378
Joined: Thu Feb 05, 2015 3:20 am
by Timmy » Wed Jun 12, 2019 9:33 am
Check my edit above
Posts: 378
Joined: Thu Feb 05, 2015 3:20 am
by Timmy » Wed Jun 12, 2019 9:41 am
Yeah have a look here ... a89505770e

Just compare the int/float numProbes output, the float actually makes it use one more instruction, it uses itof which is integer to float . Try changing the uniform to an integer in your code and see if the same problem happens
Posts: 497
Joined: Tue Feb 03, 2015 9:50 pm
by Azaezel » Wed Jun 12, 2019 9:48 am
mProbeArrayEffect->setShaderConst("$numProbes", mEffectiveProbeCount);
uniform int numProbes;

does indeed seem to have done the trick as far as the hang goes. still doesn't quite explain why it went nearly that batshit, but thanks for a much saner fix.
Posts: 378
Joined: Thu Feb 05, 2015 3:20 am
by Timmy » Wed Jun 12, 2019 10:01 am
Ok sweet it was that, the other plus is you will save 1 instruction for every probe loop :). I can't answer why it fails hard but the other difference in the asm output is the comparison function

ge: ... sm4---asm-
ige: ... sm4---asm-

I would have to pass this onto someone who actually understands asm better than me to know why it crashed hard, it could well be a driver bug? did it happen across both amd and nvidia?
Posts: 957
Joined: Tue Feb 03, 2015 9:49 pm
by JeffR » Wed Jun 12, 2019 3:47 pm
Ok, sweet, excellent spot there Timmy :D

I hadn't considered it was actually the cast/conversion rather than the loop logic gate itself. Still falls under the 'why would it do that' but hey, aces either way.
And good suggestion on verifying if it's both AMD and nVidia. Definitely happens on nVidia, but I dunno if the other guys running the build had AMDs or not. Going forward, probably worth having people append their computer info onto stability/crash bugs just to get a better scope of that.

I'll do a run on an AMD card for sure this week and then based on the results try and find the appropriate guys to shoot a bugreport to. Whether it's D3D or it's the graphics driver, it's definitely a nasty little bug that should get sorted.
Posts: 63
Joined: Sat Mar 07, 2015 3:13 pm
by XIXWYRMEXIX » Wed Jun 12, 2019 4:25 pm
User avatar
As I posted when you first released the pre RC build I had no crashes. Perhaps I did not do what others were doing and thus did not encounter anything, but it was stable for me. As I posted in my original response, I am on a full AMD rig. Ryzen 5 8 core, RX 580 4gb video card, windows 10. No crashes in my playing around with either the first or the second build. I will admit I did not run a full level or anything, just played with both options and played around with a few add on things (guns, a dude to shoot, etc.).

P.S. great job on it for everyone involved! It looks amazing!
456 posts Page 38 of 46

Who is online

Users browsing this forum: No registered users and 1 guest