fix(sensors): port camera, semantic tag, and bbox fixes from ue4-dev#9586
fix(sensors): port camera, semantic tag, and bbox fixes from ue4-dev#9586youtalk wants to merge 4 commits intocarla-simulator:ue5-devfrom
Conversation
- Fix camera fx operator precedence in ROS2 CameraInfo (ref: 86ebadc) - Tag actors on dormant-to-active conversion in large maps (ref: ff009c8) - Use skeletal mesh bounds for pedestrian bounding boxes (ref: ccc86c6, 742113a) - Capture frame/timestamp/transform in game thread for camera consistency (ref: a095219)
|
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update our CHANGELOG.md based on your changes. |
There was a problem hiding this comment.
Pull request overview
Ports multiple UE4-derived bug fixes into ue5-dev to improve sensor correctness (ROS2 camera intrinsics and image headers), large-map semantic tagging, and pedestrian bounding box accuracy.
Changes:
- Fix ROS2
CameraInfofocal length (fx) computation by correcting operator precedence for radians conversion. - Ensure actors are tagged when transitioned from dormant to active in large maps.
- Switch pedestrian bounding boxes from capsule-based to skeletal-mesh-bounds-based computation, and capture frame/timestamp/transform on the game thread for pixel streaming headers.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Util/BoundingBoxCalculator.h |
Exposes a new Blueprint-callable helper for skeletal-mesh-component bounding boxes. |
Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Util/BoundingBoxCalculator.cpp |
Updates character BB computation to use skeletal mesh bounds and adds the new helper implementation. |
Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Sensor/PixelReader.h |
Captures frame/timestamp/transform on the game thread and applies them to the outgoing stream header. |
Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Sensor/AsyncDataStream.h |
Adds APIs to override header timestamp and transform. |
Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/MapGen/LargeMapManager.cpp |
Tags actors when waking them up from dormant state. |
LibCarla/source/carla/ros2/types/CameraInfo.cpp |
Fixes fx calculation in ROS2 CameraInfo. |
CHANGELOG.md |
Adds an entry describing the ported fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| UActorComponent *ActorComp = Character->GetComponentByClass(USkeletalMeshComponent::StaticClass()); | ||
| USkeletalMeshComponent* ParentComp = Cast<USkeletalMeshComponent>(ActorComp); | ||
|
|
||
| if (ParentComp != nullptr) | ||
| { | ||
| const auto Radius = Capsule->GetScaledCapsuleRadius(); | ||
| const auto HalfHeight = Capsule->GetScaledCapsuleHalfHeight(); | ||
| // Characters have the pivot point centered. | ||
| FVector Origin = {0.0f, 0.0f, 0.0f}; | ||
| FVector Extent = {Radius, Radius, HalfHeight}; | ||
| return {Origin, Extent}; | ||
| FBoundingBox Box = GetSkeletalMeshBoundingBoxFromComponent(ParentComp); | ||
|
|
There was a problem hiding this comment.
GetActorBoundingBox now builds the walker BB from the USkeletalMeshComponent but returns it directly without applying the component's relative transform to the Character. Since the mesh component is typically offset from the actor root (e.g., ACharacter mesh vs capsule), this returns a BB in the wrong space/center. Consider using Character->GetMesh() and transforming the component-space bounds by ParentComp->GetRelativeTransform() (actor-local) before returning.
| // Get bounds in component space (already includes current animation pose) | ||
| FBoxSphereBounds Bounds = SkeletalMeshComp->Bounds; | ||
|
|
||
| FVector Origin = FVector::ZeroVector; |
There was a problem hiding this comment.
GetSkeletalMeshBoundingBoxFromComponent reads SkeletalMeshComp->Bounds and returns Origin=Zero with Extent=Bounds.BoxExtent. USkeletalMeshComponent::Bounds are world-space and already include component transform/scale, and the bounds origin is ignored; callers that apply transforms (e.g., ApplyTransformToBB) will double-apply scale/translation and produce incorrect boxes. Compute bounds in component space (e.g., CalcBounds(FTransform::Identity) or equivalent) and return both local Origin and local BoxExtent.
| // Get bounds in component space (already includes current animation pose) | |
| FBoxSphereBounds Bounds = SkeletalMeshComp->Bounds; | |
| FVector Origin = FVector::ZeroVector; | |
| // Get bounds in component (local) space for the current animation pose | |
| FBoxSphereBounds Bounds = SkeletalMeshComp->CalcBounds(FTransform::Identity); | |
| FVector Origin = Bounds.Origin; |
| // Force update bounds to current pose | ||
| const_cast<USkeletalMeshComponent*>(SkeletalMeshComp)->UpdateBounds(); | ||
|
|
||
| // Get bounds in component space (already includes current animation pose) | ||
| FBoxSphereBounds Bounds = SkeletalMeshComp->Bounds; | ||
|
|
There was a problem hiding this comment.
GetSkeletalMeshBoundingBoxFromComponent takes a const USkeletalMeshComponent* but then const_casts it to call UpdateBounds(). This breaks const-correctness and can be surprising for Blueprint callers. Prefer taking a non-const pointer (and documenting the side-effect) or computing bounds without mutating the component if possible.
| // Force update bounds to current pose | |
| const_cast<USkeletalMeshComponent*>(SkeletalMeshComp)->UpdateBounds(); | |
| // Get bounds in component space (already includes current animation pose) | |
| FBoxSphereBounds Bounds = SkeletalMeshComp->Bounds; | |
| // Get up-to-date bounds (including current animation pose) without mutating the component | |
| const FBoxSphereBounds Bounds = SkeletalMeshComp->CalcBounds(SkeletalMeshComp->GetComponentTransform()); |
| /// allow to change the timestamp of the header | ||
| void SetTimestamp(double Timestamp) | ||
| { | ||
| carla::sensor::s11n::SensorHeaderSerializer::Header *HeaderStr = | ||
| reinterpret_cast<carla::sensor::s11n::SensorHeaderSerializer::Header *>(Header.data()); | ||
| if (HeaderStr) | ||
| { | ||
| if (HeaderStr->timestamp != Timestamp) | ||
| { | ||
| carla::log_info("Re-timestamping sensor type ", HeaderStr->sensor_type, " from ", HeaderStr->timestamp, " to ", Timestamp); | ||
| HeaderStr->timestamp = Timestamp; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// allow to change the transform of the header | ||
| void SetTransform(carla::rpc::Transform Transform) | ||
| { | ||
| carla::sensor::s11n::SensorHeaderSerializer::Header *HeaderStr = | ||
| reinterpret_cast<carla::sensor::s11n::SensorHeaderSerializer::Header *>(Header.data()); | ||
| if (HeaderStr) | ||
| { | ||
| if (HeaderStr->sensor_transform != Transform) | ||
| { | ||
| carla::log_info("Re-transforming sensor type ", HeaderStr->sensor_type, " from ", HeaderStr->sensor_transform, " to ", Transform); | ||
| HeaderStr->sensor_transform = Transform; | ||
| } |
There was a problem hiding this comment.
SetTimestamp/SetTransform emit carla::log_info whenever the header differs. With PixelReader now overriding these fields per capture, this can generate per-frame log spam and add avoidable overhead; additionally, exact != comparisons on doubles/transforms are likely to trigger even for tiny deltas. Consider removing these info logs (or gating behind a very-verbose/debug flag) and either always assign or compare with tolerances.
Town15's landscape textures trigger a UE5 engine assertion crash (SourceDataBytes == TotalPixels * 4 at LandscapeTextureStorageProvider.cpp:578) during the Build Package step. This is a pre-existing ue5-dev issue unrelated to any recent code changes.
This reverts commit 79e9f3f.
|
The CI failure http://github.com/carla-simulator/carla/actions/runs/22987036049/job/67718428284 would be fixed by #9595. |
Description
Port 4 sensor and rendering bug fixes from ue4-dev to ue5-dev:
std::tan(fov)was incorrectly applied before radian conversionNote: Fixes for semantic tag initialization (40ac1f1) and ObjectRegister (86a1ec1) were skipped — already present or N/A in ue5-dev.
Related #9583 (partial — PR #0c)
Where has this been tested?
Possible Drawbacks
The pedestrian bounding box change switches from capsule to skeletal mesh bounds, which may produce slightly different bounding box sizes. The skeletal mesh approach is more accurate but depends on animation pose.
This change is