Skip to content

Commit

Permalink
Remove extra_asserts and extra_debug_asserts feature flags (#4478)
Browse files Browse the repository at this point in the history
Removes `egui_assert` etc and replaces it with normal `debug_assert`
calls.

Previously you could opt-in to more runtime checks using feature flags.
Now these extra runtime checks are always enabled for debug builds.

You are most likely to encounter them if you use negative sizes or NaNs
or other similar bugs.
These usually indicate bugs in user space.
  • Loading branch information
emilk authored May 10, 2024
1 parent 155e138 commit f19f991
Show file tree
Hide file tree
Showing 32 changed files with 90 additions and 182 deletions.
4 changes: 0 additions & 4 deletions crates/ecolor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ all-features = true
[features]
default = []

## Enable additional checks if debug assertions are enabled (debug builds).
extra_debug_asserts = []
## Always enable additional checks.
extra_asserts = []

[dependencies]
#! ### Optional dependencies
Expand Down
4 changes: 2 additions & 2 deletions crates/ecolor/src/color32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl Color32 {
/// This is perceptually even, and faster that [`Self::linear_multiply`].
#[inline]
pub fn gamma_multiply(self, factor: f32) -> Self {
crate::ecolor_assert!(0.0 <= factor && factor <= 1.0);
debug_assert!(0.0 <= factor && factor <= 1.0);
let Self([r, g, b, a]) = self;
Self([
(r as f32 * factor + 0.5) as u8,
Expand All @@ -215,7 +215,7 @@ impl Color32 {
/// You likely want to use [`Self::gamma_multiply`] instead.
#[inline]
pub fn linear_multiply(self, factor: f32) -> Self {
crate::ecolor_assert!(0.0 <= factor && factor <= 1.0);
debug_assert!(0.0 <= factor && factor <= 1.0);
// As an unfortunate side-effect of using premultiplied alpha
// we need a somewhat expensive conversion to linear space and back.
Rgba::from(self).multiply(factor).into()
Expand Down
16 changes: 0 additions & 16 deletions crates/ecolor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,22 +135,6 @@ pub fn gamma_from_linear(linear: f32) -> f32 {

// ----------------------------------------------------------------------------

/// An assert that is only active when `epaint` is compiled with the `extra_asserts` feature
/// or with the `extra_debug_asserts` feature in debug builds.
#[macro_export]
macro_rules! ecolor_assert {
($($arg: tt)*) => {
if cfg!(any(
feature = "extra_asserts",
all(feature = "extra_debug_asserts", debug_assertions),
)) {
assert!($($arg)*);
}
}
}

// ----------------------------------------------------------------------------

/// Cheap and ugly.
/// Made for graying out disabled `Ui`s.
pub fn tint_color_towards(color: Color32, target: Color32) -> Color32 {
Expand Down
8 changes: 4 additions & 4 deletions crates/ecolor/src/rgba.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,22 +98,22 @@ impl Rgba {

#[inline]
pub fn from_luminance_alpha(l: f32, a: f32) -> Self {
crate::ecolor_assert!(0.0 <= l && l <= 1.0);
crate::ecolor_assert!(0.0 <= a && a <= 1.0);
debug_assert!(0.0 <= l && l <= 1.0);
debug_assert!(0.0 <= a && a <= 1.0);
Self([l * a, l * a, l * a, a])
}

/// Transparent black
#[inline]
pub fn from_black_alpha(a: f32) -> Self {
crate::ecolor_assert!(0.0 <= a && a <= 1.0);
debug_assert!(0.0 <= a && a <= 1.0);
Self([0.0, 0.0, 0.0, a])
}

/// Transparent white
#[inline]
pub fn from_white_alpha(a: f32) -> Self {
crate::ecolor_assert!(0.0 <= a && a <= 1.0, "a: {}", a);
debug_assert!(0.0 <= a && a <= 1.0, "a: {a}");
Self([a, a, a, a])
}

Expand Down
5 changes: 0 additions & 5 deletions crates/egui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,6 @@ deadlock_detection = ["epaint/deadlock_detection"]
## If you plan on specifying your own fonts you may disable this feature.
default_fonts = ["epaint/default_fonts"]

## Enable additional checks if debug assertions are enabled (debug builds).
extra_debug_asserts = ["epaint/extra_debug_asserts"]
## Always enable additional checks.
extra_asserts = ["epaint/extra_asserts"]

## Turn on the `log` feature, that makes egui log some errors using the [`log`](https://docs.rs/log) crate.
log = ["dep:log", "epaint/log"]

Expand Down
2 changes: 1 addition & 1 deletion crates/egui/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1748,7 +1748,7 @@ impl Context {
let name = name.into();
let image = image.into();
let max_texture_side = self.input(|i| i.max_texture_side);
crate::egui_assert!(
debug_assert!(
image.width() <= max_texture_side && image.height() <= max_texture_side,
"Texture {:?} has size {}x{}, but the maximum texture side is {}",
name,
Expand Down
10 changes: 5 additions & 5 deletions crates/egui/src/frame_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl FrameState {
/// This is the "background" area, what egui doesn't cover with panels (but may cover with windows).
/// This is also the area to which windows are constrained.
pub(crate) fn available_rect(&self) -> Rect {
crate::egui_assert!(
debug_assert!(
self.available_rect.is_finite(),
"Called `available_rect()` before `Context::run()`"
);
Expand All @@ -126,7 +126,7 @@ impl FrameState {

/// Shrink `available_rect`.
pub(crate) fn allocate_left_panel(&mut self, panel_rect: Rect) {
crate::egui_assert!(
debug_assert!(
panel_rect.min.distance(self.available_rect.min) < 0.1,
"Mismatching left panel. You must not create a panel from within another panel."
);
Expand All @@ -137,7 +137,7 @@ impl FrameState {

/// Shrink `available_rect`.
pub(crate) fn allocate_right_panel(&mut self, panel_rect: Rect) {
crate::egui_assert!(
debug_assert!(
panel_rect.max.distance(self.available_rect.max) < 0.1,
"Mismatching right panel. You must not create a panel from within another panel."
);
Expand All @@ -148,7 +148,7 @@ impl FrameState {

/// Shrink `available_rect`.
pub(crate) fn allocate_top_panel(&mut self, panel_rect: Rect) {
crate::egui_assert!(
debug_assert!(
panel_rect.min.distance(self.available_rect.min) < 0.1,
"Mismatching top panel. You must not create a panel from within another panel."
);
Expand All @@ -159,7 +159,7 @@ impl FrameState {

/// Shrink `available_rect`.
pub(crate) fn allocate_bottom_panel(&mut self, panel_rect: Rect) {
crate::egui_assert!(
debug_assert!(
panel_rect.max.distance(self.available_rect.max) < 0.1,
"Mismatching bottom panel. You must not create a panel from within another panel."
);
Expand Down
2 changes: 1 addition & 1 deletion crates/egui/src/grid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl GridLayout {
// TODO(emilk): respect current layout

let initial_available = ui.placer().max_rect().intersect(ui.cursor());
crate::egui_assert!(
debug_assert!(
initial_available.min.x.is_finite(),
"Grid not yet available for right-to-left layouts"
);
Expand Down
46 changes: 23 additions & 23 deletions crates/egui/src/layout.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{egui_assert, emath::*, Align};
use crate::{emath::*, Align};
use std::f32::INFINITY;

// ----------------------------------------------------------------------------
Expand Down Expand Up @@ -66,9 +66,9 @@ impl Region {
}

pub fn sanity_check(&self) {
egui_assert!(!self.min_rect.any_nan());
egui_assert!(!self.max_rect.any_nan());
egui_assert!(!self.cursor.any_nan());
debug_assert!(!self.min_rect.any_nan());
debug_assert!(!self.max_rect.any_nan());
debug_assert!(!self.cursor.any_nan());
}
}

Expand Down Expand Up @@ -389,8 +389,8 @@ impl Layout {
/// ## Doing layout
impl Layout {
pub fn align_size_within_rect(&self, size: Vec2, outer: Rect) -> Rect {
egui_assert!(size.x >= 0.0 && size.y >= 0.0);
egui_assert!(!outer.is_negative());
debug_assert!(size.x >= 0.0 && size.y >= 0.0);
debug_assert!(!outer.is_negative());
self.align2().align_size_within_rect(size, outer)
}

Expand All @@ -416,8 +416,8 @@ impl Layout {
}

pub(crate) fn region_from_max_rect(&self, max_rect: Rect) -> Region {
egui_assert!(!max_rect.any_nan());
egui_assert!(max_rect.is_finite());
debug_assert!(!max_rect.any_nan());
debug_assert!(max_rect.is_finite());
let mut region = Region {
min_rect: Rect::NOTHING, // temporary
max_rect,
Expand Down Expand Up @@ -450,9 +450,9 @@ impl Layout {
/// Given the cursor in the region, how much space is available
/// for the next widget?
fn available_from_cursor_max_rect(&self, cursor: Rect, max_rect: Rect) -> Rect {
egui_assert!(!cursor.any_nan());
egui_assert!(!max_rect.any_nan());
egui_assert!(max_rect.is_finite());
debug_assert!(!cursor.any_nan());
debug_assert!(!max_rect.any_nan());
debug_assert!(max_rect.is_finite());

// NOTE: in normal top-down layout the cursor has moved below the current max_rect,
// but the available shouldn't be negative.
Expand Down Expand Up @@ -506,7 +506,7 @@ impl Layout {
avail.max.y = y;
}

egui_assert!(!avail.any_nan());
debug_assert!(!avail.any_nan());

avail
}
Expand All @@ -517,7 +517,7 @@ impl Layout {
/// Use `justify_and_align` to get the inner `widget_rect`.
pub(crate) fn next_frame(&self, region: &Region, child_size: Vec2, spacing: Vec2) -> Rect {
region.sanity_check();
egui_assert!(child_size.x >= 0.0 && child_size.y >= 0.0);
debug_assert!(child_size.x >= 0.0 && child_size.y >= 0.0);

if self.main_wrap {
let available_size = self.available_rect_before_wrap(region).size();
Expand Down Expand Up @@ -597,7 +597,7 @@ impl Layout {

fn next_frame_ignore_wrap(&self, region: &Region, child_size: Vec2) -> Rect {
region.sanity_check();
egui_assert!(child_size.x >= 0.0 && child_size.y >= 0.0);
debug_assert!(child_size.x >= 0.0 && child_size.y >= 0.0);

let available_rect = self.available_rect_before_wrap(region);

Expand Down Expand Up @@ -630,16 +630,16 @@ impl Layout {
frame_rect = frame_rect.translate(Vec2::Y * (region.cursor.top() - frame_rect.top()));
}

egui_assert!(!frame_rect.any_nan());
egui_assert!(!frame_rect.is_negative());
debug_assert!(!frame_rect.any_nan());
debug_assert!(!frame_rect.is_negative());

frame_rect
}

/// Apply justify (fill width/height) and/or alignment after calling `next_space`.
pub(crate) fn justify_and_align(&self, frame: Rect, mut child_size: Vec2) -> Rect {
egui_assert!(child_size.x >= 0.0 && child_size.y >= 0.0);
egui_assert!(!frame.is_negative());
debug_assert!(child_size.x >= 0.0 && child_size.y >= 0.0);
debug_assert!(!frame.is_negative());

if self.horizontal_justify() {
child_size.x = child_size.x.at_least(frame.width()); // fill full width
Expand All @@ -657,10 +657,10 @@ impl Layout {
) -> Rect {
let frame = self.next_frame_ignore_wrap(region, size);
let rect = self.align_size_within_rect(size, frame);
egui_assert!(!rect.any_nan());
egui_assert!(!rect.is_negative());
egui_assert!((rect.width() - size.x).abs() < 1.0 || size.x == f32::INFINITY);
egui_assert!((rect.height() - size.y).abs() < 1.0 || size.y == f32::INFINITY);
debug_assert!(!rect.any_nan());
debug_assert!(!rect.is_negative());
debug_assert!((rect.width() - size.x).abs() < 1.0 || size.x == f32::INFINITY);
debug_assert!((rect.height() - size.y).abs() < 1.0 || size.y == f32::INFINITY);
rect
}

Expand Down Expand Up @@ -703,7 +703,7 @@ impl Layout {
widget_rect: Rect,
item_spacing: Vec2,
) {
egui_assert!(!cursor.any_nan());
debug_assert!(!cursor.any_nan());
if self.main_wrap {
if cursor.intersects(frame_rect.shrink(1.0)) {
// make row/column larger if necessary
Expand Down
16 changes: 0 additions & 16 deletions crates/egui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,22 +549,6 @@ macro_rules! github_link_file {

// ----------------------------------------------------------------------------

/// An assert that is only active when `egui` is compiled with the `extra_asserts` feature
/// or with the `extra_debug_asserts` feature in debug builds.
#[macro_export]
macro_rules! egui_assert {
($($arg: tt)*) => {
if cfg!(any(
feature = "extra_asserts",
all(feature = "extra_debug_asserts", debug_assertions),
)) {
assert!($($arg)*);
}
}
}

// ----------------------------------------------------------------------------

/// The minus character: <https://www.compart.com/en/unicode/U+2212>
pub(crate) const MINUS_CHAR_STR: &str = "−";

Expand Down
12 changes: 6 additions & 6 deletions crates/egui/src/placer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl Placer {
/// This is what you then pass to `advance_after_rects`.
/// Use `justify_and_align` to get the inner `widget_rect`.
pub(crate) fn next_space(&self, child_size: Vec2, item_spacing: Vec2) -> Rect {
egui_assert!(child_size.is_finite() && child_size.x >= 0.0 && child_size.y >= 0.0);
debug_assert!(child_size.is_finite() && child_size.x >= 0.0 && child_size.y >= 0.0);
self.region.sanity_check();
if let Some(grid) = &self.grid {
grid.next_cell(self.region.cursor, child_size)
Expand All @@ -127,8 +127,8 @@ impl Placer {

/// Apply justify or alignment after calling `next_space`.
pub(crate) fn justify_and_align(&self, rect: Rect, child_size: Vec2) -> Rect {
crate::egui_assert!(!rect.any_nan());
crate::egui_assert!(!child_size.any_nan());
debug_assert!(!rect.any_nan());
debug_assert!(!child_size.any_nan());

if let Some(grid) = &self.grid {
grid.justify_and_align(rect, child_size)
Expand All @@ -140,7 +140,7 @@ impl Placer {
/// Advance the cursor by this many points.
/// [`Self::min_rect`] will expand to contain the cursor.
pub(crate) fn advance_cursor(&mut self, amount: f32) {
crate::egui_assert!(
debug_assert!(
self.grid.is_none(),
"You cannot advance the cursor when in a grid layout"
);
Expand All @@ -158,8 +158,8 @@ impl Placer {
widget_rect: Rect,
item_spacing: Vec2,
) {
egui_assert!(!frame_rect.any_nan());
egui_assert!(!widget_rect.any_nan());
debug_assert!(!frame_rect.any_nan());
debug_assert!(!widget_rect.any_nan());
self.region.sanity_check();

if let Some(grid) = &mut self.grid {
Expand Down
2 changes: 1 addition & 1 deletion crates/egui/src/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ impl Response {
/// You may not call [`Self::interact`] on the resulting `Response`.
pub fn union(&self, other: Self) -> Self {
assert!(self.ctx == other.ctx);
crate::egui_assert!(
debug_assert!(
self.layer_id == other.layer_id,
"It makes no sense to combine Responses from two different layers"
);
Expand Down
Loading

0 comments on commit f19f991

Please sign in to comment.