Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compact edges #5581

Merged
merged 23 commits into from Jun 6, 2023
Merged

Compact edges #5581

merged 23 commits into from Jun 6, 2023

Conversation

pmconne
Copy link
Member

@pmconne pmconne commented May 31, 2023

imodel-native: https://github.com/iTwin/imodel-native/tree/pmc/compact-edges
PR

imodel-native-internal: https://github.com/iTwin/imodel-native-internal/tree/pmc/compact-edges
PR

The amount of geometry required to render the edges of a surface is many times larger than that required for the surface itself. So, when edge display is turned off in a viewport (the common case for 3d views), the frontend requests tiles that omit the edges, to avoid having to download much more data than it actually needs. When edge display is enabled, it requests an entirely separate set of tiles that do contain the edges.

The introduction of indexed edges significantly reduced the amount of memory reserved (in tiles and on the GPU) for edges. We had hoped it would reduce it sufficiently that we could produce only one set of tiles and have the client discard the edges if it did not wish to display them. The reduction was not sufficient to justify this.

With the introduction of support for decoding tile contents in a web worker we now have the opportunity to offload a little more computation to the client without impacting the user experience. So this PR (and the linked native ones) updates the iMdl format to support encoding just enough information about the edges of the surface, in as compact a form as possible, to enable the client to construct the edge geometry itself during tile decoding. We expect the additional reduction in tile size to finally enable us to stop producing tiles containing no edge data.

A surface is defined as a triangle mesh. Its index buffer describes the triangles as triplets of consecutive indices into the vertex table. In the example below, there are 4 vertices and 2 triangles described by the index list [0,1,2, 0,3,2].

 0 ____ 3
  |\   |
  | \  |
  |  \ |
  |___\|
 1      2

The visibility of the edges of each of these triangles can be categorized as one of: never visible, always visible, or visible based on viewing angle (i.e., a silhouette). We can lay out another array parallel to the index array specifying the visibility of each edge. Assuming the diagonal above is a silhouette, the vertical edges are always visible, and the horizontal ones are never visible, we have [visible,hidden,silhouette, hidden,visible,silhouette]. Because we can convey the visibility of each edge using only 2 bits, we encode the edge visibility buffer as a bitfield. So for each 24-bit vertex index in the mesh, we add 2 more bits for the edge visibility.

However, in the example above the shared (silhouette) edge between vertices 0 and 2 appears twice - once per triangle. We don't want to draw nor allocate memory for the same edge twice, so when encoding we ensure shared edges appear only once: [visible,hidden,silhouette, hidden,visible,hidden].

To compute the visibility of silhouettes, we need a vector normal to each adjacent face. We can infer the third vertex of one of the faces, but not the other. So, if we wanted to compute the normals on the frontend, we'd need to store the 24-bit index of the third vertex of the opposite face. Alternatively, we can compute both normals on the backend and encode them as a pair of 16-bit oct-encoded normals. Trading one extra byte to save a ton of extra client-side computation seemed preferable so that's what we do. The pairs of normals are encoded into a separate buffer such that the Nth pair of normals corresponds to the Nth silhouette encountered while traversing the visibility buffer.

On the client-side all of this data is decoded into the same IndexedEdgeParams that we would previously have encoded directly into the tile. So, the amount of GPU memory is unchanged, the amount of computation on the frontend is slightly increased (but offloaded from the UI thread), and the size of the tiles is significantly reduced.

Ultimately we intend to enhance the mesh export service to encode edge visibility this way so that only one tileset will need to be produced regardless of whether the client has enabled display of edges in the viewport.

The table below shows the sizes in bytes of the encoded tiles without edges and with compact edges. The last 3 rows summarize the results.

Content Id No Edges Edges Delta % Delta
Aggr3DC.ibim:0x1c:-b-1-0-1-0-1 1767600 1916704 149104 8
Aggr3DC.ibim:0x1c:-b-1-0-0-0-1 1357144 1521424 164280 12
Aggr3DC.ibim:0x1c:-b-1-0-0-1-1 3209036 3490952 281916 9
Aggr3DC.ibim:0x1c:-b-1-1-1-0-1 3019936 3266096 246160 8
Aggr3DC.ibim:0x1c:-b-1-0-1-1-1 3834252 4032132 197880 5
Aggr3DC.ibim:0x1c:-b-1-1-0-0-1 6065352 6575052 509700 8
Aggr3DA.ibim:0x1c:-b-1-0-0-0-1 363456 379456 16000 4
Aggr3DA.ibim:0x1c:-b-1-0-1-0-1 1179796 1306424 126628 11
Aggr3DA.ibim:0x1c:-b-1-1-0-1-1 1020992 1278844 257852 25
Aggr3DA.ibim:0x1c:-b-1-1-1-0-1 1090432 1249700 159268 15
Aggr3DA.ibim:0x1c:-b-1-1-1-1-1 1462348 1552168 89820 6
Aggr3DA.ibim:0x1c:-b-1-0-1-1-1 2137808 2437272 299464 14
Aggr3DA.ibim:0x1c:-b-1-0-0-1-1 3075444 3377560 302116 10
Aggr3DA.ibim:0x1c:-b-1-1-0-0-1 6675196 7585612 910416 14
GeoLocated/ClubHouse.bim:0x40:-b-1-0-0-0-1 572784 609808 37024 6
GeoLocated/ClubHouse.bim:0x40:-b-1-0-1-0-1 490308 517044 26736 5
GeoLocated/ClubHouse.bim:0x40:-b-1-1-0-0-1 2940356 3037588 97232 3
GeoLocated/ClubHouse.bim:0x40:-b-1-1-1-0-1 4065452 4140008 74556 2
GeoLocated/ClubHouse.bim:0x40:-b-1-0-0-1-1 3079508 3225284 145776 5
GeoLocated/ClubHouse.bim:0x40:-b-1-0-1-1-1 3875276 4167224 291948 8
GeoLocated/ClubHouse.bim:0x40:-b-1-1-1-1-1 4943152 5011860 68708 1
GeoLocated/ClubHouse.bim:0x40:-b-1-1-0-1-1 7198300 7274228 75928 1
GeoLocated/ClubHouse.bim:0xa7:-b-1-0-0-0-1 59080 70060 10980 19
GeoLocated/ClubHouse.bim:0xa7:-b-1-0-0-1-1 64972 76260 11288 17
GeoLocated/ClubHouse.bim:0xa9:-b-1-0-0-0-1 295224 317080 21856 7
PSolidAndPipesNew.bim:0x1d:-b-1-1-0-1-1 3588 4408 820 23
PSolidAndPipesNew.bim:0x1d:-b-1-1-1-0-1 3245684 3788364 542680 17
PSolidAndPipesNew.bim:0x1d:-b-1-1-0-0-1 8636668 9977308 1340640 16
PSolidAndPipesNew.bim:0x1d:-b-1-0-1-0-1 11758256 13913544 2155288 18
PSolidAndPipesNew.bim:0x1d:-b-1-0-0-0-1 13996812 16269160 2272348 16
proj.ibim.animated.ibim:0x10000000001:-a-1-0-0-0-1 2624 2816 192 7
proj.ibim.animated.ibim:0x10000000001:-a-1-0-1-0-1 2584 2780 196 8
proj.ibim.animated.ibim:0x10000000001:-a-1-1-1-1-1 2740 2940 200 7
proj.ibim.animated.ibim:0x10000000001:-a-1-1-0-1-1 5124 5376 252 5
proj.ibim.animated.ibim:0x10000000001:-a-1-0-0-1-1 9444 9828 384 4
proj.ibim.animated.ibim:0x10000000001:-a-1-1-0-0-1 12520 12980 460 4
proj.ibim.animated.ibim:0x10000000001:-a-1-0-1-1-1 21649412 22199212 549800 3
proj.ibim.animated.ibim:0x10000000001:-a-1-1-1-0-1 116569836 119516184 2946348 3
ScheduleSimulation/AnimatedCastle.ibim:0x10000000001:-a-1 1656 1824 168 10
ScheduleSimulation/AnimatedCastle.ibim:0x10000000001:-a-1 1548 1716 168 11
ScheduleSimulation/AnimatedCastle.ibim:0x10000000001:-a-1 1548 1716 168 11
ScheduleSimulation/AnimatedCastle.ibim:0x10000000001:-a-1 1648 1820 172 10
ScheduleSimulation/AnimatedCastle.ibim:0x10000000001:-a-1 11073428 11385928 312500 3
ScheduleSimulation/AnimatedCastle.ibim:0x10000000001:-a-1 11721500 12023040 301540 3
ScheduleSimulation/AnimatedCastle.ibim:0x10000000001:-a-1 57374188 58816576 1442388 3
ScheduleSimulation/AnimatedCastle.ibim:0x10000000001:-a-1 81717048 83801128 2084080 3
All_Master_Original.bim:0x22:-b-1-0-0-0-1 138136 142516 4380 3
All_Master_Original.bim:0x26:-b-1-0-0-1-1 351024 362176 11152 3
All_Master_Original.bim:0x24:-b-1-0-0-1-1 474228 495392 21164 4
All_Master_Original.bim:0x26:-b-1-0-0-0-1 567256 586248 18992 3
All_Master_Original.bim:0x33:-b-1-0-1-0-1 56872 58760 1888 3
All_Master_Original.bim:0x33:-b-1-0-0-0-1 159180 163620 4440 3
All_Master_Original.bim:0x22:-b-1-0-0-1-1 285828 294992 9164 3
All_Master_Original.bim:0x22:-b-1-0-1-0-1 5648 5860 212 4
All_Master_Original.bim:0x2e:-b-1-0-0-0-1 233832 237868 4036 2
All_Master_Original.bim:0x2e:-b-1-0-0-1-1 874700 891388 16688 2
All_Master_Original.bim:0x22:-b-1-0-1-1-1 2912 3092 180 6
All_Master_Original.bim:0x2a:-b-1-1-0-0-1 9544 11156 1612 17
All_Master_Original.bim:0x2a:-b-1-0-1-1-1 5892 6748 856 15
All_Master_Original.bim:0x37:-b-1-0-0-0-1 731932 753176 21244 3
All_Master_Original.bim:0x2a:-b-1-0-1-0-1 3640 4272 632 17
All_Master_Original.bim:0x35:-b-1-0-0-0-1 1215208 1267732 52524 4
All_Master_Original.bim:0x2c:-b-1-0-0-1-1 2032352 2079504 47152 2
All_Master_Original.bim:0x28:-b-1-0-0-0-1 2604224 2761532 157308 6
All_Master_Original.bim:0x28:-b-1-0-0-1-1 3186132 3393184 207052 6
All_Master_Original.bim:0x22:-b-1-1-0-0-1 2668 2844 176 7
All_Master_Original.bim:0x50:-b-1-0-0-0-1 236508 241208 4700 2
All_Master_Original.bim:0x4e:-b-1-0-0-0-1 1376408 1446796 70388 5
All_Master_Original.bim:0x4c:-b-1-0-0-1-1 17108 18808 1700 10
All_Master_Original.bim:0x2c:-b-1-0-0-0-1 6157620 6343680 186060 3
All_Master_Original.bim:0x3d:-b-1-0-0-0-1 7779636 7946448 166812 2
All_Master_Original.bim:0x39:-b-1-0-0-0-1 12965696 13533272 567576 4
All_Master_Original.bim:0x68:-b-1-1-0-0-1 6864 7412 548 8
All_Master_Original.bim:0x70:-b-1-1-0-0-1 11764 12948 1184 10
All_Master_Original.bim:0x61:-b-1-0-0-0-1 13300 14344 1044 8
All_Master_Original.bim:0x48:-b-1-0-0-0-1 62516 64292 1776 3
All_Master_Original.bim:0x5f:-b-1-0-1-0-1 38580 40448 1868 5
All_Master_Original.bim:0x5d:-b-1-0-1-0-1 29984 31784 1800 6
All_Master_Original.bim:0x68:-b-1-0-1-1-1 41904 43672 1768 4
All_Master_Original.bim:0x44:-b-1-0-0-0-1 66448 68812 2364 4
All_Master_Original.bim:0x46:-b-1-0-0-0-1 163544 171416 7872 5
All_Master_Original.bim:0x57:-b-1-0-1-0-1 10536 11636 1100 10
All_Master_Original.bim:0x70:-b-1-0-1-1-1 109728 113072 3344 3
All_Master_Original.bim:0x72:-b-1-0-0-1-1 144524 147236 2712 2
All_Master_Original.bim:0x55:-b-1-0-0-0-1 144416 149052 4636 3
All_Master_Original.bim:0x6e:-b-1-0-0-1-1 9816 11280 1464 15
All_Master_Original.bim:0x4a:-b-1-0-0-0-1 1052052 1112932 60880 6
All_Master_Original.bim:0x6e:-b-1-1-0-0-1 16964 18532 1568 9
All_Master_Original.bim:0x66:-b-1-0-0-1-1 172692 177088 4396 3
All_Master_Original.bim:0x24:-b-1-0-0-0-1 820340 849132 28792 4
All_Master_Original.bim:0x3f:-b-1-0-0-0-1 595628 606084 10456 2
All_Master_Original.bim:0x68:-b-1-0-1-0-1 377172 391488 14316 4
All_Master_Original.bim:0x66:-b-1-0-0-0-1 371348 380864 9516 3
All_Master_Original.bim:0x68:-b-1-0-0-1-1 475420 499156 23736 5
All_Master_Original.bim:0x57:-b-1-0-0-1-1 230456 239240 8784 4
All_Master_Original.bim:0x72:-b-1-0-0-0-1 1195628 1217400 21772 2
All_Master_Original.bim:0x3b:-b-1-0-0-0-1 7555696 8043372 487676 6
All_Master_Original.bim:0x70:-b-1-0-1-0-1 655648 673220 17572 3
All_Master_Original.bim:0x6a:-b-1-0-0-0-1 2139632 2187048 47416 2
All_Master_Original.bim:0x5b:-b-1-0-0-0-1 5479668 5797336 317668 6
All_Master_Original.bim:0x57:-b-1-0-0-0-1 1550444 1605436 54992 4
All_Master_Original.bim:0x68:-b-1-0-0-0-1 2945808 3059760 113952 4
All_Master_Original.bim:0x6e:-b-1-0-1-0-1 1099848 1176920 77072 7
All_Master_Original.bim:0x70:-b-1-0-0-1-1 1769268 1818836 49568 3
All_Master_Original.bim:0x4c:-b-1-0-0-0-1 4305868 4575188 269320 6
All_Master_Original.bim:0x6e:-b-1-0-0-0-1 5366868 5753248 386380 7
All_Master_Original.bim:0x5f:-b-1-0-0-0-1 8307544 8506924 199380 2
All_Master_Original.bim:0x5d:-b-1-0-0-0-1 9419452 10056380 636928 7
All_Master_Original.bim:0x2a:-b-1-0-0-1-1 8355284 8901144 545860 7
All_Master_Original.bim:0x70:-b-1-0-0-0-1 14480408 14841392 360984 2
All_Master_Original.bim:0x2a:-b-1-0-0-0-1 11517432 12356088 838656 7
MAX 116569836 119516184 2946348 25
MIN 1548 1716 168 1
MEAN 4812754 5035481 222726 6.7

