Skip to content

Ringing around pre-rasterized circles. #5810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
karhu opened this issue Mar 16, 2025 · 5 comments · May be fixed by #5839
Open

Ringing around pre-rasterized circles. #5810

karhu opened this issue Mar 16, 2025 · 5 comments · May be fixed by #5839
Assignees
Labels
bug Something is broken visuals Renderings / graphics releated

Comments

@karhu
Copy link
Contributor

karhu commented Mar 16, 2025

Describe the bug:
Rendering darker circles on a brighter background can lead to "ringing" artifacts, where the outermost pixels of the circle are brighter than both the background and the circle. The artifact only shows when "Speed up filled circles with pre-rasterization" is enabled in the Tesselation Options (default).

Some details:

  • Circle fill is rgb(200,200,200)
  • Background fill is rgb(220,220,220)
  • Circle is rendered WITHOUT any border.
  • The observed ring is not just perceptual. Checked with MacOS color picker, and some pixels are rgb(230,230,230).
**Minimal Repro Example:**
#[derive(Default)]
struct ExampleApp {}

impl eframe::App for ExampleApp {
    fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
        // Tesselation Options GUI.
        egui::containers::SidePanel::left("left").show(ctx, |ui| {
            let mut options = ui.ctx().tessellation_options(|options| options.clone());
            ui.add(&mut options);
            ui.ctx()
                .tessellation_options_mut(|target| *target = options);
        });
        // Problematic dots/circles.
        egui::CentralPanel::default()
            .frame(egui::Frame::new())
            .show(ctx, |ui| {
                ui.painter().rect_filled(
                    ui.clip_rect(),
                    egui::CornerRadius::ZERO,
                    egui::Color32::from_rgb(220, 220, 220),
                );
                for x in 1..20 {
                    for y in 1..20 {
                        ui.painter().circle_filled(
                            ui.clip_rect().left_top() + egui::vec2(x as f32 * 10.0, y as f32 * 10.0),
                            1.5,
                            egui::Color32::from_rgb(200, 200, 200),
                        );
                    }
                }
            });
    }
}

fn main() -> eframe::Result {
    env_logger::init();
    eframe::run_native(
        "Bug Repro",
        Default::default(),
        Box::new(|_cc| Ok(Box::<ExampleApp>::default())),
    )
}

Expected behavior
No ringing. Same visuals independent of whether pre-rasterization is enabled.

Screenshots

Bad:
Image

Expected:
Image

Closeup:
Image

Desktop :

  • OSX, native

Additional context
This bears some similarities to #5751, but in a more controlled environment, given that an egui-internal texture is involved.

@karhu karhu added the bug Something is broken label Mar 16, 2025
@karhu
Copy link
Contributor Author

karhu commented Mar 17, 2025

Currently going down a rabbit hole of:

  • "Linear RGB" vs "sRGB (gamma)"
  • "Premultiplied Alpha" vs "Unpremultiplied Alpha"
  • "Alpha" vs "Coverage"
  • "Blending in linear RGB space" vs "Blending in sRGB space"
  • ...

In the meantime I found an even more basic repro: Just set the the circle fill and the background color to the same one, works better with brighter ones:

Image Image

@karhu
Copy link
Contributor Author

karhu commented Mar 17, 2025

Another update: My main suspect now is linear texture filtering in combination with sRGBA textures and premultiplied alpha.

A couple of observations:

  1. In this part of the Tessellator, circles are rendered using rectangles UV mapped to the closest (in size) circle from the texture atlas. To achieve the exact requested circle size, the rectangle is then scaled as necessary. If I hardcode this scale such that texture atlas pixels map exactly to final rendered pixels, the artifacts disappear (because no texture interpolation is necessary anymore).
    if self.options.prerasterized_discs && fill != Color32::TRANSPARENT {
    let radius_px = radius * self.pixels_per_point;
    // strike the right balance between some circles becoming too blurry, and some too sharp.
    let cutoff_radius = radius_px * 2.0_f32.powf(0.25);
    // Find the right disc radius for a crisp edge:
    // TODO(emilk): perhaps we can do something faster than this linear search.
    for disc in &self.prepared_discs {
    if cutoff_radius <= disc.r {
    let side = radius_px * disc.w / (self.pixels_per_point * disc.r);
    let rect = Rect::from_center_size(center, Vec2::splat(side));
    out.add_rect_with_uv(rect, disc.uv, fill);
    if stroke.is_empty() {
    return; // we are done
    } else {
    // we still need to do the stroke
    fill = Color32::TRANSPARENT; // don't fill again below
    break;
    }
    }
    }
    }
  2. If I force texture interpolation to NEAREST instead of LINEAR (requires modifications across the renderer) the artifacts disappear.
  3. Interpolation of sRGB textures is actually not very well specified: Implementations can choose if the "srgb to linear" conversion happens before or after the interpolation. I couldn't find anything about it in the WebGPU spec, but here is the relevant excerpt from the OpenGL Spec:

Image

@virtualritz
Copy link

virtualritz commented Mar 18, 2025

Interpolation of sRGB textures is actually not very well specified: Implementations can choose if the "srgb to linear" conversion happens before or after the interpolation. [...]

It must happen before unless you upsample. If you downsample a non-linear texture you introduce artifacts. Same rules as with alpha apply.

TLDR; you can't correctly blend/mix samples that have a burned-in curve of some sort (gamma, whatever).

Lastly, when you filter a pre-multiplied non-linear texture it gets worse at the edges (because the alpha exacerbates the error, the more transparent the sample, the more error; see also #5771).

@karhu
Copy link
Contributor Author

karhu commented Mar 18, 2025

Oh I completely agree, which is why I'm so surprised that the spec can be that vague. That being said, I'm not convinced that the incorrect interpolation order is what is happening here.

@virtualritz
Copy link

virtualritz commented Mar 18, 2025

Oh I completely agree, which is why I'm so surprised that the spec can be that vague. That being said, I'm not convinced that the incorrect interpolation order is what is happening here.

The reason is most likely that the spec was written by people who didn't know what they're talking about. At the risk of sounding like an old curmudgeon: as the years go by the quality of specs I'm confronted with in my line of work has steadily declined. Shockingly so if you compare with specs from the 80's or 90's.

The best specs are usually those written by a single individual who were at the helm of whatever tech the spec describes.
And specs of tech designed by committee (Khronos group, W3C, etc.) tend to be the worst. Both in what they contain, how they're written and how many important things they omit. 😉

@emilk emilk self-assigned this Mar 18, 2025
@emilk emilk added the visuals Renderings / graphics releated label Mar 19, 2025
emilk added a commit that referenced this issue Mar 21, 2025
The bug was in `Color32::from_rgba_unmultiplied` and by extension
affects:

* `Color32::from_rgba_unmultiplied`
* `hex_color!`
* `HexColor`
* `ColorImage::from_rgba_unmultiplied`
* All images with transparency (png, webp, …)
* `Color32::from_white_alpha`

The bug caused translucent colors to appear too bright.

## More
Color is hard.

When I started out egui I thought "linear space is objectively better,
for everything!" and then I've been slowly walking that back for various
reasons:

* sRGB textures not available everywhere
* gamma-space is more _perceptually_ even, so it makes sense to use for
anti-aliasing
* other applications do everything in gamma space, so that's what people
expect (this PR)

Similarly, pre-multiplied alpha _makes sense_ for blending colors. It
also enables additive colors, which is nice. But it does complicate
things. Especially when mixed with sRGB/gamma (As @karhu [points
out](#5824 (comment))).

## Related
* Closes #5751
* Closes #5771 ? (probably; hard to
tell without a repro)
* But not #5810

## TODO
* [x] I broke the RGBA u8 color picker. Fix it

---------

Co-authored-by: Andreas Reich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken visuals Renderings / graphics releated
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants