iTwin / itwinjs-core

Monorepo for iTwin.js Library
https://www.itwinjs.org
MIT License
620 stars 210 forks source link

Add Undefined mercatorOrigin Check to computeMercatorFractionToDb #7269

Closed andremig-bentley closed 3 weeks ago

andremig-bentley commented 1 month ago

This PR is to address this issue: https://github.com/iTwin/itwinjs-backlog/issues/1243

Currently, there is an edge case where if new BackgroundMapGeometry is created with project extents centered at (0,0,0) and an ecefLocation.origin of (0,0,0), the creation of the BackgroundMapGeometry will fail. When the point (0,0,0) is converted from ECEF to a Cartographic using Cartographic.fromEcef, the function will return undefined and cause an error to occur on the creation of BackgroundMapGeometry in the above scenario.

To fix this, ecefToPixelFraction has been changed to now also return undefined if fromEcef returns undefined, and a check has been added to computeMercatorFractionToDb to return an identity transform if mercatorOrigin, mercatorX, or mercatorY is undefined due to the change in ecefToPixelFraction. A test has also been added to BackgroundMapGeometry.test.ts which tests the scenario where the project extents are centered at (0,0,0) and the ecefLocation.origin is (0,0,0) and confirms the succesful creation of BackgroundMapGeometry.

pmconne commented 1 month ago

Did you do the following?

  1. Obtain or create iModel exhibiting the bug
  2. Confirm that without your fix, opening the iModel in display-test-app with map display enabled produces the same error .
  3. Repeat 2 with your fix.
  4. Confirm map displays correctly as you navigate the view.
andremig-bentley commented 3 weeks ago

Did you do the following?

I was able to get the model initially causing the associated bug. Without these changes, the error in the screenshot below appeared when trying to open the model in DTA. With these changes, the model opened and the background map behaved as expected when navigating through the view.

dtaError

aruniverse commented 1 week ago

@mergifyio backport release/4.9.x

mergify[bot] commented 1 week ago

backport release/4.9.x

✅ Backports have been created

* [#7328 Add Undefined mercatorOrigin Check to computeMercatorFractionToDb (backport #7269) [release/4.9.x]](https://github.com/iTwin/itwinjs-core/pull/7328) has been created for branch `release/4.9.x`