CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
12.99k stars 3.5k forks source link

Failures in Firefox #464

Closed mramato closed 11 years ago

mramato commented 11 years ago

Firefox 18 seems to have broken unit tests at head of master.

Scene/EllipsoidPrimitive renders with the default material. Scene/PolylineCollection renders more than 64K vertexes of same polyline. Scene/PolylineCollection renders more than 64K vertexes of different polylines. Scene/PolylineCollection renders more than 64K vertexes of different polylines of different widths.

@pjcozzi suspects they added some extra error checking to drawElements to conform (or incorrectly conform) to the WebGL spec. Here's his thoughts.

If a vertex attribute is enabled as an array, a buffer is bound to that attribute, and the attribute is consumed by the current program, then calls to drawArrays and drawElements will verify that each referenced vertex lies within the storage of the bound buffer. If the range specified in drawArrays or any referenced index in drawElements lies outside the storage of the bound buffer, an INVALID_OPERATION error is generated and no geometry is drawn.

pjcozzi commented 11 years ago

FYI. This doesn't fail for me on GeForce 260M:

Scene/EllipsoidPrimitive renders with the default material

mramato commented 11 years ago

I deleted my last comment, turns out my home machine was a revision behind, I'll update when I test again.

pjcozzi commented 11 years ago

Same deal on my AMD card - just the three polyline tests fail.

mramato commented 11 years ago

Scene/EllipsoidPrimitive renders with the default material (and EllipsoidPrimitive specs in igenereal) definitely have some issues which lead to sporadic failures when running all tests. I'm looking at it now and it looks like the tests themselves are bad. I'll fix this separately. The Polyline bugs are a separate issue that someone needs to look at.

mramato commented 11 years ago

It looks like the random test failures I'm seeing under Firefox on both my work and home machines are all related to context.clear(). All of our tests expect it to clear to black, but for some reason on Firefox it occasionally clears to a different color. I see we're using the default GL clear color; could something else we're doing be affecting that?

pjcozzi commented 11 years ago
mramato commented 11 years ago

I just did a fresh set of runs (and a reboot just in case) to try and confirm what the behavior is.

  1. Yes, it fails right out of the gate, every time, and continues to fail if I run multiple times with the same instance of Firefox. I get the same failure, Expected { 0 : 255, 1 : 255, 2 : 255, 3 : 255 } to equal [ 0, 0, 0, 0 ]., after the below two lines.
context.clear();
expect(context.readPixels()).toEqual([0, 0, 0, 0]);
  1. It never fails when I run just that one spec or just the EllipsoidSpec, It only fails when I run the whole test suite.
pjcozzi commented 11 years ago

Is the zoom at 100%? We ran into an issue with Chrome - I started on a fix, err, better error message.

mramato commented 11 years ago

Yes, zoom is 100%

mramato commented 11 years ago

Perhaps @kring and @shunter can try to reproduce, I think they have 8800GT cards as well.

shunter commented 11 years ago

I see the same behavior as Matt. "EllipsoidPrimitive renders with the default material." fails when running all tests, passes by itself and passes when running only EllipsoidPrimitiveSpec.

shunter commented 11 years ago

Does this have anything to do with @pjcozzi's recent change that changed _clearColorCommand in Scene from Color.BLACK to new Color() (which is white)?

I'm not totally sure, I checked out the revision before that merge came in and still saw a failure, though that was a different actual:

Expected { 0 : 0, 1 : 0, 2 : 255, 3 : 255 } to equal [ 0, 0, 0, 0 ].

mramato commented 11 years ago

Thanks @shunter. I'm glad to ehar I'm not crazy :smirk:

pjcozzi commented 11 years ago

@shunter no - that code is in Scene. This test uses the renderer directly. I wonder if whatever spec is ran before is causing the problem. I'll see if I can reproduce on my NVIDIA laptop at home.

mramato commented 11 years ago

I already tried removing previous specs to see if that was the problem, but couldn't stop it from failing that way. I'm sure something we're doing before hand is causing the problem, but I don't think it's necessarily the previous spec. Maybe you'll have better luck than me though.

pjcozzi commented 11 years ago

@bagnell can you look at this for b13? It is two separate problems - the polyline and ellipsoid (well, whatever left-behind state that makes the ellipsoid test fail). Previously, Scene/EllipsoidPrimitive did not fail on my GeForce 260M, but it does now.

pjcozzi commented 11 years ago

@bagnell any update on these? If these also happen in b12, shipping them in b13 is err, OK, but not exactly what we are going for.

bagnell commented 11 years ago

@pjcozzi I finally figured out what the problem is. In Firefox 18, the drawElements function computes the number of indices from the count and offset arguments in bytes and checks it against the size of the index buffer in bytes. The problem is that it casts the size of the index buffer in bytes to the data type of the indices.

For example, the Scene/PolylineCollection renders more than 64K vertexes of same polyline. test creates a Polyline with 2 * 2 ^16 + 2 positions. One index buffer that has a byte size of (unsigned int) 2 * 2^16 - 2 is tested to be less than (unsigned short) (2 * 2^16 + 1) = 1 and fails with an invalid operation error.

This is fixed in later versions of Firefox. Do we want to fix this?

pjcozzi commented 11 years ago

I confirm that everything passes on Aurora. Are there still issues with:

Scene/EllipsoidPrimitive renders with the default material.

Given that this doesn't happen on all machines - including mine - and it doesn't affect the examples, I'm OK with shipping b13, but I'll make a separate issue for it.