Skip to content
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

Anti-aliasing #1088

Open
sysint64 opened this issue Jul 11, 2020 · 15 comments · Fixed by #1091
Open

Anti-aliasing #1088

sysint64 opened this issue Jul 11, 2020 · 15 comments · Fixed by #1091
Labels
bug does not behave the way it is supposed to

Comments

@sysint64
Copy link
Contributor

sysint64 commented Jul 11, 2020

On a non-hi-dpi monitor, the picture looks quite blurry. As an example, look at the picture. The textbox has border-width = 1 and has black color, but it seems like border-width = 2 and has a gray color.

Screenshot_2020-07-11_11-05-22

@jneem
Copy link
Collaborator

jneem commented Jul 12, 2020

It's probably caused by us not correctly drawing in a way that's snapped to the pixel boundaries, like this. What platform are you on?

@sysint64
Copy link
Contributor Author

The platform is Linux. On macOS with retina works fine. This Is how I paint this border:

    fn paint(&mut self, ctx: &mut PaintCtx, data: &AccessorData, env: &Env) {
        let size = ctx.size();

        let rounded_rect = Rect::from_origin_size(Point::ORIGIN, size)
            .to_rounded_rect(env.get(theme::BUTTON_BORDER_RADIUS));

        let colors = TextboxColors::new(data, env);
        let gradient = LinearGradient::new(
            UnitPoint::TOP,
            UnitPoint::BOTTOM,
            vec![
                GradientStop {
                    pos: 0.0,
                    color: Color::grey(0.8),
                },
                GradientStop {
                    pos: 0.05,
                    color: Color::grey(0.9),
                },
                GradientStop {
                    pos: 0.2,
                    color: Color::grey(1.0),
                },
            ],
        );

        ctx.fill(rounded_rect, &gradient);
        ctx.stroke(rounded_rect, &env.get(theme::TEXT_BOX_BORDER_COLOR), 1.0);
    }

@sysint64
Copy link
Contributor Author

sysint64 commented Jul 12, 2020

It's probably caused by us not correctly drawing in a way that's snapped to the pixel boundaries, like this. What platform are you on?

This fixes the problem. But it little bit weird solution.

        let rounded_rect = Rect::from_origin_size(Point::ORIGIN, size)
            .inset(0.5)
            .to_rounded_rect(env.get(theme::BUTTON_BORDER_RADIUS));

@sysint64
Copy link
Contributor Author

Now I get blurry border for width = 2.0, found the problem it's caused by the Align::centered widget.
image

@rhzk
Copy link
Collaborator

rhzk commented Jul 17, 2020

Same Issue on windows, 1 pixel stroke becomes 2 blured pixels.

@luleyleo luleyleo added the bug does not behave the way it is supposed to label Jul 23, 2020
@jneem
Copy link
Collaborator

jneem commented Nov 12, 2020

As @dhardy commented on #1389, KAS also does some rounding to pixels. (But note that doing something similar in druid would require keeping around the window's Scale, since coordinates in druid aren't pixels.)

@cmyr
Copy link
Member

cmyr commented Nov 13, 2020

yea, this is definitely something we're not doing very responsibly, at the moment. There have been a few attempts, but nothing systematic. I'm really not sure what the best API for this would be; maybe something exposed on various contexts where you can pass geometric types and it will do the scaling and rounding for you? like,

trait PixelAlign {
    fn align(self, scale: Scale) -> Self;
}

// in various `Ctx`s
fn pixel_aligned<T: PixelAlign>(&self, shape: T) -> T {
    shape.align(self.scale)
}

And then we impl PixelAlign for all the kurbo types? I'm not sure if this would make sense as I haven't really thought through the problem very thoroughly, but it's what occurs to me.

@dhardy
Copy link
Contributor

dhardy commented Nov 14, 2020

Not using physical pixels internally, and then using extra steps to align to them seems like the hard way of solving a simple problem to me... but yes, it requires stashing rounded versions of each dimension for every drawable (or at least keeping the scale handy). Even using physical pixels in KAS, there are some drawing routines that require sub-pixel precision, but this is purely draw routines (e.g. drawing a slider).

@jneem
Copy link
Collaborator

jneem commented Nov 16, 2020

Did you mean to close this @cmyr? I agree that #1091 is progress, but I don't think it fixes this in general...

@jneem jneem reopened this Nov 16, 2020
@cmyr
Copy link
Member

cmyr commented Nov 16, 2020

Nope, did not close intentionally. thanks :)

@xarvic
Copy link
Collaborator

xarvic commented May 13, 2021

Since most shapes are created in each paint call again, maybe we could have two sets of draw methods in PaintCtx?
stroke, stroke_styled, fill and stroke_unaligned, stroke_styled_unaligned, fill_unaligned.

If i understand it correctly, we either align to the pixel center or the pixel border and which we use depends on weather it is stroke or fill and the line width.

@jneem
Copy link
Collaborator

jneem commented May 17, 2021

Hm, that's a very interesting idea! It wasn't even feasible until recently, when we started tracking window-relative positions. I don't quite see how to specify it in general though. I mean, for horizontal and vertical lines it's clear what it means to "align", but what if it's a diagonal line or a rectangle with non-integer-multiple-of-pixels width?

@xarvic
Copy link
Collaborator

xarvic commented May 21, 2021

but what if it's a diagonal line
I think we should still align the ends of those Lines and curves. The error is small, but it would probably look strange if some of the lines of a shape don't "touch" each other.

@cmyr
Copy link
Member

cmyr commented May 21, 2021

hmhm. I think what i would prefer is to have methods that do the alignment, and then the newly aligned shapes can be passed in to the draw methods.

This might look like:

pub trait AlignGeometry {
    fn align(&mut self);
    fn to_align(&self) -> Self;
}

and then we would have something on RenderContext, like

fn align_to_pixels<T: AlignGeometry>(&self, shape: &T) -> T;

... Or something? Do we want to expose different alignment behaviour ('expand', 'nearest', etc)? Do we want to have separate (pixel, display point) methods?

I've personally had some hesitance to dig into this problem too deeply, because it feels like there are going to be a bunch of weird platform concerns, weird behaviour on different screen resolutions, etc, but I do think it's something we want to get right. I think the first step will be to clearly define exactly what behaviour we want.

@jacksongoode
Copy link

jacksongoode commented Aug 10, 2024

Was there ever any resolution on this? Currently thinking about its resolution in Psst where Linux users get a blurry experience...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug does not behave the way it is supposed to
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants