Fix Firefox warnings #6297

bagnell · 2018-03-01T21:16:27Z

Same as #5939 but also fixes the flipped water mask.

TODO

Add tests if existing tests do not cover all new code paths

cesium-concierge · 2018-03-01T21:16:29Z

@bagnell, thanks for the pull request! Maintainers, we have a signed CLA from @bagnell, so you can review this at any time.

⚠️ I noticed that CHANGES.md has not been updated. If this change updates the public API in any way, fixes a bug, or makes any non-trivial update, please add a bullet point to CHANGES.md and comment on this pull request so we know it was updated. For more info, see the Pull Request Guidelines.

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

🌍 🌎 🌏

bagnell · 2018-03-01T21:20:40Z

@lilleyse I fixed the water mask in 4c81e8c. The other option is to flip the array buffer source in Texture. It might be confusing to have the flipY option and the texture is never flipped unless the source is a DOM element.

lilleyse · 2018-03-01T21:26:22Z

Yeah maybe in Texture makes more sense.

Are there other textures besides water mask that are defaulting to flipy? And maybe we have more control to fix them.

bagnell · 2018-03-01T21:28:11Z

Are there other textures besides water mask that are defaulting to flipy? And maybe we have more control to fix them.

I'm not sure. flipY defaults to true though. I'll add it to texture.

lilleyse · 2018-03-01T21:36:01Z

One example:

With Cesium3DTileBatchTable we can edit the 1.0 - in the shader and send in flipY : false, avoiding the flipping code completely.

We should try to find the other areas like this.

mramato · 2018-03-02T21:21:34Z

Source/Core/PixelFormat.js

