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

The image displayed by the Canvas is corrupted #694

Closed
Athorus opened this issue Oct 20, 2019 · 10 comments
Closed

The image displayed by the Canvas is corrupted #694

Athorus opened this issue Oct 20, 2019 · 10 comments

Comments

@Athorus
Copy link

Athorus commented Oct 20, 2019

Describe the bug
The image displayed at the screen by the canvas is a little bit corrupted. When Image is displayed directly, everything is fine.

To Reproduce
See code below.
File resources.zip is provided too (contains 2 png).

Expected behavior
Canvas should display the same image as the one in input. No corrupted data is expected.

Screenshots or pasted code
If you look closely at the two first dark text lines, you can observe the second line is a little bit corrupted. There are black pixels around text.
With a light image (maybe because the contrast is higher), I cannot see any difference.
canvas

use ggez::ContextBuilder;
use ggez::conf::Conf;
use ggez::Context;
use ggez::event::{self, EventHandler};
use ggez::GameResult;
use ggez::graphics::{self, Canvas, Color, DrawParam, Image};

use mint::Point2;

struct Application {
    dark_image: Image,
    dark_canvas: Canvas,
    light_image: Image,
    light_canvas: Canvas,
    canvas_image: Image
}

impl Application {
    fn new(ctx: &mut Context) -> Self {
        let (dark_image, dark_canvas) = Self::image_canvas(ctx, "/dark-image.png");
        let (light_image, light_canvas) = Self::image_canvas(ctx, "/light-image.png");

        let canvas_image = Self::canvas(ctx, &dark_image).into_inner();

        Self {dark_image, dark_canvas, light_image, light_canvas, canvas_image}
    }

    fn image_canvas(ctx: &mut Context, file: &str) -> (Image, Canvas) {
        let image = Image::new(ctx, file).unwrap();
        let canvas = Self::canvas(ctx, &image);
        (image, canvas)
    }

    fn canvas(ctx: &mut Context, image: &Image) -> Canvas {
        let canvas = Canvas::with_window_size(ctx).unwrap();
        graphics::set_canvas(ctx, Some(&canvas));
        graphics::draw(ctx, image, DrawParam::new()).unwrap();
        graphics::set_canvas(ctx, None);

        canvas
    }
}

impl EventHandler for Application {
    fn update(&mut self, _: &mut Context) -> GameResult<()> {
        Ok(())
    }

    fn draw(&mut self, ctx: &mut Context) -> GameResult<()> {
        graphics::clear(ctx, Color{r: 0., g: 0.3, b: 0., a: 1.});

        graphics::draw(ctx, &self.dark_image, DrawParam::new()).unwrap();
        let point2 = Point2 {x: 0., y: 60.};
        graphics::draw(ctx, &self.dark_canvas, DrawParam::new().dest(point2)).unwrap();

        let point2 = Point2 {x: 0., y: 200.};
        graphics::draw(ctx, &self.light_image, DrawParam::new().dest(point2)).unwrap();
        let point2 = Point2 {x: 0., y: 260.};
        graphics::draw(ctx, &self.light_canvas, DrawParam::new().dest(point2)).unwrap();

        graphics::draw(ctx, &self.canvas_image, DrawParam::new()).unwrap();

        graphics::present(ctx).unwrap();
        Ok(())
    }
}

pub fn main() {
    let c = Conf::new();
    let (ctx, event_loop) = &mut ContextBuilder::new("canvas", "athorus").conf(c).build().unwrap();
    let app = &mut Application::new(ctx);
    event::run(ctx, event_loop, app).unwrap();
}

Hardware and Software:

  • ggez version: 0.5.1
  • OS: Windows 10,
  • Graphics card: AMD Radeon(TM) R9 290X
  • Graphics card drivers: Radeon 19.9.2
@Athorus
Copy link
Author

Athorus commented Oct 20, 2019

When zoomed, we can see the light image is corrupted too, but the difference is subtle.

image
image

@icefoxen
Copy link
Contributor

Looks like something is a bit messed up with pre-multiplied alpha, or lack of it. Possibly related: #301

