Octahedral Mapping Tests

[OmarShehata]

This adds a test for the OctahedralProjectedCubeMap.js class. I also removed the temporary +Y/-Y swap from the KTX loader.

For the sample environment map, can we use something that doesn't have a visible sun, maybe something like this ?

Speaking of which, @bagnell can you write down some steps for generating these KTX files and the spherical harmonics? It might be useful in putting together a tutorial so users know how to use this until there are tools that can export this extension.

[cesium-concierge]

Thanks for the pull request @OmarShehata!

• ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

---

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

[bagnell]

Can you add more tests? For example, check that a bunch of samples from the octahedral map are close to samples from the original cube map. Also, do the same for the mipmaps.

[OmarShehata]

@bagnell I added a test to sample all 6 directions, for every mip level, and confirm that the octahedral map is pretty close to the original cubemap color. I had to flip the Y to account for the +Y/-Y face swap.

I also had to add a contextRenderAndReadPixels function since I couldn't see a way to get the rendered color from a context. I was trying to refactor expectContextToRender to use this new function instead of having this duplicated code, but the fact that it checks the cleared color in addition to checking the rendered color makes it a little hard to refactor as-is. I feel like this shouldn't be making these two checks in the same function.

It also seems to be using CesiumMath.equalsEpsilon on each individual color channel as opposed to the more general toEqualEpsilon, so I'm guessing this is an older function?

[bagnell]

This should be it instead of fit.

[bagnell]

Use the scene method here instead.

[bagnell]

Also use the scene here. You can get scene.context for the rest of the test.

[bagnell]

but the fact that it checks the cleared color in addition to checking the rendered color makes it a little hard to refactor as-is. I feel like this shouldn't be making these two checks in the same function.

Why not? The idea is that the framebuffer should be cleared to check that the command executed actual renders the right color. If it isn't cleared, the color might be the same from a previous render.

[OmarShehata]

Why not? The idea is that the framebuffer should be cleared to check that the command executed actual renders the right color. If it isn't cleared, the color might be the same from a previous render.

Wouldn't it work just as well if that was a single separate test to check that the clear command works, instead of every color test also checking it?

That would allow me to refactor expectContextToRender to just use contextRenderAndReadPixels, and the latter would always return a color. And then add a separate test somewhere that clear works.

[bagnell]

I was trying to refactor expectContextToRender to use this new function instead of having this duplicated code

Could you also do this? You could have a function return the clear color and render color. If the test isn't supposed to clear first, then don't clear and return undefined. Your test should also clear before rendering the cube map face and before the sampling the octahedral projection.

[bagnell]

Wouldn't it work just as well if that was a single separate test to check that the clear command works, instead of every color test also checking it?

I don't understand. There is a test to check that a clear works. The clear before render is to prevent something like this:

it('test 1', function() {
    // set up
    expect(scene).toRender(color);
});
it('test 2', function() {
    // set up
    expect(scene).toRender(color);
});

Now, say test 1 runs first and renders color. Now when test 2 runs and doesn't render anything. If the color wasn't cleared, it will pass and that's a bug.

[OmarShehata]

Now, say test 1 runs first and renders color. Now when test 2 runs and doesn't render anything. If the color wasn't cleared, it will pass and that's a bug.

In that scenario, test 2 would pass, but the "check that a clear works" would fail, right?

[bagnell]

but the "check that a clear works" would fail, right?

In that scenario, say the clear test runs before or after both of those tests execute. Now all of the tests still pass when test 2 should fail.

[OmarShehata]

So it's possible for the clear test to pass, but actually clearing could fail in individual tests?

[bagnell]

No, the scenario was to show what could happen if there is no clear before every render.

[OmarShehata]

So yes, we're still going to clear before each test, it's just that testing that it correctly clears in every test is redundant, but if this redundancy helps here then I'm all for it. Just wanted to make sure we were on the same page.

@bagnell fixed the fit, explained why we have to execute the commands in this test as opposed to using scene methods, and I refactor the matchers. It now returns the rendered color as well as the clear color. Everything should work as before, and I confirmed before/after that the same tests are failing (the same 10 failures that are already in this branch).