Skip to content

Commit

Permalink
Improve glow error reporting (#1403)
Browse files Browse the repository at this point in the history
* Improve glow error reporting
* Add more check_for_gl_error calls
* Remove clippy lint list from egui_glow lib.rs
  - Forgotten in #1394
* egui_glow: move vao code to own file
* Cleanup: `use glow::HasContext as _;`

Co-authored-by: Zachary Kohnen <me@dusterthefirst.com>
  • Loading branch information
emilk and DusterTheFirst authored Mar 22, 2022
1 parent 41b178b commit 6f10e2e
Show file tree
Hide file tree
Showing 7 changed files with 288 additions and 278 deletions.
140 changes: 60 additions & 80 deletions egui_glow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,85 +5,6 @@
//! This library is an [`epi`] backend.
//! If you are writing an app, you may want to look at [`eframe`](https://docs.rs/eframe) instead.
// Forbid warnings in release builds:
#![cfg_attr(not(debug_assertions), deny(warnings))]
#![deny(unsafe_code)]
#![warn(
clippy::all,
clippy::await_holding_lock,
clippy::char_lit_as_u8,
clippy::checked_conversions,
clippy::dbg_macro,
clippy::debug_assert_with_mut_call,
clippy::disallowed_method,
clippy::doc_markdown,
clippy::empty_enum,
clippy::enum_glob_use,
clippy::exit,
clippy::expl_impl_clone_on_copy,
clippy::explicit_deref_methods,
clippy::explicit_into_iter_loop,
clippy::fallible_impl_from,
clippy::filter_map_next,
clippy::flat_map_option,
clippy::float_cmp_const,
clippy::fn_params_excessive_bools,
clippy::from_iter_instead_of_collect,
clippy::if_let_mutex,
clippy::implicit_clone,
clippy::imprecise_flops,
clippy::inefficient_to_string,
clippy::invalid_upcast_comparisons,
clippy::large_digit_groups,
clippy::large_stack_arrays,
clippy::large_types_passed_by_value,
clippy::let_unit_value,
clippy::linkedlist,
clippy::lossy_float_literal,
clippy::macro_use_imports,
clippy::manual_ok_or,
clippy::map_err_ignore,
clippy::map_flatten,
clippy::map_unwrap_or,
clippy::match_on_vec_items,
clippy::match_same_arms,
clippy::match_wild_err_arm,
clippy::match_wildcard_for_single_variants,
clippy::mem_forget,
clippy::mismatched_target_os,
clippy::missing_errors_doc,
clippy::missing_safety_doc,
clippy::mut_mut,
clippy::mutex_integer,
clippy::needless_borrow,
clippy::needless_continue,
clippy::needless_for_each,
clippy::needless_pass_by_value,
clippy::option_option,
clippy::path_buf_push_overwrite,
clippy::ptr_as_ptr,
clippy::ref_option_ref,
clippy::rest_pat_in_fully_bound_structs,
clippy::same_functions_in_if_condition,
clippy::semicolon_if_nothing_returned,
clippy::single_match_else,
clippy::string_add_assign,
clippy::string_add,
clippy::string_lit_as_bytes,
clippy::string_to_string,
clippy::todo,
clippy::trait_duplication_in_bounds,
clippy::unimplemented,
clippy::unnested_or_patterns,
clippy::unused_self,
clippy::useless_transmute,
clippy::verbose_file_reads,
clippy::zero_sized_map_values,
future_incompatible,
nonstandard_style,
rust_2018_idioms,
rustdoc::missing_crate_level_docs
)]
#![allow(clippy::float_cmp)]
#![allow(clippy::manual_range_contains)]

Expand All @@ -93,7 +14,7 @@ pub use painter::Painter;
mod misc_util;
mod post_process;
mod shader_version;
mod vao_emulate;
mod vao;

#[cfg(all(not(target_arch = "wasm32"), feature = "winit"))]
pub mod winit;
Expand All @@ -105,3 +26,62 @@ mod epi_backend;

#[cfg(all(not(target_arch = "wasm32"), feature = "winit"))]
pub use epi_backend::{run, NativeOptions};

/// Check for OpenGL error and report it using `tracing::error`.
///
/// ``` no_run
/// # let glow_context = todo!();
/// use egui_glow::check_for_gl_error;
/// check_for_gl_error!(glow_context);
/// check_for_gl_error!(glow_context, "during painting");
/// ```
#[macro_export]
macro_rules! check_for_gl_error {
($gl: expr) => {{
$crate::check_for_gl_error_impl($gl, file!(), line!(), "")
}};
($gl: expr, $context: literal) => {{
$crate::check_for_gl_error_impl($gl, file!(), line!(), $context)
}};
}

