Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use stackgl/headless-gl #1447

Closed
ansis opened this issue Aug 31, 2015 · 21 comments
Closed

use stackgl/headless-gl #1447

ansis opened this issue Aug 31, 2015 · 21 comments
Assignees
Milestone

Comments

@ansis
Copy link
Contributor

ansis commented Aug 31, 2015

Switch to https://github.com/stackgl/headless-gl for render tests

@mourner
Copy link
Member

mourner commented Aug 31, 2015

Yes, was going to switch but let's wait until node-pre-gyp binaries are there. stackgl/headless-gl#19

@jfirebaugh
Copy link
Contributor

We don't have node-pre-gyp binaries currently, and this probably unblocks #1197, so I think we should do it ASAP.

@mourner
Copy link
Member

mourner commented Aug 31, 2015

We don't have binaries but it's compiled much faster. ANGLE takes a while to compile, but if it's fine, let's switch. Not sure if it fixes the layer-by-layer stuff though, I thought someone said it's probably not?

@ansis
Copy link
Contributor Author

ansis commented Sep 1, 2015

I started working on this today: 1f62992

A lot of render tests are failing.

I spent most of the day trying to figure out #1449 but didn't have any luck.

@ansis
Copy link
Contributor Author

ansis commented Sep 1, 2015

It looks like a lot of the other failing render tests could have the same cause as #1449

I extracted a test case and opened stackgl/headless-gl#21

@mikolalysenko
Copy link

Fixed that weird attribute bug, next step is getting prebuild modules up.

@mourner
Copy link
Member

mourner commented Sep 7, 2015

Finally works for me! Pushed a branch with a working render suite to https://github.com/mapbox/mapbox-gl-js/tree/stacklgl-headless-gl-2.

91 failed, 123 passed assertions.

Pretty good! Some noticeable outstanding issues:

  1. One of the line-join tests breaks the whole suite with "Abort trap 6" error, so had to disable them for now.
  2. Some weirdness with line-caps as well: image
  3. All the text is rendered as black rectangles:
    image
  4. All dashes don't render at all.

@jfirebaugh jfirebaugh added this to the 0.12 milestone Sep 10, 2015
@mikolalysenko
Copy link

Ok, I ran these test cases locally and everything passes on the nan-v2 branch of headless-gl. Unfortunately, I can't publish this yet since I am currently blocked by the following issue in nodejs/nan:

nodejs/nan#459

@jfirebaugh
Copy link
Contributor

Current status: tests and test-suite pass locally (albeit with some missing fill antialiasing) but fail in CI trying to compile the shaders:

/home/ubuntu/mapbox-gl-js/js/render/gl_util.js:31
            throw new Error(this.getShaderInfoLog(shader));
                  ^
Error: 0:2(1): error: syntax error, unexpected NEW_IDENTIFIER

    at WebGLRenderingContext.context.getShader (/home/ubuntu/mapbox-gl-js/js/render/gl_util.js:31:19)
    at WebGLRenderingContext.context.initializeShader (/home/ubuntu/mapbox-gl-js/js/render/gl_util.js:39:28)
    at Painter.setup (/home/ubuntu/mapbox-gl-js/js/render/painter.js:53:27)
    at new Painter (/home/ubuntu/mapbox-gl-js/js/render/painter.js:23:10)
    at util.extend._setupPainter (/home/ubuntu/mapbox-gl-js/js/ui/map.js:672:24)
    at new module.exports (/home/ubuntu/mapbox-gl-js/js/ui/map.js:91:10)
    at createMap (/home/ubuntu/mapbox-gl-js/test/js/ui/map.test.js:11:16)
    at Test.<anonymous> (/home/ubuntu/mapbox-gl-js/test/js/ui/map.test.js:25:19)
    at Test.bound [as _cb] (/home/ubuntu/mapbox-gl-js/node_modules/prova/node_modules/tape/lib/test.js:62:32)
    at Test.run (/home/ubuntu/mapbox-gl-js/node_modules/prova/node_modules/tape/lib/test.js:75:10)

@mikolalysenko
Copy link

@jfirebaugh That is super weird, it might be an ANGLE bug. I'll ping some of the devs on IRC and see if they know what is going on.

@mikolalysenko
Copy link

Also, can you try pushing an instrumented version of the test case to CI so that it logs the contents of the shader program? Having this information would help solve this problem.

@jfirebaugh
Copy link
Contributor

https://circleci.com/gh/mapbox/mapbox-gl-js/1022

It's failing on the first shader to be compiled, the debug fragment shader. The source being compiled is:

precision mediump float;uniform vec4 u_color;void main(){gl_FragColor=u_color;}

@jfirebaugh
Copy link
Contributor

stackgl/headless-gl#26

@jfirebaugh
Copy link
Contributor

Status update: work here is happening in the stacklgl-headless-gl-2 branch. The last version of headless-gl that I tried was 2.1.5. Instead of the "error: syntax error, unexpected NEW_IDENTIFIER", there's an earlier error, when attempting to create the context:

/home/ubuntu/mapbox-gl-js/node_modules/gl/index.js:36
  var gl = new webgl.WebGLRenderingContext(
           ^
Error: Error creating WebGLContext
    at createContext (/home/ubuntu/mapbox-gl-js/node_modules/gl/index.js:36:12)
    at new Canvas (/home/ubuntu/mapbox-gl-js/js/util/canvas.js:20:20)
    at util.extend._setupContainer (/home/ubuntu/mapbox-gl-js/js/ui/map.js:658:24)
    at new module.exports (/home/ubuntu/mapbox-gl-js/js/ui/map.js:90:10)
    at /home/ubuntu/mapbox-gl-js/test/render.test.js:16:15
    at /home/ubuntu/mapbox-gl-js/node_modules/mapbox-gl-test-suite/lib/render.js:60:9
    at runOne (/home/ubuntu/mapbox-gl-js/node_modules/mapbox-gl-test-suite/lib/harness.js:64:13)
    at pop (/home/ubuntu/mapbox-gl-js/node_modules/mapbox-gl-test-suite/node_modules/queue-async/queue.js:24:14)
    at Server.<anonymous> (/home/ubuntu/mapbox-gl-js/node_modules/mapbox-gl-test-suite/node_modules/queue-async/queue.js:38:39)
    at Server.g (events.js:180:16)

https://circleci.com/gh/mapbox/mapbox-gl-js/1230

Digging in here will likely require either reproducing the error in a VM and debugging into native code to obtain more information about the failure, or instrumentation / better error reporting to headless-gl and rerunning on CircleCI.

@mikolalysenko
Copy link

I wasn't able to get this to reproduce in circleCI when I tried it. One thing to check is that if you are destroying contexts when you are done with them, since relying on the GC to pick them up could cause problems.

@jfirebaugh
Copy link
Contributor

The error occurs on the first attempt to create a context, so leaking contexts is probably not the issue.

I tried not updating the mesa version and the build got a lot further:

https://circleci.com/gh/mapbox/mapbox-gl-js/1348

@jfirebaugh
Copy link
Contributor

Output from that last run looks really good. A lot of the discrepancies we saw between CI and OS X rendering are gone. There's just one failure, a tiny difference on circle-blur/literal:

image

@jfirebaugh
Copy link
Contributor

@lucaswoj
Copy link
Contributor

This is huge! 🎉

@mourner
Copy link
Member

mourner commented Nov 12, 2015

Amazing, finally!!! Was stackgl/headless-gl#26 resolved or is it just not reproducible without mesa version upgrade?

@mikolalysenko
Copy link

I think the issue was related to the mesa version. I couldn't get it to reproduce on my CircleCI instance, so the downgrade must've fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants