aws / dcv-color-primitives

DCV Color Primitives Library
MIT No Attribution
30 stars 22 forks source link

Webassembly Support #35

Closed Urhengulas closed 4 years ago

Urhengulas commented 4 years ago

Problem

Currently this crate doesn't support webassmbly (--target wasm32-unknown-unknown).

Error Message ```bash $ cargo build --target wasm32-unknown-unknown Compiling dcv-color-primitives v0.1.11 (/home/urhengulas/Documents/github.com/aws/dcv-color-primitives) error[E0425]: cannot find function `_bswap64` in this scope --> src/convert_image/x86.rs:1079:62 | 1079 | *(obuffer.add(obuffer_offset) as *mut i64) = _bswap64( | ^^^^^^^^ not found in this scope error[E0425]: cannot find function `_bswap64` in this scope --> src/convert_image/x86.rs:1085:66 | 1085 | *(obuffer.add(obuffer_offset + 8) as *mut i64) = _bswap64( | ^^^^^^^^ not found in this scope error[E0425]: cannot find function `_bswap64` in this scope --> src/convert_image/x86.rs:1092:67 | 1092 | *(obuffer.add(obuffer_offset + 16) as *mut i64) = _bswap64( | ^^^^^^^^ not found in this scope error[E0425]: cannot find function `_bswap64` in this scope --> src/convert_image/x86.rs:1099:67 | 1099 | *(obuffer.add(obuffer_offset + 24) as *mut i64) = _bswap64( | ^^^^^^^^ not found in this scope error[E0425]: cannot find function `_bswap` in this scope --> src/convert_image/x86.rs:1113:62 | 1113 | *(obuffer.add(obuffer_offset) as *mut i32) = _bswap( | ^^^^^^ not found in this scope error: aborting due to 5 previous errors For more information about this error, try `rustc --explain E0425`. error: could not compile `dcv-color-primitives`. To learn more, run the command again with --verbose. ```

Why is this desired?

I want to use this crate for an API build with wasm-bindgen, to convert rgba data from images to ColorSpace::Bt601, PixelFormat::I444.

What is missing to support webassembly?

As far as I can see, only this is needed:


I am happy to try this, but I'd probably need some input/mentoring, since I am not too experienced with low-level bit calculation.

@fabiosky

fabiosky commented 4 years ago

Hi @Urhengulas,

thanks for your interest in.

For the sake of progressing your test, you can simply use the path for aarch64, which has the replacement for the missing intrinsics.

How to do

  1. Remove the intrinsic import: Remove the following lines:

  2. Use always the dcp intrinsic emulation functions: Remove the following lines:

Issue analysis

For the point of view of the issue, I suspect that wasm comes without support for such intrinsics. It might require some time to test the given configuration but I think that a reasonable patch would be to replace https://github.com/aws/dcv-color-primitives/blob/master/src/convert_image/x86.rs#L26

with:

#[cfg(any(target_arch = "aarch64", target_arch = "wasm"))]

and: https://github.com/aws/dcv-color-primitives/blob/master/src/convert_image/x86.rs#L46

with:

#[cfg(any(target_arch = "x86", target_arch = "aarch64", target_arch = "wasm"))]

@Urhengulas I suggest you to try the first solution at least to check if the library compiles and runs properly; If the solution works, then you can definitely try the second (most general) solution.

In the meantime we will try to set up the given environment and reproduce the issue.

Urhengulas commented 4 years ago

Thank you @fabiosky for your fast answer :smile:

I added an implementation for _baswap(..), _bswap64(...) to be used on target_arch = "wasm32":


#[cfg(target_arch = "wasm32")]
fn _bswap(x: i32) -> i32 {
    x.swap_bytes()
}

#[cfg(target_arch = "wasm32")]
fn _bswap64(x: i64) -> i64 {
    x.swap_bytes()
}

(commit: https://github.com/Urhengulas/dcv-color-primitives/commit/28a42140b4c0669778e79690a6764a4ed8b2ec83)


With that patch the library builds for wasm32-unknown-unknown, but I don't get the tests running, so I can't verify if it works correctly.

I will try out if it generally works in my application.

fabiosky commented 4 years ago

Hi @Urhengulas,

I was able to reproduce the given configuration and I provided a patch for this: https://github.com/aws/dcv-color-primitives/pull/36

With this patch the library builds fine for wasm32 target:

cargo check --target wasm32-unknown-unknown

And it is also possible to execute the tests, through wasm-pack:

wasm-pack test --node

While the patch pass through the review/approval process, you can try the feature branch wip/wasm that contains the required support.

Thanks, Fabio

Urhengulas commented 4 years ago

Just checked it out. Patch and the tests work well 👍


On a sidenote:

I didn't know this trick, but it seems very useful:

use wasm_bindgen_test::wasm_bindgen_test as test;
fabiosky commented 4 years ago

Here's the new release that comes with wasm32 support: https://github.com/aws/dcv-color-primitives/releases/tag/v0.1.14

Crates link: https://crates.io/crates/dcv-color-primitives

fabiosky commented 4 years ago

PR https://github.com/aws/dcv-color-primitives/pull/36 closed.

wasm32 support was added to release https://github.com/aws/dcv-color-primitives/releases/tag/v0.1.14

Thanks @Urhengulas for your constructive feedback. Hope you are enjoying this library.

If there are no other issues regarding this topic, I'd close this one.

Thanks, Fabio

Urhengulas commented 4 years ago

Thank you very much :)