Skip to content

Commit

Permalink
Merge #3470
Browse files Browse the repository at this point in the history
3470: [gl] replace Surfman by EGL r=grovesNL a=kvark

Fixes #3468

This is a BIG change. It re-architects WSI of the GL backend to unconditionally use EGL (via `khronos-egl` crate atm, but we can change that). This means the backend is *only* supported on Linux/BSD/Android, it's no longer supported on Apple and Microsoft platforms.

One of the consequences - we throw Surfman away. There was too much complication and too little benefit from it anyway.

Another consequence - we are locked to GLES, given that I experienced issues trying to get a desktop GL context via EGL. We can revisit this, but really I think going with GLES 3.1 makes total sense for the backend, unconditionally. if the target platform is powerful above what GLES 3.1 offers, it should just support Vulkan.

Most importantly, it solves GL context sharing, so presentation is much more robust. However, it doesn't solve the extra copy - #3469. For this, we'll need to follow-up by directly using platform extensions, such as [EGL_WL_create_wayland_buffer_from_image](https://www.khronos.org/registry/EGL/extensions/WL/EGL_WL_create_wayland_buffer_from_image.txt).

PR checklist:
- [x] `make` succeeds (on *nix)
- [x] `make reftests` succeeds
- [x] tested examples with the following backends: Linux


Co-authored-by: Dzmitry Malyshau <kvark@fastmail.com>
  • Loading branch information
bors[bot] and kvark committed Nov 9, 2020
2 parents fa32352 + d8fe67c commit cc82d34
Show file tree
Hide file tree
Showing 18 changed files with 365 additions and 641 deletions.
2 changes: 1 addition & 1 deletion examples/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ path = "../src/backend/vulkan"
version = "0.6"
optional = true

[target.'cfg(unix)'.dependencies.gfx-backend-gl]
[target.'cfg(all(unix, not(any(target_os = "ios", target_os = "macos"))))'.dependencies.gfx-backend-gl]
path = "../src/backend/gl"
version = "0.6"
features = ["x11"]
Expand Down
9 changes: 7 additions & 2 deletions examples/compute/main.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
#![cfg_attr(
not(any(
feature = "vulkan",
feature = "gl",
feature = "dx11",
feature = "dx12",
feature = "metal"
feature = "metal",
)),
allow(dead_code, unused_extern_crates, unused_imports)
)]
Expand All @@ -12,6 +13,8 @@
extern crate gfx_backend_dx11 as back;
#[cfg(feature = "dx12")]
extern crate gfx_backend_dx12 as back;
#[cfg(feature = "gl")]
extern crate gfx_backend_gl as back;
#[cfg(feature = "metal")]
extern crate gfx_backend_metal as back;
#[cfg(feature = "vulkan")]
Expand All @@ -23,9 +26,10 @@ use hal::{adapter::MemoryType, buffer, command, memory, pool, prelude::*, pso};

#[cfg(any(
feature = "vulkan",
feature = "gl",
feature = "dx11",
feature = "dx12",
feature = "metal"
feature = "metal",
))]
fn main() {
env_logger::init();
Expand Down Expand Up @@ -288,6 +292,7 @@ unsafe fn create_buffer<B: hal::Backend>(

#[cfg(not(any(
feature = "vulkan",
feature = "gl",
feature = "dx11",
feature = "dx12",
feature = "metal"
Expand Down
2 changes: 1 addition & 1 deletion examples/mesh-shading/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
extern crate gfx_backend_dx11 as back;
#[cfg(feature = "dx12")]
extern crate gfx_backend_dx12 as back;
#[cfg(all(unix, feature = "gl"))]
#[cfg(feature = "gl")]
extern crate gfx_backend_gl as back;
#[cfg(feature = "metal")]
extern crate gfx_backend_metal as back;
Expand Down
2 changes: 1 addition & 1 deletion examples/quad/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ extern crate gfx_backend_dx12 as back;
feature = "gl",
)))]
extern crate gfx_backend_empty as back;
#[cfg(all(unix, feature = "gl"))]
#[cfg(feature = "gl")]
extern crate gfx_backend_gl as back;
#[cfg(feature = "metal")]
extern crate gfx_backend_metal as back;
Expand Down
12 changes: 4 additions & 8 deletions src/backend/gl/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,12 @@ readme = "README.md"
documentation = "https://docs.rs/gfx-backend-gl"
workspace = "../../.."
edition = "2018"
build = "build.rs"

[lib]
name = "gfx_backend_gl"

[features]
default = ["surfman"]
x11 = ["surfman/sm-x11"]
default = []

[dependencies]
arrayvec = "0.5"
Expand All @@ -32,9 +30,10 @@ parking_lot = "0.11"
spirv_cross = { version = "0.22", features = ["glsl"] }
lazy_static = "1"
raw-window-handle = "0.3"
x11 = { version = "2", features = ["xlib"], optional = true }

[target.'cfg(all(unix, not(target_os = "ios")))'.dependencies]
surfman = { version = "0.3", features = ["sm-raw-window-handle"], optional = true }
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
egl = { package = "khronos-egl", version = "2.1.0" }

[target.'cfg(target_arch = "wasm32")'.dependencies]
js-sys = "0.3.6"
Expand All @@ -59,6 +58,3 @@ features = [
"WebGlTexture",
"Window",
]

[build-dependencies]
cfg_aliases = "0.1.0"
2 changes: 2 additions & 0 deletions src/backend/gl/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

[OpenGL](https://www.khronos.org/opengl/) backend for gfx.

Can only be used on non-Apple Unix systems. The WSI is hard-coded to EGL.

## Normalized Coordinates

Render | Depth | Texture
Expand Down
16 changes: 0 additions & 16 deletions src/backend/gl/build.rs

This file was deleted.

2 changes: 1 addition & 1 deletion src/backend/gl/src/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1752,7 +1752,7 @@ impl d::Device<B> for Device {
}
}

