Glutin update

[paulrouget]

[tomaka]

Does it work for you before the glutin update?

OS/X only supports shader versions 100 or 140+. If you want version 100 you need to add precision qualifiers to all the variables, like mediump. If you want version 140 then the attribute and varying keywords no longer exist.

If you omit the #version declaration I think it implicitely means version 110. The shaders seem to be in GLSL 1.1, so that would be coherent. But version 110 is not supported by OS/X.

So I don't see how rust-layers even works on OS/X in its current state except if it's relying on non-standard behavior.

[tomaka]

Oh, wait, duh.

I said that OS/X didn't support version 110, but that's only true if you use a recent version of OpenGL.

There's also the possibility to create a legacy context (opengl 3.1 or less) at initialization, in which case it is supported.

This is what servo is doing:

• https://github.com/servo/servo/blob/44c4bb00c1cb8645ee2fc303848a5136108e594f/ports/glutin/window.rs#L141-L144
• https://github.com/servo/servo/blob/44c4bb00c1cb8645ee2fc303848a5136108e594f/ports/glutin/window.rs#L96

That's the answer to your problem, just use with_gl(GlRequest::Specific(Api::OpenGl, (3, 0))) at initialization.

[metajack]

@mrobinson @glennw What should we be doing here? Should we upgrade the context or keep using the legacy one?

[paulrouget]

@mrobinson @glennw What should we be doing here? Should we upgrade the context or keep using the legacy one?

For the record, using NSOpenGLProfileVersionLegacy works.

If that's ok, we can use the legacy context and once the update has landed, we upgrade?

[metajack]

I think it's fine to keep doing what we were doing and upgrade later if that is the correct thing long term.

[paulrouget]

Note: we hit a Rust bug in the match code that selects the right OpenGL profile. Not using ranges appears to solve the issue.

[paulrouget]

See rust-windowing#567 for profile selection fix.

[paulrouget]

@glennw / @tomaka can you please tell me if both of these patches are needed, and where they should go (servo/glutin or tomaka/glutin):

• rust-windowing@5124bda ("This was previously in glutin, but was removed since Weak is not marked stable here: 0f7bd90")
• glennw@7ee0318

And does one require the other?

7ee0318 is a bit tricky because I don't know what to do in wakeup_event_loop where things changed quite a bit (display.display and x11.display).

[metajack]

I believe Glenn took out Weak support so rust-windowing@5124bda is no longer needed for current Servo.

glennw@7ee0318 is needed though. But seems like this could go upstream. @tomaka ?

[tomaka]

Sure. As long as it's not unstable I'm ok with it.

[paulrouget]

Upstream PRs:

• Fix OpenGL profile selection rust-windowing/glutin#567
• Fix a rare crash in some X11 implementations rust-windowing/glutin#568

[paulrouget]

All required PRs have now been merged upstreams.
Can someone review this?

[paulrouget]

Once merged, I'll update servo's Cargo.lock in servo/servo#7096

[paulrouget]

Review status: 0 of 83 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{src/api/x11/window.rs, line 122 [r1] (raw file):}
Addressed. Change made it upstream.

---

Comments from the review on Reviewable.io

[glennw]

@paulrouget This is quite difficult to review as is. Perhaps we could rename the existing servo branch to servo_ and create a new servo branch from glutin/master then just apply our local patches on top? (We've done this in the past a few times). It would be a lot easier to see the differences and give us a cleaner history.

[paulrouget]

This PR is against the master branch, not the servo branch. Creating a new PR: #44