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

Feature request: more control about the pixel alignment (?) of draw_image() #719

Closed
yutannihilation opened this issue Oct 19, 2024 · 6 comments · Fixed by #722
Closed

Feature request: more control about the pixel alignment (?) of draw_image() #719

yutannihilation opened this issue Oct 19, 2024 · 6 comments · Fixed by #722

Comments

@yutannihilation
Copy link
Contributor

yutannihilation commented Oct 19, 2024

(I'm not very familiar with graphics things, so excuse me if I'm missing some point)

Currently, drawing these 4 pixels by draw_image() gives the following result.
I'm not familiar with graphics things, so I'm not sure what's the correct behavior, but, in my usage, I prefer

  1. sharp edge
  2. the color is drawn at the center of each pixel instead of the top left corner

(cf. yutannihilation/vellogd-r#14 (comment) is the expected result)

Regarding the first request, I think this is what I should handle by adding extra pixels at the left edge and at the bottom edge and then cut.

For the second one, does it make sense to introduce some option to Scene::draw_image()? Probably, adding kurbo::Affine::translate((0.5, 0.5)) to brush_transform of Scene::fill().

    let mut blob: Vec<u8> = Vec::new();
    [Color::RED, Color::BLUE, Color::BLUE, Color::BLUE]
        .iter()
        .for_each(|c| {
            let b = c.to_premul_u32().to_ne_bytes();
            blob.push(b[3]);
            blob.push(b[2]);
            blob.push(b[1]);
            blob.push(b[0]);
        });

    let data = vello::peniko::Blob::new(Arc::new(blob));
    let image = vello::peniko::Image::new(data, vello::peniko::Format::Rgba8, 2, 2);
    scene.draw_image(
        &image,
        Affine::scale(300.0).then_translate((100.0, 100.0).into()),
    );

image

@DJMcNab
Copy link
Member

DJMcNab commented Oct 21, 2024

cc @dfrg.

The relevant code is here:

let my_xy = vec2(xy.x + f32(i), xy.y);
let atlas_uv = image.matrx.xy * my_xy.x + image.matrx.zw * my_xy.y + image.xlat + image.atlas_offset;
// This currently clips to the image bounds. TODO: extend modes
if all(atlas_uv < atlas_extents) && area[i] != 0.0 {
let uv_quad = vec4(max(floor(atlas_uv), image.atlas_offset), min(ceil(atlas_uv), atlas_extents));
let uv_frac = fract(atlas_uv);
let a = premul_alpha(textureLoad(image_atlas, vec2<i32>(uv_quad.xy), 0));
let b = premul_alpha(textureLoad(image_atlas, vec2<i32>(uv_quad.xw), 0));
let c = premul_alpha(textureLoad(image_atlas, vec2<i32>(uv_quad.zy), 0));
let d = premul_alpha(textureLoad(image_atlas, vec2<i32>(uv_quad.zw), 0));
let fg_rgba = mix(mix(a, b, uv_frac.y), mix(c, d, uv_frac.y), uv_frac.x);
let fg_i = fg_rgba * area[i];
rgba[i] = rgba[i] * (1.0 - fg_i.a) + fg_i;

Your provided image suggests that the calculations are incorrect; the alpha bleed on the bottom right is really concerning. One thing to try would be subtracting (1, 1) from atlas_extents; I think that would probably fix the transparency section.

I'm not sure what the expected behaviour for pixel position is.

@yutannihilation
Copy link
Contributor Author

Thanks for investigating!

I'm not sure what the expected behaviour for pixel position is.

I too am not sure. I guess this matters less when the number of pixels is large, so this might not be very important.

@dfrg
Copy link
Collaborator

dfrg commented Oct 23, 2024

Thanks for this test case that perfectly identifies deficiencies in the current code. Both clamping the extents and sampling at pixel center seem reasonable to me.

I’m not sure when I’ll have time to work on this but I’ve added it to my list.

dfrg added a commit that referenced this issue Oct 23, 2024
Constrains image sampling to the actual atlas region for a given image and adds a half pixel adjustment to sample from the pixel center.

Addresses #719 and maybe #656
@dfrg
Copy link
Collaborator

dfrg commented Oct 23, 2024

Needed a bit of a break from other things so I took a swing at this. Please test out #722 and see if this provides the expected rendering.

@yutannihilation
Copy link
Contributor Author

Wow, thanks for the fix! I confirmed it gives the expected result perfectly (yutannihilation/vellogd-r#18).

github-merge-queue bot pushed a commit that referenced this issue Nov 8, 2024
#722)

Constrains image sampling to the actual atlas region for a given image
and adds a half pixel adjustment to sample from the pixel center.

Addresses #719 and maybe #656

Example 2x2 image with red, blue, cyan and magenta pixels:

<img width="387" alt="vello_image_sampling"
src="https://github.com/user-attachments/assets/6bc02607-e9fa-4c79-83c9-241e76a42fb0">

---------

Co-authored-by: Daniel McNab <[email protected]>
@yutannihilation
Copy link
Contributor Author

Thanks for fixing!

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

Successfully merging a pull request may close this issue.

3 participants