When comparing tile sizes for tiles without edges vs those with non-compact indexed edges, the max % increase is 179%, min is 22%, and the mean is 73% So compact edges produce an order of magnitude reduction in tile size.

TODO

  • Evaluate ImageTests results
  • Add a tile content flag to enable this feature
  • Bump tile format version

@pmconne pmconne marked this pull request as ready for review June 6, 2023 11:01
@pmconne pmconne requested review from bbastings, kabentley and a team as code owners June 6, 2023 11:01
@pmconne pmconne requested a review from grigasp June 6, 2023 11:01
@pmconne
Copy link
Member Author

pmconne commented Jun 6, 2023

@wgoehrig @paulius-valiunas this PR includes a tile format version bump and a change to the default TileOptions.

@pmconne pmconne requested a review from a team as a code owner June 6, 2023 11:26
Copy link
Contributor

@markschlosseratbentley markschlosseratbentley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through all of the code and it all looked good to me.

@pmconne
Copy link
Member Author

pmconne commented Jun 6, 2023

@grigasp presentation tests failing.
@DanRod1999 how did this get past PR validation (did PR validation fail to run?)

https://dev.azure.com/bentleycs/iModelTechnologies/_build/results?buildId=2194004&view=logs&jobId=e538c5b1-ca02-55cf-272d-45e0b52dd4fb&j=e538c5b1-ca02-55cf-272d-45e0b52dd4fb&t=99805134-a3d2-5f30-5ea2-cbff40d4dd4f

