Amjad50 / ul-next

Rust bindings for Ultralight: Lightweight, high-performance HTML renderer
Other
6 stars 1 forks source link

Glium example panics on texture creation #6

Open kanerogers opened 1 month ago

kanerogers commented 1 month ago

Hey there!

When I try running the glium example, I get the following panic:

thread 'main' panicked at /home/kane/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glium-0.34.0/src/texture/any.rs:156:13:
Texture data size mismatch
stack backtrace:
   0: rust_begin_unwind
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/panicking.rs:74:14
   2: glium::texture::any::new_texture
             at /home/kane/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glium-0.34.0/src/texture/any.rs:156:13
   3: glium::texture::textures::srgb_texture2d::SrgbTexture2d::new_impl
             at ./target/debug/build/glium-4e81025d719b59c5/out/textures.rs:4171:99
   4: glium::texture::textures::srgb_texture2d::SrgbTexture2d::with_format
             at ./target/debug/build/glium-4e81025d719b59c5/out/textures.rs:4155:21
   5: ul_next::gpu_driver::glium::GliumGpuDriverReceiver::create_texture
             at ./src/gpu_driver/glium.rs:547:21
   6: ul_next::gpu_driver::glium::GliumGpuDriverReceiver::render
             at ./src/gpu_driver/glium.rs:584:29
   7: glium_custom_gpu_driver::main::{{closure}}
             at ./examples/glium_custom_gpu_driver.rs:129:9
   8: glium_custom_gpu_driver::main
             at ./examples/glium_custom_gpu_driver.rs:153:5
   9: core::ops::function::FnOnce::call_once
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

I'm not much of an OpenGL person, but taking a quick look at the OpenGL implementation of GPUDriver, it looks like the bitmaps coming from Ultralight are 1 byte aligned; which is a little odd (and undocumented!):

https://github.com/ultralight-ux/AppCore/blob/5e9d9e2c8c288e2890635c76aac42f91c02968c3/src/linux/gl/GPUDriverGL.cpp#L210

Amjad50 commented 1 month ago

That's not the issue with the 1 byte alignment, I found the issue. Bug from my driver.

The Bitmap "may" have the rows aligned to a specific length. For example, the part where it crashes the image is 111x111 pixels, but each row is 448 instead of 444. When creating the image using glium we don't deal with these paddings after each row and thus it crashes.

I tried fixing it manually and it works

let expected_row_bytes = bitmap.width() * 4;
let got_row_bytes = bitmap.row_bytes();
let data = if expected_row_bytes != got_row_bytes {
    // there is some padding after each row, fix it manually
    let mut new_pixels = Vec::with_capacity(
        bitmap.height() as usize * expected_row_bytes as usize,
    );
    for row in bitmap_pixels.chunks(got_row_bytes as usize) {
        new_pixels.extend_from_slice(&row[..expected_row_bytes as usize]);
    }
    Cow::Owned(new_pixels)
} else {
    Cow::Borrowed(bitmap_pixels)
};

I'll see what glium has to offer for this.

Thanks for reporting this, probably some regression after some update, but it could have happened at any point, the Bitmap API hasn't changed.

Amjad50 commented 1 month ago

We need to use UNPACK_ROW_LENGTH parameter when creating the texture (check the code to ultralight gpudriver link above) and this will tell opengl to use the correct length, but its not supported by glium at the moment. I'll open an issue

I'll probably add the above fix as a hotfix and wait for glium to fix it or implement it ourselves later on

kanerogers commented 1 month ago

Nice pickup @Amjad50! I'm not too familiar with glium, but is there a way you could grab the global OpenGL context and make a raw OpenGL call with it instead? ie. does glium have an "escape hatch" mechanism you could use?

Amjad50 commented 1 month ago

Interesting idea.

We can use get_proc_addr to get the address of the function we want (glPixelStorei): https://docs.rs/glium/latest/glium/backend/trait.Backend.html#tymethod.get_proc_address.

And then use the enum value of UNPACK_ROW_LENGTH, one of the annoyances is that glium doesn't expose the bindings publicly or use something like glium_sys, its private to the crate.

Using raw number value for UNPACK_ROW_LENGTH could work, since this mostly won't change ever, not nice, but we could do this temporarily

kanerogers commented 1 month ago

Nice find!! Yeah, that seems like a good short term solution.

Amjad50 commented 1 month ago

Got a bit busy last week. But ...

Update, after looking a bit around, I didn't notice this but its not easy to call gl functions that modify context used by glium, since glium manages its own textures, so at least for now using opengl functions directly needs manual management for textures and maybe other stuff as well.

I'll quickly just use the manual pixels copy for now and create glium issue today for future fix