Changes for Rust 2018. Removed distinction between {Combined,Separated}Contexts. Style changes.

[goddessfreya]

Fixes #1102
Reverts: #1107, closing #1109
Hopefully rust-mobile/android-rs-glue#203 gets merged.
Signed-off-by: Hal Gentz [email protected]

[goddessfreya]

Waiting: rust-mobile/android-rs-glue#202

[goddessfreya]

Also, while I'm waiting, is there anything objectionable with the current separated context api, @Osspial?

[Osspial]

There are some bikesheddy/documentation things, but for the most part the API looks good. The biggest question I have is, if you use GlContext::new_separated to create multiple contexts for the same window, how does that work? I don't know if most platforms specifically disallow it, but I don't really see the utility of it.

Also, creating a context for a window, deleting said context, then adding a new context with a different underlying pixel format may cause some problems on Windows. I'm not familiar enough with the backend to know if it runs into that problem, but it doesn't seem outside the realm of possibility.

The other stuff I have is relatively nitpicky, though. I'll go ahead and add that in with a review.

[Osspial]

The WindowRef type currently isn't publicly exposed on the crate level, so it's impossible for a user to view documentation for it or match against it.

[Osspial]

SeparatedContext is no longer the correct type.

[Osspial]

I don't like how GlContext and Context are named. Having that distinction implies that Context is a general, non-OpenGL-specific context and that GlContext is the version of that for OpenGL, but that isn't what the types actually are. Instead, GlContext is an OpenGL Context that's associated with a window and Context is an OpenGL context that might be associated with a window. The naming should reflect that.

[goddessfreya]

This is a very good point, altho I can't think of any good names. I've changed it toWindowedContext?

[Osspial]

See issues I mentioned in the main comment. I'm not entirely sure what this function exists for, and its utility compared to just GlContext::new isn't properly explained in the documentation.

[goddessfreya]

That's actually a complete oversight, the scenario where there has already been a context made on the window, oops. The idea was that they'd pass in a window that's already made, but that's lacking a context, and we'd make it on it.

The assumption was that someday in the near future winit will support making winit's Windows via an unsafe os-specific from_raw function, and that users could then assemble a winit Window using parts they received from another (possibly non-rust) library and we would make the context for them.

I'll have to investigate further into this before the 0.20 release.

[goddessfreya]

So here is what I've gathered:
On X11 + egl: afaik the only thing we got to watch out for is that the surface is actually destroyed, and not still "in use".
On X11 + glx: I couldn't find much, but I assume the same thing applies as above.
On X11 + *: The window's pixel format comes pre-chosen, so it doesn't matter.
On macos, emscripten, android and ios: We don't support new_separated.
For Wayland, maybe @vberger can clue me in, but afaik, only the egl thing mentioned above possibly applies.

With that said, w/ X11/Wayland + egl + mesa + radeonsi, mesa issues these harmless warnings when anybody calls eglInitialize(...); eglTerminate(...); eglInitialize(...) in the same address space:

mesa: for the -simplifycfg-sink-common option: may only occur zero or one times!
mesa: for the -global-isel-abort option: may only occur zero or one times!

This is a mesa bug, and I'm looking into sending a possible solution upstream to the llvm devs.

[Osspial]

Hmm. I feel iffy about exposing new_separated in the cross-platform API space when it's not supported on MacOS. Perhaps we should hold off on exposing that function until we figure out what's happening with a from_raw function? If the main purpose is to support creating contexts for externally-managed windows, there doesn't seem to be much of a use for it now while externally-managed windows can't really exist.

We could also add platform-specific context creation functions for external windows via the os module (i.e. new_with_hwnd(hwnd: *mut c_void) on Windows) so that contexts could be created without going through Winit's Window at all.

[goddessfreya]

Platform-specific context creation functions sounds like a good idea, I'll probably switch to that.