#[doc(hidden)]
pub fn check_for_gl_error_impl(gl: &glow::Context, file: &str, line: u32, context: &str) {
use glow::HasContext as _;
#[allow(unsafe_code)]
let error_code = unsafe { gl.get_error() };
if error_code != glow::NO_ERROR {
let error_str = match error_code {
glow::INVALID_ENUM => "GL_INVALID_ENUM",
glow::INVALID_VALUE => "GL_INVALID_VALUE",
glow::INVALID_OPERATION => "GL_INVALID_OPERATION",
glow::STACK_OVERFLOW => "GL_STACK_OVERFLOW",
glow::STACK_UNDERFLOW => "GL_STACK_UNDERFLOW",
glow::OUT_OF_MEMORY => "GL_OUT_OF_MEMORY",
glow::INVALID_FRAMEBUFFER_OPERATION => "GL_INVALID_FRAMEBUFFER_OPERATION",
glow::CONTEXT_LOST => "GL_CONTEXT_LOST",
0x8031 => "GL_TABLE_TOO_LARGE1",
0x9242 => "CONTEXT_LOST_WEBGL",
_ => "<unknown>",
};

if context.is_empty() {
tracing::error!(
"GL error, at {}:{}: {} (0x{:X}). Please file a bug at https://github.com/emilk/egui/issues",
file,
line,
error_str,
error_code,
);
} else {
tracing::error!(
"GL error, at {}:{} ({}): {} (0x{:X}). Please file a bug at https://github.com/emilk/egui/issues",
file,
line,
context,
error_str,
error_code,
);
}
}
}
116 changes: 1 addition & 115 deletions egui_glow/src/misc_util.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,6 @@
#![allow(unsafe_code)]
use glow::HasContext;
use std::option::Option::Some;

pub fn check_for_gl_error(gl: &glow::Context, context: &str) {
let error_code = unsafe { gl.get_error() };
if error_code != glow::NO_ERROR {
tracing::error!(
"GL error, at: '{}', code: {} (0x{:X})",
context,
error_code,
error_code
);
}
}
use glow::HasContext as _;

pub(crate) unsafe fn compile_shader(
gl: &glow::Context,
Expand Down Expand Up @@ -50,105 +38,3 @@ pub(crate) unsafe fn link_program<'a, T: IntoIterator<Item = &'a glow::Shader>>(
Err(gl.get_program_info_log(program))
}
}
///Wrapper around Emulated VAO and GL's VAO
pub(crate) enum VAO {
Emulated(crate::vao_emulate::EmulatedVao),
Native(crate::glow::VertexArray),
}

impl VAO {
pub(crate) unsafe fn native(gl: &glow::Context) -> Self {
Self::Native(gl.create_vertex_array().unwrap())
}

pub(crate) unsafe fn emulated() -> Self {
Self::Emulated(crate::vao_emulate::EmulatedVao::new())
}

pub(crate) unsafe fn bind_vertex_array(&self, gl: &glow::Context) {
match self {
VAO::Emulated(vao) => vao.bind_vertex_array(gl),
VAO::Native(vao) => gl.bind_vertex_array(Some(*vao)),
}
}

pub(crate) unsafe fn bind_buffer(&mut self, gl: &glow::Context, buffer: &glow::Buffer) {
match self {
VAO::Emulated(vao) => vao.bind_buffer(buffer),
VAO::Native(_) => gl.bind_buffer(glow::ARRAY_BUFFER, Some(*buffer)),
}
}

pub(crate) unsafe fn add_new_attribute(
&mut self,
gl: &glow::Context,
buffer_info: crate::vao_emulate::BufferInfo,
) {
match self {
VAO::Emulated(vao) => vao.add_new_attribute(buffer_info),
VAO::Native(_) => {
gl.vertex_attrib_pointer_f32(
buffer_info.location,
buffer_info.vector_size,
buffer_info.data_type,
buffer_info.normalized,
buffer_info.stride,
buffer_info.offset,
);
gl.enable_vertex_attrib_array(buffer_info.location);
}
}
}

pub(crate) unsafe fn unbind_vertex_array(&self, gl: &glow::Context) {
match self {
VAO::Emulated(vao) => vao.unbind_vertex_array(gl),
VAO::Native(_) => {
gl.bind_vertex_array(None);
}
}
}
}

/// If returned true no need to emulate vao
pub(crate) fn supports_vao(gl: &glow::Context) -> bool {
const WEBGL_PREFIX: &str = "WebGL ";
const OPENGL_ES_PREFIX: &str = "OpenGL ES ";

let version_string = unsafe { gl.get_parameter_string(glow::VERSION) };
tracing::debug!("GL version: {:?}.", version_string);

// Examples:
// * "WebGL 2.0 (OpenGL ES 3.0 Chromium)"
// * "WebGL 2.0"

if let Some(pos) = version_string.rfind(WEBGL_PREFIX) {
let version_str = &version_string[pos + WEBGL_PREFIX.len()..];
if version_str.contains("1.0") {
// need to test OES_vertex_array_object .
gl.supported_extensions()
.contains("OES_vertex_array_object")
} else {
true
}
} else if version_string.contains(OPENGL_ES_PREFIX) {
// glow targets es2.0+ so we don't concern about OpenGL ES-CM,OpenGL ES-CL
if version_string.contains("2.0") {
// need to test OES_vertex_array_object .
gl.supported_extensions()
.contains("OES_vertex_array_object")
} else {
true
}
} else {
// from OpenGL 3 vao into core
if version_string.starts_with('2') {
// I found APPLE_vertex_array_object , GL_ATI_vertex_array_object ,ARB_vertex_array_object
// but APPLE's and ATI's very old extension.
gl.supported_extensions()
.contains("ARB_vertex_array_object")
} else {
true
}
}
}
Loading

0 comments on commit 6f10e2e

Please sign in to comment.