#[cfg(wasm)]
#[cfg(target_arch = "wasm32")]
unsafe fn wait_for_fences<I>(
&self,
fences: I,
Expand Down
21 changes: 10 additions & 11 deletions src/backend/gl/src/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,22 +254,23 @@ pub enum Requirement<'a> {
Ext(&'a str),
}

const IS_WEBGL: bool = cfg!(target_arch = "wasm32");

impl Info {
fn get(gl: &GlContainer) -> Info {
let platform_name = PlatformName::get(gl);
let version = Version::parse(get_string(gl, glow::VERSION).unwrap_or_default()).unwrap();
#[cfg(not(wasm))]
let shading_language =
let shading_language = if IS_WEBGL {
Version::new_embedded(3, 0, String::from(""))
} else {
Version::parse(get_string(gl, glow::SHADING_LANGUAGE_VERSION).unwrap_or_default())
.unwrap();
#[cfg(wasm)]
let shading_language = Version::new_embedded(3, 0, String::from(""));
.unwrap()
};
// TODO: Use separate path for WebGL extensions in `glow` somehow
// Perhaps automatic fallback for NUM_EXTENSIONS to EXTENSIONS on native
#[cfg(wasm)]
let extensions = HashSet::new();
#[cfg(not(wasm))]
let extensions = if version >= Version::new(3, 0, None, String::from("")) {
let extensions = if IS_WEBGL {
HashSet::new()
} else if version >= Version::new(3, 0, None, String::from("")) {
let num_exts = get_usize(gl, glow::NUM_EXTENSIONS).unwrap();
(0..num_exts)
.map(|i| unsafe { gl.get_parameter_indexed_string(glow::EXTENSIONS, i as u32) })
Expand Down Expand Up @@ -327,8 +328,6 @@ impl Info {
}
}

const IS_WEBGL: bool = cfg!(wasm);

/// Load the information pertaining to the driver and the corresponding device
/// capabilities.
pub(crate) fn query_all(
Expand Down
99 changes: 13 additions & 86 deletions src/backend/gl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ extern crate bitflags;
#[macro_use]
extern crate log;

#[cfg(surfman)]
use parking_lot::RwLock;

use std::{
cell::Cell,
fmt,
Expand All @@ -35,19 +32,11 @@ mod state;
mod window;

// Web implementation
#[cfg(wasm)]
pub use window::web::Surface;

// Surfman implementation
#[cfg(surfman)]
pub use crate::window::surfman::{Instance, Surface, Swapchain};
// Helps windows detect discrete GPUs
#[cfg(surfman)]
surfman::declare_surfman!();
#[cfg(target_arch = "wasm32")]
pub use window::web::{Surface, Swapchain};

// Catch-all dummy implementation
#[cfg(dummy)]
pub use window::dummy::{Instance, Surface, Swapchain};
#[cfg(not(target_arch = "wasm32"))]
pub use crate::window::egl::{Instance, Surface, Swapchain};

pub use glow::Context as GlContext;
use glow::HasContext;
Expand All @@ -56,51 +45,19 @@ type ColorSlot = u8;

pub(crate) struct GlContainer {
context: GlContext,

#[cfg(surfman)]
surfman_device: Starc<RwLock<surfman::Device>>,

#[cfg(surfman)]
surfman_context: Starc<RwLock<surfman::Context>>,
}

impl GlContainer {
#[cfg(not(wasm))]
fn make_current(&self) {
#[cfg(surfman)]
self.surfman_device
.write()
.make_context_current(&self.surfman_context.read())
.expect("TODO");
}

#[cfg(any(glutin, wgl))]
fn from_fn_proc<F>(fn_proc: F) -> GlContainer
#[cfg(not(target_arch = "wasm32"))]
unsafe fn from_fn_proc<F>(fn_proc: F) -> GlContainer
where
F: FnMut(&str) -> *const std::os::raw::c_void,
{
let context = glow::Context::from_loader_function(fn_proc);
GlContainer { context }
}

#[cfg(surfman)]
fn from_fn_proc<F>(
fn_proc: F,
surfman_device: Starc<RwLock<surfman::Device>>,
surfman_context: Starc<RwLock<surfman::Context>>,
) -> GlContainer
where
F: FnMut(&str) -> *const std::os::raw::c_void,
{
let context = unsafe { glow::Context::from_loader_function(fn_proc) };
GlContainer {
context,
surfman_device,
surfman_context,
}
}

#[cfg(wasm)]
#[cfg(target_arch = "wasm32")]
fn from_canvas(canvas: &web_sys::HtmlCanvasElement) -> GlContainer {
let context = {
use wasm_bindgen::JsCast;
Expand Down Expand Up @@ -136,31 +93,18 @@ impl GlContainer {
impl Deref for GlContainer {
type Target = GlContext;
fn deref(&self) -> &GlContext {
#[cfg(not(wasm))]
self.make_current();
&self.context
}
}

#[cfg(surfman)]
impl Drop for GlContainer {
fn drop(&mut self) {
// Contexts must be manually destroyed to prevent a panic
self.surfman_device
.read()
.destroy_context(&mut self.surfman_context.write())
.expect("TODO");
}
}

#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq)]
pub enum Backend {}

impl hal::Backend for Backend {
#[cfg(not(wasm))]
#[cfg(not(target_arch = "wasm32"))]
type Instance = Instance;

#[cfg(wasm)]
#[cfg(target_arch = "wasm32")]
type Instance = Surface;

type PhysicalDevice = PhysicalDevice;
Expand Down Expand Up @@ -223,7 +167,6 @@ impl Error {
}
}

#[cfg(not(wasm))]
fn debug_message_callback(source: u32, gltype: u32, id: u32, severity: u32, message: &str) {
let source_str = match source {
glow::DEBUG_SOURCE_API => "API",
Expand Down Expand Up @@ -280,14 +223,6 @@ enum MemoryUsage {
/// Internal struct of shared data between the physical and logical device.
struct Share {
context: GlContainer,

/// Context associated with an instance.
///
/// Parenting context for all device contexts shared with it.
/// Used for querying basic information and spawning shared contexts.
#[allow(unused)]
instance_context: DeviceContext,

info: Info,
supported_features: hal::Features,
legacy_features: info::LegacyFeatures,
Expand Down Expand Up @@ -438,12 +373,8 @@ unsafe impl<T: ?Sized> Sync for Wstarc<T> {}
#[derive(Debug)]
pub struct PhysicalDevice(Starc<Share>);

#[cfg(not(wgl))]
type DeviceContext = ();

impl PhysicalDevice {
#[allow(unused)]
fn new_adapter(instance_context: DeviceContext, gl: GlContainer) -> adapter::Adapter<Backend> {
fn new_adapter(gl: GlContainer) -> adapter::Adapter<Backend> {
// query information
let (info, supported_features, legacy_features, hints, limits, private_caps) =
info::query_all(&gl);
Expand Down Expand Up @@ -520,7 +451,6 @@ impl PhysicalDevice {
// create the shared context
let share = Share {
context: gl,
instance_context,
info,
supported_features,
legacy_features,
Expand Down Expand Up @@ -634,12 +564,9 @@ impl adapter::PhysicalDevice<Backend> for PhysicalDevice {
// initialize permanent states
let gl = &self.0.context;

#[cfg(not(wasm))]
{
if cfg!(debug_assertions) && gl.supports_debug() {
gl.enable(glow::DEBUG_OUTPUT);
gl.debug_message_callback(debug_message_callback);
}
if cfg!(debug_assertions) && !cfg!(target_arch = "wasm32") && gl.supports_debug() {
gl.enable(glow::DEBUG_OUTPUT);
gl.debug_message_callback(debug_message_callback);
}

if self
Expand Down

0 comments on commit cc82d34

Please sign in to comment.