Glup

[zmike]

@pcwalton @larsbergstrom

[pcwalton]

Yeah, only in debug builds please. glGetError() can force a FIFO flush.

[metajack]

Skia only does this when specifically enabled. Ie, debug builds don't even do it. Because it makes it unusably slow, so you only turn it on when you suspect some GL API issue. Maybe this should be a Cargo feature?

[metajack]

@zmike Are you going to address the performance concerns?

[zmike]

Sorry, this got away from me. What exactly are we looking to do here, make it a cargo-based enable?

[metajack]

I think we want it to be opt in through cargo features if that is possible so that people can turn this on at compile time and no one else has to suffer the performance penalty.

[zmike]

I think this is what you meant; I added a "gldebug" cargo feature which gates the error checking.

[metajack]

@bors-servo r+

Awesome! Thanks.

---

Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io

[zmike]

No prob, sorry for the (extended) delay.

[metajack]

@bors-servo r+

Homu was desynced so doing this again.

[bors-servo]

📌 Commit 2dff76f has been approved by metajack

[bors-servo]

⌛ Testing commit 2dff76f with merge 0dd3ff0...

[bors-servo]

💔 Test failed - travis

[metajack]

Looks like duplicate depedency is getting pulled in.

[zmike]

I'm not entirely sure what this means.

[larsbergstrom]

Oh, it means you're getting broken by some dependencies that have a dependency libc = "*" but are trying to share types with ones that have libc = "0.1", as the former are getting libc 0.2, whose types are not compatible.

@mbrubeck How have you been fixing these? Are we switching everything to explicitly ask for 0.2 or 0.1?

[mbrubeck]

The graphics crates like glutin are trying to move things from * to 0.1 first. Also in cases where not all dependencies are updated at once, you can fix some type mismatches like this: rust-windowing/glutin#651

[larsbergstrom]

@bors-servo r+

[bors-servo]

📌 Commit bd84b41 has been approved by larsbergstrom

[bors-servo]

⌛ Testing commit bd84b41 with merge 600677b...

[bors-servo]

💔 Test failed - travis

[larsbergstrom]

Yup, looks like we need skiaup first

[mrobinson]

@bors-servo retry

After servo/io-surface-rs#42 this should build properly or at least fail at a later point.

[bors-servo]

⌛ Testing commit bd84b41 with merge 85fd404...

[bors-servo]

💔 Test failed - travis

[jdm]

Now failing in skia proper.

[mrobinson]

@bors-servo retry

Trying one more time now that @mbrubeck has updated cgl-rs.

[bors-servo]

⌛ Testing commit bd84b41 with merge fc5a1ee...

[bors-servo]

💔 Test failed - travis

[jdm]

Now failing in rust-layers proper!

[mrobinson]

Looks like the issue might be in https://github.com/Daggerbot/x11-rs this time, which is unfortunately not part of the servo family of repositories.

[jdm]

Filed AltF02/x11-rs#30 and AltF02/x11-rs#31.

[bors-servo]

🔒 Merge conflict

[KiChjang]

@bors-servo Talk about a delayed response...

[bors-servo]

🔒 Merge conflict

[jdm]

@bors-servo r-