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

Drawing on the web is very slow #207

Open
GDX64 opened this issue Mar 29, 2024 · 4 comments
Open

Drawing on the web is very slow #207

GDX64 opened this issue Mar 29, 2024 · 4 comments
Labels

Comments

@GDX64
Copy link

GDX64 commented Mar 29, 2024

It looks like drawing with wasm is way slower than it needs to be. I'm using tiny-skia and it takes 14ms just to present a 1000x1000 picture:

image

doing this I could draw way faster:

#[cfg(target_arch = "wasm32")]
fn draw_buffer_web(win: &Window, pixmap: &Pixmap) {
    use wasm_bindgen::prelude::*;
    web_sys::console::log_1(&"draw_buffer_web".into());
    let canvas = get_a_canvas(win);
    let ctx: web_sys::CanvasRenderingContext2d = canvas
        .get_context("2d")
        .expect("Failed to get 2d context")
        .expect("Failed to get 2d context")
        .dyn_into()
        .expect("Failed to convert to CanvasRenderingContext2d");

    let width = pixmap.width();
    let clamped = wasm_bindgen::Clamped(pixmap.data());
    let image = web_sys::ImageData::new_with_u8_clamped_array(clamped, width)
        .expect("Failed to create image data");
    ctx.put_image_data(&image, 0.0, 0.0)
        .expect("Failed to put image data");

    fn get_a_canvas(win: &Window) -> web_sys::HtmlCanvasElement {
        use winit::platform::web::WindowExtWebSys;
        win.canvas().expect("Failed to get canvas")
    }
}

image

It looks like the current present_with_damage function iterates throught the whole buffer and makes a copy each time. Wouldn't it be possible to just take in some buffer and present it with no copies or individual byte manipulation?

@ids1024
Copy link
Member

ids1024 commented Mar 29, 2024

I assume this is a release build?

One problem with the web backend is that softbuffer uses BGRX color, which is converted to RGBA in the web backend. Softbuffer needs a format API to be able to use a different pixel format without conversion (#98). Though any application that only supports rendering to one format will need conversion on some platforms, if there's not one format that's supported everywhere.

There may be other ways to improve the web backend, but its the backend I'm personally least familiar with.

@GDX64
Copy link
Author

GDX64 commented Mar 29, 2024

Yes, it is a release build. Hum. I had not noticed that it was using BGRX, I thought it was just using a u32 instead of 4 bytes of RGBA. It is wierd that its taking so long, even to iterate 1M values should not be that bad. I will see if I can find a way to improve it a bit.

@GDX64
Copy link
Author

GDX64 commented Mar 29, 2024

Just took off all the skips and takes on the iterators and it ran in 1.5ms the present function. Maybe they are preventing some compiler optimizations since it gets harder to know the final buffer size. I don't really know what are those damages for, but I think that in my special case it is just the size of the whole window so I don't need them. I dont know if there is a better general case, but maybe you could add a check to see if there is just one damage and it is the size of the view so we do just:

// Create a bitmap from the buffer.
        let bitmap: Vec<u8> = self
            .buffer
            .iter()
            .flat_map(|&pixel| [(pixel >> 16) as u8, (pixel >> 8) as u8, pixel as u8, 255])
            .collect();

It wont be as fast as the other aproach I said above, because in the other case we dont need to iterate on anything, but maybe it is the case that most people will find anyways.

@daxpedda
Copy link
Member

daxpedda commented Apr 1, 2024

If I remember correctly, the reason why we do this in the first place is because we want to avoid converting pixels we won't draw. If conversion wouldn't be necessary, we can remove the whole thing here not even needing union_damage() anymore.

So I would rather wait for #98 before doing any major refactoring here, but I'm happy to look at a PR improving the status quo.

@madsmtm madsmtm added the Web label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants