Naive AO is ready for a first look! #96

likangning93 · 2016-06-17T22:16:34Z

Run:

node ./bin/gltf-pipeline.js ./specs/data/ambientOcclusion/cube_over_ground.gltf --ao_diffuse=true --ao_samples=128 --ao_rayDepth=1.0 --ao_resolution=64 -o hello_AO.gltf

This takes a couple seconds on my machine, but it's a really small model.

This version of naiveAO outputs a gltf with a separate image for each primitive and adds materials and textures as needed. It won't work with models that don't have any diffuse-textured materials, though, since it tries to find reference materials and textures to clone.

Tasks for this PR

improve memory efficiency by raytracing "on the fly" instead of precomputing and storing all the sample points
improve sampling to avoid artifacts near the edges of triangles in UV space
profile the code to get an idea of how to make speed improvements

Tasks for upcoming PRs

evaluate speed benefits using a uniform grid for faster raytracing
add an option to bake the AO to vertices or a separate texture and automatically modify relevant shader code
evaluate the speed and memory benefits of moving the raytracing math to basic types instead of Cartesian3
GPU-accelerate with CUDA or WebGL rasterization, e.g. hemicube
- Expand to prebaking IBL?
UV preprocessing, checking, and cleanup: currently, this stage only works with primitives that already have normalized, non-overlapping UVs

coveralls · 2016-06-17T22:18:52Z

Coverage increased (+0.04%) to 94.183% when pulling eae1712 on naiveAO_PR into c86e629 on master.

likangning93 · 2016-06-18T02:01:07Z

Ongoing things that we'll have to deal with:

speed - the naive raytrace gets much slower as the number of triangles increases
memory efficiency - models like the milk truck crash the pipeline for me even with about 6 gb of memory allocated. This is a major problem.

likangning93 · 2016-06-18T02:43:42Z

Please refrain looking at this too deeply before next week. I might try to make some memory efficiency improvements before Monday, since frankly right now this is pretty alarming. I think the gist of the problem is in how I handle the texel points that the algorithm casts rays from. Currently at a high level the stage raytraces like this:

generate a soup of world-space triangles
rasterize every triangle in UV space to generate a massive cache of world-space texelPoints
for every texelPoint, cast out the requisite number of feeler rays

I think a big part of the problem is in the texelPoint cache - this thing must be huge, and it's not even necessary, mostly just an artifact from reading too much about hemicube irradiance baking.

There may also be some space efficiency problems in storing the triangles using cartesian3 but I'll tackle the texelPoint cache problem first.

pjcozzi · 2016-06-18T14:48:32Z

This is exciting news!

Can you please:

Post example with/without AO screenshots
Add a tasklist to the top of this PR with items that you want to do for this PR, e.g., memory efficiency, and a tasklist with roadmap items for after this PR, e.g., uniform grid, optimal AO map size, etc.

likangning93 · 2016-06-18T22:10:47Z

cube_over_ground.gltf" with and without AO:

pjcozzi · 2016-06-19T15:06:30Z

Cool, thanks @likangning93!

pjcozzi · 2016-06-19T15:11:52Z

improve sampling to avoid artifacts near the edges of triangles

I don't know that it is needed, but this could be of interest: http://jcgt.org/published/0002/01/05/

Tasks for upcoming PRs

Also add

GPU-accelerate with CUDA or WebGL rasterization, e.g. hemicube
- Expand to prebaking IBL?

These could also be 565 projects.

…t as it is computed

coveralls · 2016-06-20T02:10:29Z

Coverage increased (+0.07%) to 94.216% when pulling 375f969 on naiveAO_PR into c86e629 on master.

likangning93 · 2016-06-20T02:38:08Z

So with the texelPoint cache removed, running the AO pipeline with a bigger model like CesiumMilkTruck.gltf no longer causes dramatic and unsustainable increases in memory use, and the process can get to raytracing without crashing. I also want to profile if removing this cache also helps with speed, but I'll need to find a suitable model since just time node ... doesn't show any improvements for tiny models like the one I have in the specs folder. I'm going to run this on CesiumMilkTruck.gltf overnight just to see what happens.

likangning93 · 2016-06-20T02:51:15Z

I think something is wrong with how CesiumMilkTruck.gltf's UVs are being read. I'll take another look tomorrow.

likangning93 · 2016-06-20T13:43:48Z

The problem with the milk truck model is that its UVs aren't normalized, they're much bigger and get normalized in the shader using a hardcoded scaling parameter. The AO code currently does a pixel march over each triangle's AABB in UV space, so if the UV coordinates aren't normalized the march is 1) wrong and 2) seems to find far more "pixels" than it should. I'll throw an error for now if the pixel march code detects non-normalized UVs.

lasalvavida · 2016-06-20T13:47:35Z

@likangning93 You should be able to normalize them pretty easily if that makes sense.

likangning93 · 2016-06-20T13:56:35Z

@lasalvavida I should have been more specific about what I meant by normalized. The model's UV coordinates are in a space from (small)-(big) by (small)-(big), but I would have to look in the shader to figure out how to map back to 0-1 x 0-1. Here's a picture of the vertex shader:

I'm guessing this is a precision thing?

lasalvavida · 2016-06-20T14:02:25Z

As discussed offline, it looks like the milk truck model you have has oct-encoded normals. You should be able to grab a clean model from here.

likangning93 · 2016-06-20T14:02:38Z

Thanks Rob!

pjcozzi · 2016-06-20T14:14:54Z

Can we also add a stage to un-encode oct-encoded normals?

lasalvavida · 2016-06-20T14:18:28Z

Can we also add a stage to un-encode oct-encoded normals?

I can certainly decode the normal data easily enough, but un-modifying the shader is a pretty complex problem.

pjcozzi · 2016-06-20T14:31:32Z

I can certainly decode the normal data easily enough, but un-modifying the shader is a pretty complex problem.

Ah, right, I would not bother now since we have control of the pipeline and can just oct-encode after AO.

likangning93 · 2016-06-20T14:34:08Z

Some changes I need to make along with the first round of comments:

raytracing and baking shouldn't be done for every primitive in the gltf, it should be done for every primitive in the scene
remove some unnecessary memory allocations and move some more expensive math out of loops

lasalvavida · 2016-06-20T15:17:04Z

lib/bakeAmbientOcclusion.js

+
+function addNodeToSoup(parameters, node) {
+    var transform = node.extras._pipeline.flatTransform;
+    var mesheIDs = node.meshes;


Nitpick: should probably be meshIDs

Updated, changed in NodeHelpers.js

… to run over only primitives in the scene. also small changes for efficiency

coveralls · 2016-06-20T15:43:31Z

Coverage decreased (-0.1%) to 93.996% when pulling ebe5471 on naiveAO_PR into c86e629 on master.

…lpers.

likangning93 · 2016-06-20T17:27:28Z

Updated! I think this is actually ready for a deeper look now.

coveralls · 2016-06-20T17:28:09Z

Coverage decreased (-0.2%) to 93.983% when pulling 9efcfbd on naiveAO_PR into c86e629 on master.

lilleyse · 2016-06-20T21:40:25Z

bin/gltf-pipeline.js

+    ao_scene : ao_scene,
+    ao_rayDepth : ao_rayDepth,
+    ao_resolution : ao_resolution,
+    ao_samples : ao_samples,


Perhaps pack all the AO properties into an object. Also this makes it less verbose in gltfPipeline.

lilleyse · 2016-06-20T21:44:21Z

Pretty minor comments here, I like the organization of the AO stage, it was easy to follow. As you already noted in your comments, it would be good to handle the case of creating ao textures even if no diffuse textures are present. Great start here!

…ormed by inverse transpose

likangning93 · 2016-06-21T13:37:09Z

Updated!

coveralls · 2016-06-21T13:40:13Z

Coverage decreased (-0.06%) to 94.083% when pulling 9db1fae on naiveAO_PR into c86e629 on master.

lilleyse · 2016-06-21T13:56:38Z

lib/gltfPipeline.js

-        };
-        bakeAmbientOcclusion(gltfWithExtras, aoOptions);
-    }
+    bakeAmbientOcclusion(gltfWithExtras, options.aoOptions);


I think it's a little clearer and more in-line with the other stages if we check if we can run the ao stage before going into the function.

if (defined(options.aoOptions) && options.aoOptions.runAO) { bakeAmbientOcclusion(gltfWithExtras, options.aoOptions); }

And possibly just remove runAO altogether. As long as the ao options exist, it should be able to run the stage. This would require some edits in gltf-pipeline.js.

Oh that makes sense... no idea why someone would have aoOptions but then not run the AO.

…ation

likangning93 · 2016-06-21T14:20:55Z

Updated!

coveralls · 2016-06-21T14:24:14Z

Coverage decreased (-0.1%) to 94.021% when pulling 51ccfd1 on naiveAO_PR into c86e629 on master.

lilleyse · 2016-06-21T18:23:02Z

All good to me. Are there any other items on the checklist you plan on finishing before this is ready? They can all be separate PRs if you prefer.

In terms of the ao workflow, we should have a dedicated ao branch and all ao-related pull requests would go into that branch. When ao is "done", we'll merge the ao branch into master.

So once this is ready, can you close this PR, create an ao-master branch, then create a new PR with all these commits that targets ao-master (and just reference back to this PR so we know where all the code came from).

likangning93 · 2016-06-21T19:44:09Z

Last notes before closing:

I'm putting conservative rasterization aside for a few days to try baking per-vertex, since not all our models will have texture coordinates or textures
here are some results from baking:
models.zip
- the cube model in here demonstrates the need for conservative rasterization, you can see that no AO was baked to texels on some edges
- as a low bar for efficiency, the building model (normals mangled by blender) took 139 minutes, 47.361 seconds to bake at 128 samples per texel, 512x512 AO texels wit ha ray depth of 2.0. This model is 5521 triangles

likangning93 · 2016-06-21T19:51:15Z

moved to #102

likangning93 added 3 commits June 17, 2016 18:02

naiveAO ready for first review

0522743

added new files

6000cf5

added command line changes for AO

eae1712

eliminated texelPoints cache by raytracing the data in each texelPoin…

375f969

…t as it is computed

lasalvavida reviewed Jun 20, 2016
View reviewed changes

changed some steps that were running over every primitive in the gltf…

ebe5471

… to run over only primitives in the scene. also small changes for efficiency

addressed some pull request comments. cleaned up some comments and he…

9efcfbd

…lpers.

lilleyse reviewed Jun 20, 2016
View reviewed changes

addressed some PR issues: style, test typos, normals should be transf…

9db1fae

…ormed by inverse transpose

lilleyse reviewed Jun 21, 2016
View reviewed changes

addressed some PR issues: cleaned up AO specs and gltfPipeline integr…

51ccfd1

…ation

likangning93 closed this Jun 21, 2016

This was referenced Jun 21, 2016

merge naiveAO_PR to ao-master #101

Closed

merge naiveAO_PR to ao-master #102

Merged

Naive AO is ready for a first look! #96

Naive AO is ready for a first look! #96

Conversation

likangning93 commented Jun 17, 2016 • edited Loading

Tasks for this PR

Tasks for upcoming PRs

coveralls commented Jun 17, 2016

likangning93 commented Jun 18, 2016

likangning93 commented Jun 18, 2016 • edited Loading

pjcozzi commented Jun 18, 2016 • edited Loading

likangning93 commented Jun 18, 2016

pjcozzi commented Jun 19, 2016

pjcozzi commented Jun 19, 2016

coveralls commented Jun 20, 2016

likangning93 commented Jun 20, 2016

likangning93 commented Jun 20, 2016

likangning93 commented Jun 20, 2016

lasalvavida commented Jun 20, 2016

likangning93 commented Jun 20, 2016 • edited Loading

lasalvavida commented Jun 20, 2016

likangning93 commented Jun 20, 2016

pjcozzi commented Jun 20, 2016

lasalvavida commented Jun 20, 2016

pjcozzi commented Jun 20, 2016

likangning93 commented Jun 20, 2016

lasalvavida Jun 20, 2016

Choose a reason for hiding this comment

likangning93 Jun 20, 2016

Choose a reason for hiding this comment

coveralls commented Jun 20, 2016

likangning93 commented Jun 20, 2016

coveralls commented Jun 20, 2016

lilleyse Jun 20, 2016

Choose a reason for hiding this comment

lilleyse commented Jun 20, 2016

likangning93 commented Jun 21, 2016

coveralls commented Jun 21, 2016

lilleyse Jun 21, 2016

Choose a reason for hiding this comment

likangning93 Jun 21, 2016

Choose a reason for hiding this comment

likangning93 commented Jun 21, 2016

coveralls commented Jun 21, 2016

lilleyse commented Jun 21, 2016

likangning93 commented Jun 21, 2016

likangning93 commented Jun 21, 2016

likangning93 commented Jun 17, 2016 •

edited

Loading

likangning93 commented Jun 18, 2016 •

edited

Loading

pjcozzi commented Jun 18, 2016 •

edited

Loading

likangning93 commented Jun 20, 2016 •

edited

Loading