Fix reality data not being reprojected correctly when its CRS is different than iModel#9059
Conversation
|
@Mergifyio backport release/5.6.x release/5.7.x |
✅ Backports have been createdDetails
Cherry-pick of d122ee6 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
Cherry-pick of d122ee6 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 |
aruniverse
left a comment
There was a problem hiding this comment.
Nice work! Do we need to run image tests?
There was a problem hiding this comment.
Pull request overview
Fixes incorrect reprojection of reality tile geometry when the reality data’s CRS differs from the iModel’s CRS by converting the reprojection transform from root-tile coordinates into iModel coordinates before transforming collected polyfaces.
Changes:
- Conjugate
tile.reprojectionTransformbyRealityTileTree.iModelTransformwhen cloning transformed polyfaces during geometry collection. - Extend reality tile loader unit tests to cover reprojection when the tree’s
iModelTransformis non-identity. - Add a change file for
@itwin/core-frontend.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| core/frontend/src/internal/tile/RealityTileLoader.ts | Applies reprojection in the correct coordinate space by conjugating the root-tile CRS transform into iModel CRS before transforming polyfaces. |
| core/frontend/src/test/tile/RealityTile.test.ts | Adds tests around reprojection behavior when iModelTransform is non-identity and when it is identity. |
| common/changes/@itwin/core-frontend/eringram-rd-reproject-crs_2026-03-04-23-15.json | Records the change for the package. |
@eringram I think we should just to be safe. |
|
No image tests differences of note. The V4 tests had some missing reality data, but I confirmed in the logs it was related to flaky requests for the data and not due to the rendering. The same reality data looked correct in the V1 tests, and since it's reality data and not iModel geometry, the tiles are the same in both versions. (correct me if that doesn't sound right, @markschlosseratbentley ..?) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
markschlosseratbentley
left a comment
There was a problem hiding this comment.
Does a NextVersion.md entry make sense?
Added |
Thanks. LGTM. |
|
Doing some more testing of this before merging |
|
This pull request is now in conflicts. Could you fix it @eringram? 🙏 |
|
@eringram are you planning on getting this merged in today? I am waiting on this to release the next 5.6 patch release. No worries if you are not planning on merging today, just wanted to check :) |
|
@anmolshres98 Hey sorry for the delay, yes planning to merge today. I'll fix the conflicts to do that now. |
…erent than iModel (#9059) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Ben Polinsky <78756012+ben-polinsky@users.noreply.github.com> Co-authored-by: Anmol Shrestha <98850418+anmolshres98@users.noreply.github.com> (cherry picked from commit d122ee6) # Conflicts: # docs/changehistory/NextVersion.md
…erent than iModel (#9059) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Ben Polinsky <78756012+ben-polinsky@users.noreply.github.com> Co-authored-by: Anmol Shrestha <98850418+anmolshres98@users.noreply.github.com> (cherry picked from commit d122ee6) # Conflicts: # docs/changehistory/NextVersion.md
Co-authored-by: hl662 <50554904+hl662@users.noreply.github.com>
There is a bug where when using the reproject parameter of
TileTreeReference.createGeometryTreeReference, if the reality data is not in the same CRS than the iModel, the reprojection will put the geometry at the wrong place.This was because in
RealityTileLoader.loadGeometryFromStream, the geometry returned byRealityTileLoader.readGltfAndCreateGeometryis in iModel coordinates, but the reprojection transform is in the reality tree's coordinates.The fix is to take this difference into account by doing the following:
iModelTransformto "convert" polyface from iModel CRS->tile tree CRSxFormiModelTransformto convert back from tile tree CRS->iModel CRSTested with a couple different iModels with reality data in different CRS. I also added some unit tests for this.