Introduce image data conversion pipeline, taking over existing YUV conversions #7640

Wumpf · 2024-10-08T16:10:06Z

What

Major part of Unify GPU texture conversion (with focus on chroma downsampled formats) #7608
Related to Web-view crashes in chrome on linux when shaders are too large #3931
- since it makes the rectangle shader smaller which kept acting up in the past

via pixi run -e py python ./tests/python/chroma_downsample_image/main.py:
https://rerun.io/viewer/pr/7640?url=https%3A%2F%2Fstatic.rerun.io%2Frrd%2F0.19%2Fchroma_formats_ad9697d05933f4104fbbe66af23073ad4e9e4a58.rrd

The new functionality is tightly integrated into TextureManager2D which now ingests all incoming data via a new internal method transfer_image_data_to_texture.
This in turn takes care of data upload via GpuReadCpuWriteBelt (new! previously, we would use queue.write_texture in this case) and may if needed run gpu based conversion.

Gpu based conversion is for convenience implemented like a Renderer - while it is not used like a typical Renderer, the lifecycle (create lazily once, store on context, feed with data bundles [...]) was so similar that it made sense to me to use this existing pattern even though it's not a perfect match.

A curious sideeffect of this is that you can now put chroma downsampled textures on custom meshes!

Next steps:

support formats required for AV1
- while on it, check if things could get simpler by us
move BGR handling into the new pipeline
- revisit the re_renderer sided input data description. Not sure if I'm happy about it drifting away from the user facing one!
- once we're there we're also very close to get rid of SourceImageDataFormat::WgpuCompatible. But it's not strictly necessary, this might yet prove a convenient loophole, although its presence makes me a little bit nervous (details see code comments) 🤔
consider exposing color primaries to public API
- the only thing keeping us from it is adding a new enum to the color format!
- should it be called "primaries" or "color space" 🤔

Checklist

test on web Mac Chrome/Firefox/Safari
test on web Window Chrome/Firefox
test on web Firefox Chrome/Firefox (@jleibs plz 🥺 )
I have read and agree to Contributor Guide and the Code of Conduct
I've included a screenshot or gif (if applicable)
I have tested the web demo (if applicable):
- Using examples from latest main build: rerun.io/viewer
- Using full set of examples from nightly build: rerun.io/viewer
The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
If applicable, add a new check to the release checklist!
If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

Wumpf · 2024-10-08T16:14:54Z

crates/viewer/re_renderer/shader/rectangle_fs.wgsl

-// WARNING! Adding anything else to this shader is very likely to push us over a size threshold that
-// causes the failure reported in: https://github.com/rerun-io/rerun/issues/3931
-// Make sure any changes are tested in Chrome on Linux using the Intel Mesa driver.


the expectation is that this is no longer the case now.
Is it true though? @jleibs :)

…ure conversion

…the converting shader

…decode

emilk

Beautiful! I love all the comments

LGTM if tested with NV12 and YUV on natie, WebGL and WebGPU (I'm paranoid!)

crates/viewer/re_renderer/shader/conversions/chroma_subsampling_converter.wgsl

emilk · 2024-10-08T16:36:00Z

crates/viewer/re_renderer/shader/conversions/chroma_subsampling_converter.wgsl

+
+    var rgb: vec3f;
+
+    switch (primaries) {


wat, wgsl supports switch!? NICE! That could be useful in crates/viewer/re_renderer/shader/rectangle_fs.wgsl too 😄

learned that today as well 😄. Followed a hunch there - probably I've seen it in the meanwhile somewhere.

I think it didn't back when we started. But Naga & Tint (Dawn's compiler) are happy with it so full steam ahead!

crates/viewer/re_renderer/src/renderer/rectangles.rs

emilk · 2024-10-08T16:38:47Z

crates/viewer/re_renderer/src/resource_managers/chroma_subsampling_converter.rs

+pub enum ChromaSubsamplingPixelFormat {
+    /// 4:2:0 subsampling with a separate Y plane, followed by a UV plane.
+    ///
+    /// Expects single channel texture format.
+    ///
+    /// First comes entire image in Y in one plane,
+    /// followed by a plane with interleaved lines ordered as U0, V0, U1, V1, etc.
+    ///
+    ///          width
+    ///          __________
+    ///          |         |
+    /// height   |    Y    |
+    ///          |         |
+    ///          |_________|
+    /// height/2 | U,V,U,… |
+    ///          |_________|
+    Y_UV12 = 0,
+
+    /// YUV 4:2:2 subsampling, single plane.
+    ///
+    /// Expects single channel texture format.
+    ///
+    /// The order of the channels is Y0, U0, Y1, V0, all in the same plane.
+    ///
+    ///             width * 2
+    ///        __________________
+    ///        |                 |
+    /// height | Y0, U0, Y1, V0… |
+    ///        |_________________|
+    ///
+    YUYV16 = 1,
+}


I love this 🤩

crates/viewer/re_renderer/src/resource_managers/chroma_subsampling_converter.rs

crates/viewer/re_renderer/src/resource_managers/image_data_to_texture.rs

crates/viewer/re_renderer/src/workspace_shaders.rs

crates/viewer/re_viewer_context/src/gpu_bridge/image_to_gpu.rs

emilk · 2024-10-08T16:59:40Z

crates/viewer/re_renderer/src/resource_managers/texture_manager.rs

            zeroed_texture_sint,
            zeroed_texture_uint,
-            device,
-            queue,
            inner: Default::default(),
        }
    }

    /// Creates a new 2D texture resource and schedules data upload to the GPU.
    /// TODO(jleibs): All usages of this should be replaced with `get_or_create`, which is strictly preferable


Looks like we're only using create in crates/viewer/re_renderer/src/importer/gltf.rs

and this one is surprisingly hard to get rid of because for that we'd to be sure that we get a unique enough description of the mesh without having to resort to a hash of the texture content within it.

emilk

Beautiful! I love all the comments

LGTM if tested with NV12 and YUV on natie, WebGL and WebGPU (I'm paranoid!)

…which widths and heights we're talking about

Wumpf · 2024-10-09T08:58:58Z

skipping linux web check for now. still have to do that though!

Wumpf added 🔺 re_renderer affects re_renderer itself 🚜 refactor Change the code, not the functionality include in changelog labels Oct 8, 2024

Wumpf commented Oct 8, 2024

View reviewed changes

Wumpf added 10 commits October 8, 2024 18:25

first sketch of transfer_image_data_to_texture

c205d2a

Make all texture manager operations go through new image-data-to-text…

2724b17

…ure conversion

chroma subsampling conversion setup

00e1f67

chroma subsampling converter now passes all the relevant data now to …

fe9dedd

…the converting shader

chroma conversion shader has now parity with rectangle shader chroma …

a853a2e

…decode

remove chroma subsampling shader decode from rectangle shader

deec337

cleanup

3b81ae4

typo fixes

4df5bc6

roll back unneeded changes

f3efc9d

improve commentrary on expected_data_texture_format

1c31cff

Wumpf force-pushed the andreas/flexible-and-fast-image-conversion branch from 61d287f to 1c31cff Compare October 8, 2024 16:25

emilk self-requested a review October 8, 2024 16:31

emilk approved these changes Oct 8, 2024

View reviewed changes

Wumpf added 9 commits October 9, 2024 09:30

use ERROR_RGBA

7f3c538

todo item references

5dbfca5

collapse width & height into width_height in various places, clarify …

d3552eb

…which widths and heights we're talking about

rename all things chroma subsampling

7e5963a

resolve "this"-style comments

2c54928

error improvements

88be036

more profiling scopes

97a61ef

remove unused/renamed file remnant

0099cf9

typos and doc string fixes

6b52176

Wumpf merged commit c3af105 into main Oct 9, 2024
32 of 33 checks passed

Wumpf deleted the andreas/flexible-and-fast-image-conversion branch October 9, 2024 08:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce image data conversion pipeline, taking over existing YUV conversions #7640

Introduce image data conversion pipeline, taking over existing YUV conversions #7640

Wumpf commented Oct 8, 2024 •

edited by github-actions bot

Loading

Wumpf Oct 8, 2024

emilk left a comment

emilk Oct 8, 2024

Wumpf Oct 8, 2024

emilk Oct 8, 2024

emilk Oct 8, 2024

Wumpf Oct 9, 2024

emilk left a comment

Wumpf commented Oct 9, 2024

Introduce image data conversion pipeline, taking over existing YUV conversions #7640

Introduce image data conversion pipeline, taking over existing YUV conversions #7640

Conversation

Wumpf commented Oct 8, 2024 • edited by github-actions bot Loading

What

Checklist

Wumpf Oct 8, 2024

Choose a reason for hiding this comment

emilk left a comment

Choose a reason for hiding this comment

emilk Oct 8, 2024

Choose a reason for hiding this comment

Wumpf Oct 8, 2024

Choose a reason for hiding this comment

emilk Oct 8, 2024

Choose a reason for hiding this comment

emilk Oct 8, 2024

Choose a reason for hiding this comment

Wumpf Oct 9, 2024

Choose a reason for hiding this comment

emilk left a comment

Choose a reason for hiding this comment

Wumpf commented Oct 9, 2024

Wumpf commented Oct 8, 2024 •

edited by github-actions bot

Loading