emilk / egui

egui: an easy-to-use immediate mode GUI in Rust that runs on both web and native
https://www.egui.rs/
Apache License 2.0
22.23k stars 1.6k forks source link

Selective feathering (antialias) #2735

Open cdobrescu opened 1 year ago

cdobrescu commented 1 year ago

Is your feature request related to a problem? Please describe. I would like to be able to turn off antialiasing for a specific plot or even a chart inside the plot. Bar charts, which consist only of shapes composed from vertical and horizontal lines, have a crispier pixel perfect look when antialiasing is disabled. Antialiased: image Pixel perfect: image Also, the gridlines look a lot better without antialiasing

Describe the solution you'd like I'm not sure what's the best API change for this would be:

Describe alternatives you've considered I tried turning it off inside a function that creates the chart:

 ui.ctx().memory_mut(|mem| mem.options.tessellation_options.feathering = false);

and turn it back later after the shapes were added to the draw list. But I think that shape creation and drawing are async so the final result was still showing everything antialiased.

parasyte commented 1 year ago

The way feathering is integrated into the tessellator makes it kind of difficult to control from the user side. As a quick test, I decided to try hacking the stroke methods to conditionally turn off feathering if the path is only composed of axis-aligned line segments:

diff --git a/crates/epaint/src/tessellator.rs b/crates/epaint/src/tessellator.rs
index a28daa60..66e3b634 100644
--- a/crates/epaint/src/tessellator.rs
+++ b/crates/epaint/src/tessellator.rs
@@ -474,18 +474,58 @@ impl Path {

     /// Open-ended.
     pub fn stroke_open(&self, feathering: f32, stroke: Stroke, out: &mut Mesh) {
+        let feathering = if self.is_axis_aligned(PathType::Open) {
+            0.0
+        } else {
+            feathering
+        };
         stroke_path(feathering, &self.0, PathType::Open, stroke, out);
     }

     /// A closed path (returning to the first point).
     pub fn stroke_closed(&self, feathering: f32, stroke: Stroke, out: &mut Mesh) {
+        let feathering = if self.is_axis_aligned(PathType::Closed) {
+            0.0
+        } else {
+            feathering
+        };
         stroke_path(feathering, &self.0, PathType::Closed, stroke, out);
     }

     pub fn stroke(&self, feathering: f32, path_type: PathType, stroke: Stroke, out: &mut Mesh) {
+        let feathering = if self.is_axis_aligned(path_type) {
+            0.0
+        } else {
+            feathering
+        };
         stroke_path(feathering, &self.0, path_type, stroke, out);
     }

+    fn is_axis_aligned(&self, path_type: PathType) -> bool {
+        let mut points = self.0.iter().peekable();
+
+        if let Some(point) = points.peek() {
+            let first_pos = point.pos;
+
+            while let Some(point) = points.next() {
+                let this_pos = point.pos;
+                let next_pos = match points.peek() {
+                    None if path_type == PathType::Closed => first_pos,
+                    None => break,
+                    Some(point) => point.pos,
+                };
+
+                if this_pos.x != next_pos.x && this_pos.y != next_pos.y {
+                    return false;
+                }
+            }
+
+            true
+        } else {
+            false
+        }
+    }
+
     /// The path is taken to be closed (i.e. returning to the start again).
     ///
     /// Calling this may reverse the vertices in the path if they are wrong winding order.

For a simple patch, the results are ok:

Before:

before

After:

after

Compare these images in two tabs. The difference is subtle with GitHub's scaling.

One of the issues with this approach is that it ignores paths like rounded rectangles, which have long axis-aligned segments and very short non-aligned segments. You can see this in the window border on the "after" screenshot. It's shifted by a subpixel in each direction, so it appears thicker than the separator lines in the side bar.

A better approach would be selective feathering inside the stroke_path() function on a per-path-segment basis instead of the whole path.

Another issue with disabling any of the feathering is that some rendering looks much worse, like the bar chart plot in the demo app:

image

This happens, of course, because the bars are drawn right next to each other and whether or not the lines share the same pixel column depends on the scale of the Plot widget. You can adjust the window size until all of the lines overlap perfectly and then it looks nice. 😂 But that's a bad user experience.

cdobrescu commented 1 year ago

Thanks for looking into this @parasyte. Looking at your not antialiased gridlines also make me think that some pixel rounding/alignment is necessary

I also gave it a try using a different approach that basically disables antialiasing for a single shape (or even a vector or shapes), instead of detecting if line or path is vertical or horizontal.

diff --git a/crates/epaint/src/shape.rs b/crates/epaint/src/shape.rs
index 9304a27b..751feb12 100644
--- a/crates/epaint/src/shape.rs
+++ b/crates/epaint/src/shape.rs
@@ -24,6 +24,9 @@ pub enum Shape {
     /// Paint nothing. This can be useful as a placeholder.
     Noop,

+    /// Shape that won't be antialiased during tesselation
+    NoAntialias(Box<Shape>),
+
     /// Recursively nest more shapes - sometimes a convenience to be able to do.
     /// For performance reasons it is better to avoid it.
     Vec(Vec<Shape>),
@@ -249,6 +252,7 @@ impl Shape {
     pub fn visual_bounding_rect(&self) -> Rect {
         match self {
             Self::Noop => Rect::NOTHING,
+            Self::NoAntialias(shape) => (*shape).visual_bounding_rect(),
             Self::Vec(shapes) => {
                 let mut rect = Rect::NOTHING;
                 for shape in shapes {
@@ -290,6 +294,9 @@ impl Shape {
     pub fn translate(&mut self, delta: Vec2) {
         match self {
             Shape::Noop => {}
+            Shape::NoAntialias(shape) => {
+                (*shape).translate(delta);
+            }
             Shape::Vec(shapes) => {
                 for shape in shapes {
                     shape.translate(delta);

And in tesselator I'm doing this:

diff --git a/crates/epaint/src/tessellator.rs b/crates/epaint/src/tessellator.rs
index a28daa60..4bd9e041 100644
--- a/crates/epaint/src/tessellator.rs
+++ b/crates/epaint/src/tessellator.rs
@@ -1096,6 +1096,16 @@ impl Tessellator {
     pub fn tessellate_shape(&mut self, shape: Shape, out: &mut Mesh) {
         match shape {
             Shape::Noop => {}
+            Shape::NoAntialias(shape) => {
+                let orig_feathering = self.feathering;
+                if orig_feathering > 0.0 {
+                    self.feathering = 0.0;
+                }
+                self.tessellate_shape(*shape, out);
+                if orig_feathering > 0.0 {
+                    self.feathering = orig_feathering;
+                }
+            }
             Shape::Vec(vec) => {
                 for shape in vec {
                     self.tessellate_shape(shape, out);

To test if this is working I modified the Bar::add_shapes method to something like this:

diff --git a/crates/egui/src/widgets/plot/items/bar.rs b/crates/egui/src/widgets/plot/items/bar.rs
index a94a7d44..658c1e47 100644
--- a/crates/egui/src/widgets/plot/items/bar.rs
+++ b/crates/egui/src/widgets/plot/items/bar.rs
@@ -127,12 +127,12 @@ impl Bar {
         };

         let rect = transform.rect_from_values(&self.bounds_min(), &self.bounds_max());
-        let rect = Shape::Rect(RectShape {
+        let rect = Shape::NoAntialias(Box::new(Shape::Rect(RectShape {
             rect,
             rounding: Rounding::none(),
             fill,
             stroke,
-        });
+        })));

         shapes.push(rect);
     }

Now I'm struggling to find a good way to add a context or painter flag that says "Every shape that will be added to the painter while this flag is set, wrap it as NoAntialias(Shape)" Ideas are welcome!

cdobrescu commented 1 year ago

I think I got it! I just added dummy shape that is able to toggle antialiasing either on or off

diff --git a/crates/epaint/src/shape.rs b/crates/epaint/src/shape.rs
index 9304a27b..c166534b 100644
--- a/crates/epaint/src/shape.rs
+++ b/crates/epaint/src/shape.rs
@@ -24,6 +24,10 @@ pub enum Shape {
     /// Paint nothing. This can be useful as a placeholder.
     Noop,

+    /// Dummy shape that can be used to toggle antaliasing on or off for the
+    /// next shapes in the paint list
+    Antialias { enabled: bool },
+
     /// Recursively nest more shapes - sometimes a convenience to be able to do.
     /// For performance reasons it is better to avoid it.
     Vec(Vec<Shape>),
@@ -249,6 +253,7 @@ impl Shape {
     pub fn visual_bounding_rect(&self) -> Rect {
         match self {
             Self::Noop => Rect::NOTHING,
+            Self::Antialias { .. } => Rect::NOTHING,
             Self::Vec(shapes) => {
                 let mut rect = Rect::NOTHING;
                 for shape in shapes {
@@ -290,6 +295,7 @@ impl Shape {
     pub fn translate(&mut self, delta: Vec2) {
         match self {
             Shape::Noop => {}
+            Shape::Antialias { .. } => {}
             Shape::Vec(shapes) => {
                 for shape in shapes {
                     shape.translate(delta);
diff --git a/crates/epaint/src/tessellator.rs b/crates/epaint/src/tessellator.rs
index a28daa60..1257e6b8 100644
--- a/crates/epaint/src/tessellator.rs
+++ b/crates/epaint/src/tessellator.rs
@@ -1096,6 +1096,14 @@ impl Tessellator {
     pub fn tessellate_shape(&mut self, shape: Shape, out: &mut Mesh) {
         match shape {
             Shape::Noop => {}
+            Shape::Antialias { enabled } => {
+                if enabled {
+                    self.feathering =
+                        self.options.feathering_size_in_pixels * (1.0 * self.pixels_per_point);
+                } else {
+                    self.feathering = 0.0;
+                }
+            }
             Shape::Vec(vec) => {
                 for shape in vec {
                     self.tessellate_shape(shape, out);

Here are two plots, first not antialiased scond aone antiaalised

ui.painter().add(Shape::Antialias{enabled: false});
// first plot
Plot::new(..)
ui.painter().add(Shape::Antialias{enabled: true});
//second plot
Plot::new(..)

image

parasyte commented 1 year ago

Looking at your not antialiased gridlines also make me think that some pixel rounding/alignment is necessary

Disabling the feathering feature forces tessellated lines to align with the pixel grid. I.e. it is rounding and that's why you see the grid lines not-evenly spaced. The rounding caused by disabling feathering is going to result in what is shown.

Fixing this is not a matter of rounding the positions of individual shapes, but scaling everything properly so they align with the pixel grid. What I mean is, the grid lines have a non-integer spacing between them and the only fix is making the spacing an exact integer. Thus the "scale" of the entire object is what is important for rendering quality with feathering disabled.

I also gave it a try using a different approach that basically disables antialiasing for a single shape (or even a vector or shapes), instead of detecting if line or path is vertical or horizontal.

This might work out for some shapes, but the obvious downside is that it looks really bad in most cases. (See above: grid lines, bar chart, etc.)

I think I got it! I just added dummy shape that is able to toggle antialiasing either on or off

It would be better if it was hierarchical so that it isn't possible to misuse. E.g. forgetting to turn antialiasing back on would be disastrous to rendering quality and hard to debug in complex UIs.

cdobrescu commented 1 year ago

Thanks for clarifying all these things. I'll continue studying the plot api, screen to plot, and plot to screen functions, I think there's some potential there, to get things evenly spread on a pixel basis. As for making toggling logic hierarchical I agree it would better a pattern but i'm not sure if i can do it without attaching someting to the shape, that the tesselator can use