search

iTwin / itwinjs-core

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

Reality data should cast shadows #3946

Closed markschlosseratbentley closed 2 years ago

markschlosseratbentley commented 2 years ago

This sample (https://www.itwinjs.org/sandboxes/iTwinPlatform/Shadow%20Study) was recently cloned and made to use reality data instead of an iModel. The reality data does not cast shadows on the map. We believe it should be casting shadows on the map.

In addition to the map receiving shadows from reality data, should reality data receive shadows itself?

pmconne commented 2 years ago

Additional findings: TileTreeReference has a castsShadows property, which is true for RealityTileTreeReference. Open TPB maps or Exton campus (same model, different names) in display-test-app and turn on shadows and background map. Adjust time so that you can see a shadow cast onto the map by the building model. Key in fdt set building display to enable OpenStreetMaps Buildings reality models. Observe: the shadows cast by the building model vanishes. Expect: both the building and the OSM building should cast shadows onto the map. (Adjust model height to align model to map or vice-versa -- no effect).

Repeat keyin to disable OSM buildings. Shadow reappears. Enable terrain. Observe: shadow vanishes. Expect: shadows should be cast on terrain (valid expectation?)

Sign-in is broken, at least on Linux (claims port 3000 is already in use when attempting to use dta signin keyin), so I have not tested with reality models from reality data service.

markschlosseratbentley commented 2 years ago

7/29: Shadows should be working. Need to figure out where problem is. (Could have been one particular PR that reworked reality stuff). Went back as far as May 2020 and it was not working.

pmconne commented 2 years ago

The issue with OSM buildings may be unrelated. The OSM buildings span the globe. SolarShadowMap.ts calls TileTreeReference.accumulateTransformedRange which for OSM produces an enormous range.

pmconne commented 2 years ago

@DStradley tracked down a commit at which the shadows definitely worked, and another commit (from an addon merge) at which they definitely do not work: Works: d40e0a9812b6f7d48a65438ba6338f91d948ecdc 2020-11-09T23:04:03-06:00 imodeljs-admin: 2.9.0-dev.16 Fails: ed98bb0efc14287a1da805157e0aa78d0767be53 2020-11-10T01:11:58-06:00 Paul Connelly: Merge branch 'master' into addon-2.9.3

This specific commit from #163 looks suspicious.