@@ -150,6 +150,7 @@ define([
                // Many GPUs store RGB as RGBA internally
                // https://devtalk.nvidia.com/default/topic/699479/general-graphics-programming/rgb-auto-converted-to-rgba/post/4142379/#4142379


Since we return 3 now, should this comment be removed?

I'm surprised having it as 4 didn't cause any problems since we use this to create arrays. I guess we don't use RGB textures at all. I kept this there for now because I plan on moving it to a separate function (for when we measure GPU memory usage)

pjcozzi · 2018-03-11T16:17:13Z

Source/Core/PixelFormat.js

+            var textureWidth = width * numberOfComponents;
+            for (var i = 0; i < height; ++i) {
+                for (var j = 0; j < textureWidth; ++j) {
+                    flipped[(height - i - 1) * height + j] = bufferView[i * height + j];


Why not factor (height - i - 1) * height and i * height out of the inter loop?

pjcozzi · 2018-03-11T16:19:13Z

Added above:

[ ] Add tests if existing tests do not cover all new code paths

bagnell · 2018-03-26T21:05:25Z

@lilleyse Can you look at Cesium3DTileBatchTable renders with featuresLength greater than maximumTextureSize test. It is the only remaining test that fails (causes others to fail by not resetting maximumTextureSize). The other tests were fixed in #6370.

The texture creation doesn't fail for the batch table, but when creating the BRDF LUT. How does this pass in master?

bagnell · 2018-03-26T21:53:46Z

This is ready with the exception of that one failing test.

lilleyse · 2018-03-27T16:28:57Z

The code looks good, but I still think we need to scrub through the code to avoid flipY as much as we can. For example hover over picking will repeatedly call the flip code and incur a bunch of overhead.

Fix Firefox warnings #6297 (comment)

lilleyse · 2018-03-27T17:32:37Z

Cesium3DTileBatchTable renders with featuresLength greater than maximumTextureSize test

I started to look into this...
If frameState.brdfLutGenerator.update(frameState); is commented out the test still fails. It passes when using gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, flipY); instead of the new flipY function so maybe there is something wrong there?

bagnell · 2018-03-27T19:32:20Z

@lilleyse This is ready.

Also if you run this test standalone in master it fails:
http://localhost:8080/Specs/SpecRunner.html?spec=Scene%2FCesium3DTileBatchTable%20renders%20with%20featuresLength%20greater%20than%20maximumTextureSize

lilleyse · 2018-04-04T20:59:22Z

I looked around to see if any other areas could set flipY : false in addition to Cesium3DTileBatchTable. We can do it in:

BatchTable
OIT (unless we just remove the workaround in Firefox OIT fix #4762)
GlobeSurfaceTile for the water mask. We could invert the .y coordinate in GlobeFS instead.

bagnell · 2018-04-05T17:59:29Z

@lilleyse Updated.

bagnell · 2018-04-05T18:00:09Z

Fixes #4815.

lilleyse · 2018-04-05T18:31:44Z

Now I always see the warning:

Error: WebGL warning: drawElements: This operation requires zeroing texture data. This is slow.

I turned OIT off and it didn't appear. I guess we should continue to keep the OIT workaround code in #4762 and even close #4815.

bagnell · 2018-04-05T20:15:15Z

@lilleyse Updated. Only the second color texture needed to be zeroed out. Seems like a Firefox bug.

lilleyse · 2018-04-05T20:21:37Z

Ok - looks good!

bagnell added 2 commits March 1, 2018 15:47

Re-add changes to fix Firefox warnings.

fe326f7

Flip water mask.

4c81e8c

Update CHANGES.md.

3588b29

Flip y for array buffer views when creating a texture or cubemap.

fbd2ed8

mramato reviewed Mar 2, 2018

View reviewed changes

pjcozzi reviewed Mar 11, 2018

View reviewed changes

bagnell added 4 commits March 26, 2018 15:00

Updates from review.

70a1594

Merge branch 'master' into warnings

b77fb4e

fix clipping plane collection tests.

b61bb41

Fix remaining tests.

7fcd57c

Add tests for new code paths in texture abd cube map.

f3e2099

bagnell added 4 commits March 27, 2018 14:54

Do not flip batch table texture.

6c1149b

Merge branch 'master' into warnings

a31ed73

Fix failing tests.

0bd4045

Merge branch 'master' into warnings

1923f4e

bagnell added 2 commits April 2, 2018 16:49

Merge branch 'master' into warnings

95ea7fa

Update CHANGES.md.

57790af

Do not flip water mask, batch table or OIT framebuffer textures.

07e52da

Merge branch 'master' into warnings

b9d3ed0

Fix CHANGES.md merge

229f3a6

Fix OIT warning.

295372d

lilleyse merged commit 9259693 into master Apr 5, 2018

lilleyse deleted the warnings branch April 5, 2018 20:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Firefox warnings #6297

Fix Firefox warnings #6297

bagnell commented Mar 1, 2018 •

edited

Loading

cesium-concierge commented Mar 1, 2018

bagnell commented Mar 1, 2018

lilleyse commented Mar 1, 2018

bagnell commented Mar 1, 2018

lilleyse commented Mar 1, 2018

mramato Mar 2, 2018

bagnell Mar 2, 2018

pjcozzi Mar 11, 2018

pjcozzi commented Mar 11, 2018

bagnell commented Mar 26, 2018

bagnell commented Mar 26, 2018

lilleyse commented Mar 27, 2018

lilleyse commented Mar 27, 2018

bagnell commented Mar 27, 2018

lilleyse commented Apr 4, 2018 •

edited

Loading

bagnell commented Apr 5, 2018

bagnell commented Apr 5, 2018

lilleyse commented Apr 5, 2018 •

edited

Loading

bagnell commented Apr 5, 2018

lilleyse commented Apr 5, 2018

		@@ -150,6 +150,7 @@ define([
		// Many GPUs store RGB as RGBA internally
		// https://devtalk.nvidia.com/default/topic/699479/general-graphics-programming/rgb-auto-converted-to-rgba/post/4142379/#4142379

Fix Firefox warnings #6297

Fix Firefox warnings #6297

Conversation

bagnell commented Mar 1, 2018 • edited Loading

cesium-concierge commented Mar 1, 2018

bagnell commented Mar 1, 2018

lilleyse commented Mar 1, 2018

bagnell commented Mar 1, 2018

lilleyse commented Mar 1, 2018

mramato Mar 2, 2018

Choose a reason for hiding this comment

bagnell Mar 2, 2018

Choose a reason for hiding this comment

pjcozzi Mar 11, 2018

Choose a reason for hiding this comment

pjcozzi commented Mar 11, 2018

bagnell commented Mar 26, 2018

bagnell commented Mar 26, 2018

lilleyse commented Mar 27, 2018

lilleyse commented Mar 27, 2018

bagnell commented Mar 27, 2018

lilleyse commented Apr 4, 2018 • edited Loading

bagnell commented Apr 5, 2018

bagnell commented Apr 5, 2018

lilleyse commented Apr 5, 2018 • edited Loading

bagnell commented Apr 5, 2018

lilleyse commented Apr 5, 2018

bagnell commented Mar 1, 2018 •

edited

Loading

lilleyse commented Apr 4, 2018 •

edited

Loading

lilleyse commented Apr 5, 2018 •

edited

Loading