--[ FAILURE: presentation-full-stack-tests ]--------[ 1 minute 28.3 seconds ]--


  280 passing (1m)
  2 failing

  1) Hierarchies
       Getting node paths
         gets node paths based on instance key paths:

      AssertionError: expected value to match snapshot Hierarchies Getting node paths gets node paths based on instance key paths 1
      + expected - actual
  ...166 lines omitted...
      +        "6d6f3fe0cbaecea3f22a752ca98877e3",
             ],
             "type": "ECClassGroupingNode",
             "version": 2,
           },
      
      at D:\vsts_b\154\s\full-stack-tests\presentation\src\frontend\Hierarchies.test.ts:1087:30

Copy link
Contributor

@bbastings bbastings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detailed commit comment much appreciated!

@saskliutas
Copy link
Member

@grigasp presentation tests failing. @DanRod1999 how did this get past PR validation (did PR validation fail to run?)

https://dev.azure.com/bentleycs/iModelTechnologies/_build/results?buildId=2194004&view=logs&jobId=e538c5b1-ca02-55cf-272d-45e0b52dd4fb&j=e538c5b1-ca02-55cf-272d-45e0b52dd4fb&t=99805134-a3d2-5f30-5ea2-cbff40d4dd4f

--[ FAILURE: presentation-full-stack-tests ]--------[ 1 minute 28.3 seconds ]--


  280 passing (1m)
  2 failing

  1) Hierarchies
       Getting node paths
         gets node paths based on instance key paths:

      AssertionError: expected value to match snapshot Hierarchies Getting node paths gets node paths based on instance key paths 1
      + expected - actual
  ...166 lines omitted...
      +        "6d6f3fe0cbaecea3f22a752ca98877e3",
             ],
             "type": "ECClassGroupingNode",
             "version": 2,
           },
      
      at D:\vsts_b\154\s\full-stack-tests\presentation\src\frontend\Hierarchies.test.ts:1087:30

