search

Azure / azure-remote-rendering

SDK and samples for Azure Remote Rendering
MIT License
106 stars 38 forks source link

[Conversion Problem] numFaces output statistics doesn't account for instancing #122

Closed WikkidEdd closed 4 months ago

WikkidEdd commented 5 months ago

Describe the problem

On our platform when a user converts a model we use the output statistics to guide the user into using Premium if the numFaces is greater than 20m.

The documentation for the numFaces stat says

The total number of triangles in the whole model. This number contributes to the primitive limit in the standard rendering server size.

However, we've noticed that the numFaces parameter doesn't account for instanced models. For example we have a model that reports 11m faces, but when rendering the model with ARR the runtime stats show it has 52m faces.

Getting the correct number to tell a user whether they should use standard or premium is really important to us. Can that stat be updated to include the full face count including instances or failing that a different stat that does?

FlorianBorn71 commented 5 months ago

Thanks @WikkidEdd for letting us know! We will have a look into the counting and keep you updated here.

FlorianBorn71 commented 4 months ago

Hi @WikkidEdd , we fixed this issue for the next service release - just look out for new client-side releases to see when it lands.

Fwiw, the number "numFaces" refers to the number in the source files, which may differ from the output triangle count (more than often they are identical though). For the most accurate number, use numPrimitives in the output section instead. Accordingly, for the input part, the current statement "this number contributes to the primitive limit", is not accurate. I'm going to fix the docs. Note the "numPrimitives" currently does not consider instancing either, so the problem was in both numbers ;-)

WikkidEdd commented 4 months ago

Great, thanks!

The input vs output statistics did cross my mind when looking at the documentation. However, instead of fully reading the section on the output statistics I just looked at the example json and I didn't spot anything that seemed relevant so assumed there wasn't an equivalent in output statistics 🤦 Probably worth updating that example json too for documentation skimmers like me 😄

FlorianBorn71 commented 4 months ago

Very good point, I updated the sample output as well