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

WebAssembly support #6

Closed
ibaryshnikov opened this issue Sep 9, 2019 · 4 comments
Closed

WebAssembly support #6

ibaryshnikov opened this issue Sep 9, 2019 · 4 comments

Comments

@ibaryshnikov
Copy link
Contributor

Is it a goal or non-goal?
It worked out of the box before 0.1, but not supported by inventory yet.

@Enet4
Copy link
Owner

Enet4 commented Sep 10, 2019

Thank you for reporting. WebAssembly support is indeed planned, but it did not occur to me that the flexible transfer syntax registry would break compatibility.

If inventory really can't work in a wasm environment, we'll have to look for an alternate implementation.

@ibaryshnikov
Copy link
Contributor Author

Thanks for a quick reply!

There's a related issue amethyst/amethyst#1893, but linkme support for wasm is not ready yet dtolnay/linkme#6

From the other side, it's possible to have mutable statics

lazy_static! {
    static ref REGISTRY: Mutex<TransferSyntaxRegistry> = {
        Mutex::new(TransferSyntaxRegistry { m: initialize_codecs() })
    };
}

the drawback is that it can affect performance.

One more option is to use decoder builder, keeping a local copy of transfer syntax registry per decoder

let decoder = DecoderBuilder::default()
    .with_transfer_syntax(extend it)
    .with_transfer_syntax(extend again)
    .build()?;
let object = decoder.from_reader(input)?;

What do you think? May be there is a better alternative which I missed.

@Enet4
Copy link
Owner

Enet4 commented Sep 12, 2019

That would be a similar approach to the data dictionary, which is a type parameter in many places of the implementation. So that may indeed be feasible for a scenario where we can't have an inventory of transfer syntaxes, while still keeping an intuitive default of using said inventory.

I've been busy at a conference this week, but I may be able to continue pushing some code and roadmap text soon.

@Enet4
Copy link
Owner

Enet4 commented Oct 27, 2019

Considering that #17 has been merged, we can consider this resolved. We should strive to keep WebAssembly as a working target, although there is much to be done regarding testing. Please create a new issue if another problem emerges when using DICOM-rs in WebAssembly.

@Enet4 Enet4 closed this as completed Oct 27, 2019
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

No branches or pull requests

2 participants