Looks like it fails because your branch is behind imodel02. This commit is missing in your branch 4ad7164

@pmconne
Copy link
Member Author

pmconne commented Jun 6, 2023

Looks like it fails because your branch is behind imodel02

Thanks. Bit of a pain to have to start from scratch with the native builds every time there's an update to itwinjs-core.

@DanRod1999
Copy link
Contributor

DanRo

@grigasp presentation tests failing. @DanRod1999 how did this get past PR validation (did PR validation fail to run?)
https://dev.azure.com/bentleycs/iModelTechnologies/_build/results?buildId=2194004&view=logs&jobId=e538c5b1-ca02-55cf-272d-45e0b52dd4fb&j=e538c5b1-ca02-55cf-272d-45e0b52dd4fb&t=99805134-a3d2-5f30-5ea2-cbff40d4dd4f

--[ FAILURE: presentation-full-stack-tests ]--------[ 1 minute 28.3 seconds ]--


  280 passing (1m)
  2 failing

  1) Hierarchies
       Getting node paths
         gets node paths based on instance key paths:

      AssertionError: expected value to match snapshot Hierarchies Getting node paths gets node paths based on instance key paths 1
      + expected - actual
  ...166 lines omitted...
      +        "6d6f3fe0cbaecea3f22a752ca98877e3",
             ],
             "type": "ECClassGroupingNode",
             "version": 2,
           },
      
      at D:\vsts_b\154\s\full-stack-tests\presentation\src\frontend\Hierarchies.test.ts:1087:30

Looks like it fails because your branch is behind imodel02. This commit is missing in your branch 4ad7164

I checked the pipeline, i believe its running fine. These are the branches its picking up
itwinjs-core commit: 754a366
itwinjs-core branch: pmc/compact-edges
imodel-native commit: 4643e65784bc72c0c9c6ba65760a48520cd25fe6
imodel-native branch: pmc/compact-edges

@pmconne pmconne merged commit 1940e2e into imodel02 Jun 6, 2023
7 checks passed
@pmconne pmconne deleted the pmc/compact-edges branch June 6, 2023 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants