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

Spatial heuristics are broken when when run inside a notebook #5654

Closed
jleibs opened this issue Mar 22, 2024 · 1 comment · Fixed by #5674
Closed

Spatial heuristics are broken when when run inside a notebook #5654

jleibs opened this issue Mar 22, 2024 · 1 comment · Fixed by #5674
Assignees
Labels
🪳 bug Something isn't working heuristics Space-view heuristics etc notebook Jupyter notebooks etc user-request This is a pressing issue for one of our users
Milestone

Comments

@jleibs
Copy link
Member

jleibs commented Mar 22, 2024

This is impacting at least 0.14.1

To repro, run the following in a notebook:

import rerun as rr
import numpy as np

rr.init("rerun_example_images")

image1 = np.zeros((200, 300, 3), dtype=np.uint8)
image1[:, :, 0] = 255

image2 = np.zeros((200, 300, 3), dtype=np.uint8)
image2[:, :, 1] = 255

image3 = np.zeros((400, 500, 3), dtype=np.uint8)
image3[:, :, 2] = 255

rec = rr.memory_recording()

rr.log("vrs/a", rr.Image(image1))
rr.log("vrs/b", rr.Image(image2))
rr.log("vrs/c", rr.Image(image3))

rec

See that the heuristics incorrectly stack the image:
image

Whereas the expected behavior is to have them split
image

Theory

Based on initial investigation my best through is the SpatialTopology store subscriber is getting confused when used with stream_rrd_from_event_listener, which is only used for injecting data into the viewer from notebooks.

I strongly suspect it's missing messages. Possibly related to store-subscriber initialization timing?

@jleibs jleibs added 🪳 bug Something isn't working notebook Jupyter notebooks etc user-request This is a pressing issue for one of our users heuristics Space-view heuristics etc labels Mar 22, 2024
@jleibs jleibs added this to the 0.15 milestone Mar 22, 2024
@jleibs
Copy link
Member Author

jleibs commented Mar 22, 2024

Confirming my suspicions... I monkey-patched the html-renderer to inject a dummy RRD prior to the "real" RRD. This appears to be enough to "prime the pump" so the the StoreSubscriber does the right thing when it is subsequently injected.

dummy = rr.new_recording(application_id="dummy").memory_recording()

def patched_as_html(
    self,
    *,
    width: int = rr.recording.DEFAULT_WIDTH,
    height: int = rr.recording.DEFAULT_HEIGHT,
    app_url: str | None = None,
    timeout_ms: int = rr.recording.DEFAULT_TIMEOUT,
    other: rr.recording.MemoryRecording | None = None,
) -> str:
    from rerun import bindings
    import base64
    import logging
    import random
    import string

    if app_url is None:
        app_url = bindings.get_app_url()

    # Use a random presentation ID to avoid collisions when multiple recordings are shown in the same notebook.
    presentation_id = "".join(random.choice(string.ascii_letters) for i in range(6))

    base64_dummy = base64.b64encode(dummy.storage.concat_as_bytes(None)).decode("utf-8")
    
    if other:
        other = other.storage
    base64_data = base64.b64encode(self.storage.concat_as_bytes(other)).decode("utf-8")

    html_template = f"""
    <div id="{presentation_id}_rrd" style="display: none;" data-rrd="{base64_data}"></div>
    <div id="{presentation_id}_dummy" style="display: none;" data-dummy="{base64_dummy}"></div>
    <div id="{presentation_id}_error" style="display: none;"><p>Timed out waiting for {app_url} to load.</p>
    <p>Consider using <code>rr.start_web_viewer_server()</code></p></div>
    <script>
        {presentation_id}_timeout = setTimeout(() => {{
            document.getElementById("{presentation_id}_error").style.display = 'block';
        }}, {timeout_ms});

        window.addEventListener("message", function(abuffs) {{
            return async function {presentation_id}_onIframeReady(event) {{
                var iframe = document.getElementById("{presentation_id}_iframe");
                if (event.source === iframe.contentWindow) {{
                    clearTimeout({presentation_id}_timeout);
                    document.getElementById("{presentation_id}_error").style.display = 'none';
                    iframe.style.display = 'inline';
                    window.removeEventListener("message", {presentation_id}_onIframeReady);

                    let buffs = await abuffs;
                    iframe.contentWindow.postMessage(buffs[0], "*");
                    await new Promise(r => setTimeout(r, 200));
                    iframe.contentWindow.postMessage(buffs[1], "*");
                }}
            }}
        }}(async function() {{
            await new Promise(r => setTimeout(r, 0));
            
            var dummy_div = document.getElementById("{presentation_id}_dummy");
            var dummy_base64Data = dummy_div.dataset.dummy;
            var dummy_intermediate = atob(dummy_base64Data);
            var dummy_buff = new Uint8Array(dummy_intermediate.length);
            for (var i = 0; i < dummy_intermediate.length; i++) {{
                dummy_buff[i] = dummy_intermediate.charCodeAt(i);
            }}
            
            var div = document.getElementById("{presentation_id}_rrd");
            var base64Data = div.dataset.rrd;
            var intermediate = atob(base64Data);
            var buff = new Uint8Array(intermediate.length);
            for (var i = 0; i < intermediate.length; i++) {{
                buff[i] = intermediate.charCodeAt(i);
            }}
            return [dummy_buff, buff];
        }}()));
    </script>
    <iframe id="{presentation_id}_iframe" width="{width}" height="{height}"
        src="{app_url}?url=web_event://&persist=0&notebook=1"
        frameborder="0" style="display: none;" allowfullscreen=""></iframe>
    """

    return html_template

rr.MemoryRecording.as_html = patched_as_html

@Wumpf Wumpf self-assigned this Mar 25, 2024
Wumpf added a commit that referenced this issue Mar 26, 2024
### What

* Fixes #5654

could in theory affect also non-notebooks, but practically in only
manifested there.
Fixes a few typos I encountered on the way and added a new pixi command
for building the rerun webviewer wasm.


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5674/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5674/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5674/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/5674)
- [Docs
preview](https://rerun.io/preview/618e0091e723020b96d06445d36c51c14a9067e3/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/618e0091e723020b96d06445d36c51c14a9067e3/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working heuristics Space-view heuristics etc notebook Jupyter notebooks etc user-request This is a pressing issue for one of our users
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants