Fix when with pitch != width*BBP in Image Display - #1719
Merged
Conversation
Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
abaeyens
reviewed
May 25, 2026
Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
Contributor
Author
|
Pulls: #1719
|
Contributor
Author
|
https://github.com/Mergifyio backport lyrical kilted jazzy humble |
Contributor
✅ Backports have been createdDetails
Cherry-pick of 9eebc57 has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
This was referenced May 26, 2026
ahcorde
added a commit
that referenced
this pull request
May 26, 2026
ahcorde
added a commit
that referenced
this pull request
May 26, 2026
abaeyens
added a commit
to abaeyens/rviz
that referenced
this pull request
Jul 5, 2026
The previous "Bayer input is linear-light, apply sRGB gamma" rule was hardcoded. Expose it as a per-display "Linear input" BoolProperty on ImageDisplay; hidden unless the current message is Bayer; default off. Toggling on a latched frame re-runs the converter without needing a new publish (mirrors setSmoothScaling). Now that PR ros2#1719 (buffer repack in update()) has landed upstream: - Replace the inline stride-aware 16-bit min/max scan with a call to the shared getMinimalAndMaximalValueToNormalize<uint16_t>. - Delete unreachable defensive stride/alignment checks and the now-obsolete odd_stride_16bit_rejected test. Also: parameterise linear-passthrough tests over all four Bayer layouts (8 and 16-bit); add tests for fixed-max above actual, median-frames tracking, toggle round-trip on a held frame, and initialize-time propagation of the default. Fix a -Wreorder in the constructor init list. Name the sRGB piecewise constants (IEC 61966-2-1). Apply ament_uncrustify to ros_image_texture.cpp. Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
abaeyens
added a commit
to abaeyens/rviz
that referenced
this pull request
Jul 6, 2026
Resolves ros2#887. Bayer encodings (bayer_rggb8 / bayer_bggr8 / bayer_gbrg8 / bayer_grbg8 and their 16-bit variants) were previously routed to a stub that handed the raw mosaic to Ogre as PF_BYTE_L, rendering as a visible checkerboard. They are now bilinearly demosaiced to sRGB-encoded RGB. The demosaic is hand-rolled to avoid pulling in OpenCV / cv_bridge as a dependency, matching the policy established in ros-visualization/rviz#1009 and the existing YUYV / UYVY / NV12 converter pattern in this file. A single templated implementation (parameterised on the input pixel type and on the (R_DY, R_DX) position of R within the 2x2 mosaic cell) covers all four layouts at both 8-bit and 16-bit. The interior loop is bounds-check-free; four separate border passes handle the 1-pixel edge with clamp-to-edge and a count-of-valid- neighbours average. Each demosaiced linear value passes through an sRGB transfer-function LUT (256 entries at 8-bit, 65536 entries / 64 KiB at 16-bit, built once on first use). The sRGB step is always-on for Bayer because the bilinear output is linear-light and the texture is displayed as sRGB; a user-toggleable linear-input property is left for a follow-up if a real use case appears. For 16-bit Bayer, the existing Normalize Range / Min Value / Max Value / Median window properties are reused. Cameras packing 10/12/14-bit data into 16-bit messages can be displayed correctly by disabling Normalize Range and setting Max Value to the actual bit-depth maximum (1023 / 4095 / 16383). Min/max statistics are computed over the input mosaic (not the 3x-larger demosaiced output) using a stride-aware row-by-row scan that ignores padding bytes between rows; this scan and the bayer_16bit_padded_stride regression test become removable once PR ros2#1719 lands and repacks padded buffers in update() before dispatch (see in-file comment for cleanup notes in both directions). Input validation rejects malformed messages upfront with UnsupportedImageEncoding: zero or oversize dimensions (> 32768), inconsistent stride (< width * bpp), truncated data (< height * stride), odd stride for 16-bit, and misaligned data_ptr for 16-bit (the latter guards against strict-alignment platforms like ARMv7 where the uint16_t reinterpret_cast would otherwise be undefined behaviour). Unknown bayer_* encodings now throw rather than being silently rendered as luminance; a small deliberate user-visible behaviour change. In ImageDisplay, the private got_float_image_ flag is renamed to has_normalizable_range_ to reflect that it now also gates the normalize controls for 16-bit Bayer. Tooltip on Normalize Range is extended to mention Bayer applicability. Tests cover: all four 8-bit layouts (pure-red and pure-blue, which are discriminating across layouts because a layout-swap bug surfaces as swapped output channels); all four 16-bit layouts with fixed Max Value; 16-bit black-point via Min Value; running-median smoke test for the auto-normalisation path; padded-stride regression test (verified to fail under the previous contiguous min/max scan); scale-then-sRGB ordering; multi-anchor sRGB transfer-function validation across the linear toe, the knee at 0.0031308, and the upper exponent range; small-image edge cases (2x2 and 3x3); unknown-encoding rejection; and oversize / truncated / odd-stride-16-bit rejection. Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
abaeyens
added a commit
to abaeyens/rviz
that referenced
this pull request
Jul 6, 2026
The previous "Bayer input is linear-light, apply sRGB gamma" rule was hardcoded. Expose it as a per-display "Linear input" BoolProperty on ImageDisplay; hidden unless the current message is Bayer; default off. Toggling on a latched frame re-runs the converter without needing a new publish (mirrors setSmoothScaling). Now that PR ros2#1719 (buffer repack in update()) has landed upstream: - Replace the inline stride-aware 16-bit min/max scan with a call to the shared getMinimalAndMaximalValueToNormalize<uint16_t>. - Delete unreachable defensive stride/alignment checks and the now-obsolete odd_stride_16bit_rejected test. Also: parameterise linear-passthrough tests over all four Bayer layouts (8 and 16-bit); add tests for fixed-max above actual, median-frames tracking, toggle round-trip on a held frame, and initialize-time propagation of the default. Fix a -Wreorder in the constructor init list. Name the sRGB piecewise constants (IEC 61966-2-1). Apply ament_uncrustify to ros_image_texture.cpp. Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Related to #512
Did you use Generative AI?
Claude Sonnet 4.6