@PSteinhaus
Copy link
Member

PSteinhaus commented May 28, 2021

Just tested, this issue is still acute.
Updated example code:

use ggez::ContextBuilder;
use ggez::conf::Conf;
use ggez::Context;
use ggez::event::{self, EventHandler};
use ggez::GameResult;
use ggez::graphics::{self, Canvas, Color, DrawParam, Image};

use mint::Point2;

struct Application {
    dark_image: Image,
    dark_canvas: Canvas,
    light_image: Image,
    light_canvas: Canvas,
    canvas_image: Image
}

impl Application {
    fn new(ctx: &mut Context) -> Self {
        let (dark_image, dark_canvas) = Self::image_canvas(ctx, "/dark-image.png");
        let (light_image, light_canvas) = Self::image_canvas(ctx, "/light-image.png");

        let canvas_image = Self::canvas(ctx, &dark_image).to_image(ctx).unwrap();

        Self {dark_image, dark_canvas, light_image, light_canvas, canvas_image}
    }

    fn image_canvas(ctx: &mut Context, file: &str) -> (Image, Canvas) {
        let image = Image::new(ctx, file).unwrap();
        let canvas = Self::canvas(ctx, &image);
        (image, canvas)
    }

    fn canvas(ctx: &mut Context, image: &Image) -> Canvas {
        let canvas = Canvas::with_window_size(ctx).unwrap();
        graphics::set_canvas(ctx, Some(&canvas));
        graphics::draw(ctx, image, DrawParam::new()).unwrap();
        graphics::set_canvas(ctx, None);

        canvas
    }
}

impl EventHandler for Application {
    fn update(&mut self, _: &mut Context) -> GameResult<()> {
        Ok(())
    }

    fn draw(&mut self, ctx: &mut Context) -> GameResult<()> {
        graphics::clear(ctx, Color{r: 0., g: 0.3, b: 0., a: 1.});

        graphics::draw(ctx, &self.dark_image, DrawParam::new()).unwrap();
        let point2 = Point2 {x: 0., y: 60.};
        graphics::draw(ctx, &self.dark_canvas, DrawParam::new().dest(point2)).unwrap();

        let point2 = Point2 {x: 0., y: 200.};
        graphics::draw(ctx, &self.light_image, DrawParam::new().dest(point2)).unwrap();
        let point2 = Point2 {x: 0., y: 260.};
        graphics::draw(ctx, &self.light_canvas, DrawParam::new().dest(point2)).unwrap();

        graphics::draw(ctx, &self.canvas_image, DrawParam::new()).unwrap();

        graphics::present(ctx).unwrap();
        Ok(())
    }
}

pub fn main() {
    let c = Conf::new();
    let (mut ctx, event_loop) = ContextBuilder::new("canvas", "athorus").build().unwrap();
    let app = Application::new(&mut ctx);
    event::run(ctx, event_loop, app)
}

@PSteinhaus
Copy link
Member

PSteinhaus commented Jun 2, 2021

Ok two things:

One: The canvas_image is always empty when using the code posted here.

This is due to the fact that we don't flush the draw buffer in the canvas function. For something to actually be on that canvas before we destroy it we therefore need to change it like this:

fn canvas(ctx: &mut Context, image: &Image) -> Canvas {
        let mut canvas = Canvas::with_window_size(ctx).unwrap();
        graphics::set_canvas(ctx, Some(&canvas));
        graphics::draw(ctx, image, DrawParam::new()).unwrap();
        graphics::present(ctx);
        graphics::set_canvas(ctx, None);

        canvas
    }

Two: I got the source of the problem.

The alpha blend mode is defined as follows:

pub const ALPHA: Blend = Blend {
        color: BlendChannel {
            equation: Equation::Add,
            source: Factor::ZeroPlus(BlendValue::SourceAlpha),
            destination: Factor::OneMinus(BlendValue::SourceAlpha),
        },
        alpha: BlendChannel {
            equation: Equation::Add,
            source: Factor::One,
            destination: Factor::One,
        },
    };

Think about what that means for the color channels when the background (destination) is transparent black.

  • What we want: The color value should be that of the source
  • What we get: The color value is darker, because the source color value is multiplied with the source alpha value and thereby decreased (except if source alpha = 1)

Now we only need to think about whether the alpha blend mode could be defined differently so that it still works for non-transparent backgrounds AND also for transparent backgrounds.

EDIT:
I'm pretty sure we can't solve this through redefinition. Currently a color value is computed like this:

new_col_val = source_col_val * source_alpha + dest_col_val * (1 - source_alpha)

This works perfectly fine if the target isn't transparent. But if it IS then we'd want to calculate color values like this:

new_col_val = source_col_val * (source_alpha + (1 - dest_alpha * (1 - source_alpha)))
              + dest_col_val * (dest_alpha * (1 - source_alpha))

The reasoning behind this formula is that the final value must be

  • the source value scaled by (the source alpha PLUS the portion of the value that does not get overwritten by the background) [line 1]
  • plus the background value scaled by the portion of the value that does get overwritten by the background [line 2]

And the "portion of the value that does get overwritten by the background" is given by dest_alpha * (1 - source_alpha) because (1 - source_alpha) gives us the percentage that doesn't yet get covered by the foreground (i.e. source) and dest_alpha needs to be multiplied to that to give us how much of that percentage is actually used by the background color value.

It's late and maybe there's a way to simplify this a bit, but even if there was I'm pretty sure that we'd still have to pay a significant performance cost for this (and we'd probably still not even be able to implement it using the given API). There are applications such as certain painting programs that include a feature/option sometimes called "preserve transparency", which achieves exactly the effect we strife for here. But if you ever enable this option you'll see that it's never free.

@nobbele
Copy link
Member

nobbele commented Jun 2, 2021

https://user-images.githubusercontent.com/17962514/120549217-94738500-c3f3-11eb-9c32-627038f2ec3c.png
Really bad mspaint job but you are correct, the color in the canvas is slightly darker/not green.
I tried researching it but I just get GL_SRC_ALPHA and GL_ONE_MINUS_SRC_ALPHA there too.

@PSteinhaus
Copy link
Member

This is really annoying. For a moment I thought "maybe clearing newly created canvases with transparent white could solve this issue", since that would stop the darkening. But sadly that would still corrupt the image by shifting all values towards 1, thereby making it more white, or brighter if you will.

The Add and Replace blend modes both work as a workaround in certain cases since they do not corrupt the image in any such way. But these cases do not include those where you want to draw multiple things on top of each other onto a canvas. There you suddenly DO care about whether your blend mode is Add/Replace or really Alpha...

@PSteinhaus
Copy link
Member

PSteinhaus commented Jun 2, 2021

I guess the best we can really do is add a link to this issue and some explanations to the docs of BlendMode warning users of the behavior of Alpha when used in conjunction with a transparent background such as an empty Canvas.
I mean, if you spend an hour digging and scratching your head you end up realizing that the current state is actually expected behavior in a way, just one that's not yet correctly communicated.

There just aren't any ways to really fix this, are there?

@PSteinhaus
Copy link
Member

Ok, I think I've found somewhat of a solution to this: premultiplied alpha

Imagine we didn't use the classic alpha blend mode for drawing the canvas here, but instead used the premultiplied alpha blend mode described in #905.

When we draw on the canvas using the classic alpha blend mode we generate a premultiplied image. That's just the way this blend mode works. It premultiplies the color channels with the alpha channel before adding it to what's behind. We cannot get around that.

But when we now draw this canvas containing the premultiplied (i.e. "corrupted") image onto the screen using the premultiplied alpha mode we get exactly the same result we'd have gotten by drawing all the things directly to the screen using the classic alpha blend mode (that is, if you ignore the minor inconsistency in the calculcation of the alpha value between these two methods for now).

So really, this is not necessarily a bug, the image is not "corrupted", it just went from unpremultiplied, to premultiplied when drawn on the canvas.
So to allow our users to navigate this issue we need to add the premultiplied blend mode and explain how/when to use it.

Side Note

As stated by @emilk here

For translucent textures especially, premultiplied alpha is the only right choice

I think this is true. Because imagine, if a user started off with PNGs containing premultiplied images already, then there'd be no hassle for him/her at all, because he could just use the premultiplied alpha blend mode all the way, drawing on opaque backgrounds, drawing on transparent backgrounds, it doesn't matter.

Therefore I think we should create an example, or a doc page, explaining this concept and how to prepare your sprites for it.
(Actually we could even supply a helper function (as a method of Image?) that takes an unpremultiplied image and premultiplies it, for convenience, what do you think?)

@PSteinhaus
Copy link
Member

Just tested with the new devel branch with #905 merged using this code:

use ggez::conf::Conf;
use ggez::event::{self, EventHandler};
use ggez::graphics::{self, Canvas, Color, DrawParam, Image, BlendMode};
use ggez::Context;
use ggez::ContextBuilder;
use ggez::GameResult;
use ggez::graphics::Drawable;

use mint::Point2;
use ggez::graphics::BlendMode::Add;

struct Application {
    dark_image: Image,
    dark_canvas: Canvas,
    light_image: Image,
    light_canvas: Canvas,
    canvas_image: Image,
}

impl Application {
    fn new(ctx: &mut Context) -> Self {
        let (dark_image, dark_canvas) = Self::image_canvas(ctx, "/dark-image.png");
        let (light_image, light_canvas) = Self::image_canvas(ctx, "/light-image.png");

        let canvas_image = Self::canvas(ctx, &dark_image).to_image(ctx).unwrap();

        Self {
            dark_image,
            dark_canvas,
            light_image,
            light_canvas,
            canvas_image,
        }
    }

    fn image_canvas(ctx: &mut Context, file: &str) -> (Image, Canvas) {
        let mut image = Image::new(ctx, file).unwrap();
        let canvas = Self::canvas(ctx, &image);
        (image, canvas)
    }

    fn canvas(ctx: &mut Context, image: &Image) -> Canvas {
        let mut canvas = Canvas::with_window_size(ctx).unwrap();
        graphics::set_canvas(ctx, Some(&canvas));
        graphics::draw(ctx, image, DrawParam::new()).unwrap();
        graphics::present(ctx);
        graphics::set_canvas(ctx, None);
        canvas.set_blend_mode(Some(BlendMode::Premultiplied));

        canvas
    }
}

impl EventHandler for Application {
    fn update(&mut self, _: &mut Context) -> GameResult<()> {
        Ok(())
    }

    fn draw(&mut self, ctx: &mut Context) -> GameResult<()> {
        graphics::clear(
            ctx,
            Color {
                r: 0.,
                g: 0.3,
                b: 0.,
                a: 1.,
            },
        );

        graphics::draw(ctx, &self.dark_image, DrawParam::new()).unwrap();
        let point2 = Point2 { x: 0., y: 60. };
        graphics::draw(ctx, &self.dark_canvas, DrawParam::new().dest(point2)).unwrap();

        let point2 = Point2 { x: 0., y: 200. };
        graphics::draw(ctx, &self.light_image, DrawParam::new().dest(point2)).unwrap();
        let point2 = Point2 { x: 0., y: 260. };
        graphics::draw(ctx, &self.light_canvas, DrawParam::new().dest(point2)).unwrap();

        let point2 = Point2 { x: 0., y: 360. };
        graphics::draw(ctx, &self.canvas_image, DrawParam::new().dest(point2)).unwrap();

        graphics::present(ctx).unwrap();
        Ok(())
    }
}

pub fn main() {
    let c = Conf::new();
    let (mut ctx, event_loop) = ContextBuilder::new("canvas", "athorus").build().unwrap();
    let app = Application::new(&mut ctx);
    event::run(ctx, event_loop, app)
}

Now, using the premultiplied blend mode for canvases it works. Notice that the last image still looks "corrputed" since it's just the image contained in the dark_canvas, but drawn with the default blend mode, which is Alpha and not Premultiplied.

@IGBC
Copy link
Contributor

IGBC commented Jun 3, 2021

This being two years old, workaround implemented, and no input from the OP in nearly 2 years I am closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants