Search Unity

  1. Welcome to the Unity Forums! Please take the time to read our Code of Conduct to familiarize yourself with the forum rules and how to post constructively.
  2. Dismiss Notice

Bug Unity's static batching: why does it shoot itself in the foot?

Discussion in 'General Graphics' started by Gondophares, Feb 4, 2021.

  1. FM-Productions

    FM-Productions

    Joined:
    May 1, 2017
    Posts:
    72
    Correct, the overhead would come from the game needing to render move geometry than necessary, since the batched objects have larger bounds to enclose all elements that were batched. So it is easier for the whole batched object to fall into the camera frustum, or to be rendered as when Occlusion Culling is active, which means that the whole geometry of the object is processed then. As for a fix of the batch grouping inside Unity, I'd like to see a possible backport as well. During testing and talking to others, it seems that it depends on whether static batching or pure material instancing achieves a better performance. I'm also curious about the new BatchRendererGroup API in terms of performance improvements in cases where default static batching would have been used
     
    Last edited: Dec 6, 2022
  2. virtualjay

    virtualjay

    Joined:
    Aug 4, 2020
    Posts:
    68
    I would think Unity's inefficient groupings can throw frustum culling (and occlusion culling) out the window, though. Because in our scenes, it would combine objects from all over the scene together into various groups, meaning they'd be drawn even when they were behind the camera. That doesn't seem zero impact. When we create them manually, we can combine together ones that are close together spatially and more likely to be completely culled.
     
    FM-Productions likes this.
  3. AcidArrow

    AcidArrow

    Joined:
    May 20, 2010
    Posts:
    11,001
    The original meshes are still kept and are turned off and on to accommodate culling, at least that's how things used to work. so there are definitely things happening at runtime.

    Maybe no one knows how things are supposed to work vs how they are working though.
     
    Gondophares and virtualjay like this.
  4. virtualjay

    virtualjay

    Joined:
    Aug 4, 2020
    Posts:
    68
    It really does seem like too much of a black box.
     
  5. joshuacwilde

    joshuacwilde

    Joined:
    Feb 4, 2018
    Posts:
    692
    It's because there is still a draw call issued per object, at least at the graphics API level. It's just that Unity copies all the data into a big buffer so there are minimal graphics API calls for each new mesh drawn. Should just be 1 graphics API call for each new mesh. So you still get the benefit of culling. The issue is you now have a giant blob in memory of all your combined meshes, and also because of how the rest of Unity's render pipeline works, it still isn't very performant.
     
    Gondophares likes this.
  6. Gondophares

    Gondophares

    Joined:
    Mar 9, 2013
    Posts:
    28
    Well, that's actually the strength of Unity's static batching (or rather: should *be* its strength, if it worked sensibly). Individual objects are batched into a single mesh, but are still kept as separate submeshes. (Their MeshRenderers are sneakily told which submesh index they should use.) This allows us to still do "per object" culling and even toggle individual MeshRenderers at runtime, even though we're dealing with a "single mesh".

    This is distinct from more naive batching where you would indeed render the entire elephant when just its toenail is on screen. I've tried to build a system that preserves Unity's capabilities before and had to conclude it's not a trivial undertaking. Even one of the more popular batching plugins on the Asset Store, which we sometimes use, takes the more basic "monolithic mesh" approach.

    All this to point out that I think Unity's static batching is actually pretty nifty and has value. If, again, it worked as intended...

    It does, yeah. I do feel this is one of those decidedly "old Unity" parts of the engine that would benefit from a more modern, parametric/extensible approach, akin to the SRP. A basic API with which to define interchangeable "batching strategies", with a sensible default implementation (e.g. reimplementation of current system) and the option to build custom implementations for specific use cases.
     
  7. AcidArrow

    AcidArrow

    Joined:
    May 20, 2010
    Posts:
    11,001
  8. tatoforever

    tatoforever

    Joined:
    Apr 16, 2009
    Posts:
    4,337
    Then Unity must be wasting CPU cycles doing something else when Static Batching is enabled cause while the number of drawcalls is significantly lower, the CPU time is higher. At least on the Nintendo Switch. For example; one of the scene I have, it exhibit in average ~1600 drawcalls (without static batching) and around 500 drawcalls (with static batching is enabled). However CPU time is slower (by 10%) if Static Batching is enabled.
    I have seen this behavior in all my scenes (it can vary from 5% to 10%). I'm also on 2019.4.40 (although runtime performance has degraded on 2020 and 2021).
    If i share many meshes/materials and they are static, I should expect less CPU time right (at least the same)? Well is not the case for me, so I just disable Static batching.
     
  9. Gondophares

    Gondophares

    Joined:
    Mar 9, 2013
    Posts:
    28
    Good point. Could it be the increased CPU time is a bandwidth issue caused by having to (repeatedly) upload the much bigger blob of mesh data for the merged mesh? I imagine this would be especially prominent if the batched mesh is iterated over inefficiently, because then it would have to be re-uploaded several times?
     
    tatoforever likes this.
  10. Error-md1

    Error-md1

    Joined:
    Nov 9, 2022
    Posts:
    13
    So, I just want to point out the current solution to the static batching sorting is almost as bad as sorting on instanceID. Lets look at the internal code posted by richardkettlewell which is very obviously a straight copy of the suggestion by Gondophares.

    Code (CSharp):
    1.                    // simple "spatial" sort on axes (prefer x/z plane)
    2.                    auto lPos = lhs.localToWorld.GetPosition();
    3.                    auto rPos = rhs.localToWorld.GetPosition();
    4.                    if (lPos.x != rPos.x)
    5.                        return lPos.x < rPos.x;
    6.                    if (lPos.z != rPos.z)
    7.                        return lPos.z < rPos.z;
    8.                    if (lPos.y != rPos.y)
    9.                        return lPos.y < rPos.y;
    10.  
    Three 1d sorts is NOT a 3d spatial sort! This basically means all objects in the scene get sorted on the X axis alone unless they happen to have exactly equal X positions, falling back to z then y. Aside from edge cases where objects are duplicated and moved on one axis, this will preferentially group objects that are probably very far away but have similar X values. This is actually useless! Furthermore, assuming this actually worked, making static batching optimized for the XZ-plane assumes way to much. You shouldn't be making sweeping assumptions about what kinds of games people are making, especially given how flexible Unity is!

    The best way (that I know of, you guys should really be doing your own research on such core systems rather than copy-pasting forum code!) is to use a space filling curve. This works by converting the positions to integer coordinates within some very large bounding box, getting their 1d positions on a space-filling curve that passes through every integer point, and sorting by that. This is exactly what apkdev's patch posted in this thread does, using a Hilbert curve for sorting. A perhaps better option is the Z-order curve which is fast, easy to implement, and extremely commonly used in graphics applications for stuff like this. In any case, this actually groups nearby objects together, unlike the 3 1d sorts, and properly sorts all axes with equal weight.

    Additionally, sorting should be done with the Renderer's bounding box center and not the transform position, as it is common for static meshes to not actually be centered on their transform position.

    Also, you need to backport any fixes for static batching to 2022. Static batching and batched meshes have no public API beyond calling Mesh.CombineMeshes, so it is literally impossible that anyone's code is relying on the current implementation. Furthermore, in 2022 you've already broken apkdev's patch by moving the sorting function into the C++ side of the engine where it can't be redirected, so the one possible thing that users might be relying on is already broken.
     
  11. Noisecrime

    Noisecrime

    Joined:
    Apr 7, 2010
    Posts:
    2,000
    From what I observed ( and can remember ) when testing this a few years ago, whilst all the vertex data is uploaded to the card once, I'm pretty sure the index lists ( triangle index arrays ) are ( or rather can be ) uploaded per frame depending on which sub-objects are in view.

    Now a decade ago when we still had relatively low vertex count models this would be fine, but could easily see in more modern titles with much greater vertex counts that the shear index count of lists being uploaded could present efficiency issues - thus also explaining the higher cpu usage.

    Actually thinking about it, its likely vastly more complicated than that since static batches can have different materials, lightmap ids, etc , but I still think Unity may be trying to reduce drawcalls when possible by combing several smaller index buffers into a single one.

    That said one would assume the index lists wouldn't be uploaded every frame if they hadn't changed, but who knows with such an old legacy piece of code.

    As others have said static batching seems to suffer from
    • Being a black box - no-one outside of Unity really knows what goes on.
    • The code has probably never been revisited in a decade or more
    • There are probably improved methods for dealing with static meshes. ( e.g. instancing will be more performant, but if its not static in unity you lose things like lightmaps )
    • Zero API hooks. In the past I wanted to custom render static batched items but you simply don't have api access to get the information.
    So overall I get the feeling that Unity should probably dedicate some time to investigate the current batching solution and see what improvements could be made, or if it should be marked as legacy and an improved system built to replace it, but give user option to choose which to use.

    I wonder if the new SRP batching system is partly a solution for this? I know its meant to be far superior to old object/material/shader system, but not sure if its really a replacement for static batching, since static batching concept would seem to still have benefits if done correctly.

    I wonder if with modern GPU's/API's if the cpu hit mentioned above could be effectively removed if the dynamic creation of index lists for static batches were moved to the GPU? I'm thinking perhaps a compute shader could maybe do that, and likely faster than could ever be achieved on cpu, especially as you wouldn't even need to transfer any data between them. Would be tempting to try it out, but again the lack of open API for static batching makes it impossible.
     
    Last edited: Feb 1, 2023
    LuGus-Studios-Kasper likes this.
  12. Noisecrime

    Noisecrime

    Joined:
    Apr 7, 2010
    Posts:
    2,000
    I got the feeling this was just a quick fix to the issue that was found in this thread, a band-aid, rather than a through investigation and solution.

    Based on the information you provided it would seem like static batching should at least provide an option as to the sorting method used, so it can be tailored to requirements and efficiency of algorithm.

    That said, overall as I said in my previous post it feels like all the various problems highlighted that the old static batching code may well be so out dated in approach that it needs a complete overhaul.
     
    LuGus-Studios-Kasper likes this.
  13. Error-md1

    Error-md1

    Joined:
    Nov 9, 2022
    Posts:
    13
    Oh two more things I forgot to mention.

    1. As far as I can tell, combined meshes seem to only ever use 16 bit index buiffers. This makes static batching useless with high poly meshes. Maybe have a scene or project setting to enable 32 bit combined meshes or try to intelligently choose the correct index buffer size based on number of objects in the mesh before it hits the limit.

    2. I'm pretty sure meshes with multiple materials don't get split up, so attempting to sort by material only works if each mesh only has one material. Submeshes should be treated as individual meshes when sorting and combining to ensure materials actually get sorted.
     
  14. Error-md1

    Error-md1

    Joined:
    Nov 9, 2022
    Posts:
    13
    Static batching currently sucks because of the lack of real sorting. If multiple objects in the frame share the same shader variant and inputs, and are contiguous in the combined mesh vertex buffer then unity will draw them in a single call. With the instance ID or X-only sort, that essentially never happens. Also even if they are sorted, differing non-material shader inputs like light and reflection probes will break them into separate drawcalls as well. Oh, and I'm pretty sure unity is not splitting meshes by material before mesh combining, so multiple material meshes are guaranteed to force multiple calls. And then there's also the issue that combined meshes use 16 bit index buffers, so they can only be 65k verts. So if you have high poly meshes static batching does literally nothing.

    That being said, if you use the patch pre-2022 and are careful about reflection probe placement and only using single material meshes, you can get unity to reduce actual drawcalls by a very large margin. With apkd's patch, properly breaking up meshes into single material parts, using an uber-shader that takes texture arrays and reflection-probe cube arrays, I've literally got 80% of a scene to draw in a single call.

    The main issue here though is unity is stuck using render pipeline architecture from almost a decade ago. Everyone else has moved on to GPU driven rendering techniques where drawcalls basically don't matter. But that's a separate issue
     
  15. AcidArrow

    AcidArrow

    Joined:
    May 20, 2010
    Posts:
    11,001
    Not even that, static batching used to work better in the 3-4.x era (although the issue with multiple materials in one mesh existed even then)
     
  16. Gondophares

    Gondophares

    Joined:
    Mar 9, 2013
    Posts:
    28
    Yeah, for the record, that code there was to illustrate how even the most naive sorting implementation would improve upon the randomized ordering. Though if I'm speaking for our use cases, a properly intelligent sorting doesn't even matter that much, as long as the horror show from the first post can be avoided. I'd much prefer something better, but I'd have settled for something remotely sensible. Though yes, if we're talking about the bigger issue here, look no further:


    This is anecdotal at best, but I do have one complex scene running on Built-In RP where the old batching is problematic. I also have that same scene in a different projected, converted to URP, with SRP batching and without static batching. In the latter case, performance and overhead are noticeably better. I suspect this is largely to do with the SRP batching, but I haven't investigated it in detail.

    That said, I think there are definitely legitimate uses cases for the "old" static batching, even going forward into URP, so I wouldn't see that as a reason not to improve/rethink it.
     
  17. Noisecrime

    Noisecrime

    Joined:
    Apr 7, 2010
    Posts:
    2,000
    Ouch, only just saw this while re-reading the thread, so the only benefit of the old broken static batching system where we could over-ride the behaviour has now been removed! This is really bad.

    I was coming back to this thread as I was thinking perhaps the community could make inroads into a better static batching solution. Was even thinking of starting a thread to discussion all the various aspects that would need to be considered ( e.g. additional effects like reflection probes breaking batches, so how could that be accounted for, and providing options for 16 or 32 bit index buffers etc ), but doesn't seem to be much point now.

    I've wondered if the SRP Batcher is meant to be a complete replacement for static batching, but even as good as it is, I can't believe it wouldn't also benefit from improved static batches, especially as SRP appears to still support static batching. Perhaps the SRP Batcher and static batching for SRP have already considered all this, that Unity have made the best modern solution, but haven't really said as much because they don't want to back-port it ( even if possible ) to the legacy built-in renderer.


    These are the sort of stats I would have liked to see from Unity. Granted this is likely a special case, but the fact you were able to accomplish this, with the 'expert' knowledge of your projects needs, strongly suggest to me that Unity should have opened up the static batcher and not closed source it as they now have. Additionally they could have added native support for different sort types and supplemented it all with a nice e-book that outlines all the batch breaking issues ( e.g. reflection probe ) and describe methods to minimise that.

    Can you clarify what you mean by 'drawcalls don't matter'?

    While drawcalls themselves are not the overhead they once were ( or maybe ever were ) they still provide a degree of overhead ( when using many thousands or tens of thousands ), so I find it difficult to understand how GPU driven techniques means they don't matter? Unless by GPU driven you mean that there are methods that simply mean we no longer have to issue many of the drawcalls we used to - i.e. drawcalls still matter but GPU driven techniques reduce the need for drawcalls in the first place.


    So my takeaways from this are;
    1. Unity needs to provide open API with hooks into/out of the editor static batcher.
    2. Perhaps provide a nice frontend and backend logging to the batcher so we can understand what is going on without simply resorting to playing the project and using the frame debugger.
    3. Unity should provide a collection of native sorting methods to choose, but option to hook into a custom c# solution ( include legacy sorting methods to avoid breaking old projects )
    4. Unity should provide improved documentation and breakdowns of how to get the best out of static batching.
     
  18. Noisecrime

    Noisecrime

    Joined:
    Apr 7, 2010
    Posts:
    2,000
    So I'm going to submit a 'critical' 'new idea' to the Unity Foundation Roadmap ( seems the most appropriate, you have to switch to the correct sub-tab on the page ) in the hope that this whole situation gets the attention it deserves. I've written my initial proposal below and would like to get input from others in case I've missed anything.

    I've focused on the sorting method mainly, but I feel there are probably other aspects of the static batching system that might be useful to expose too. Does anyone have any suggestions? If so would you mind just listing any other points as a bullet point list I can then look to incorporate them into the text below, as opposed to people trying to rewrite the whole thing.

    One thing that might help would be if other developers also submit new ideas after I've added this one. Perhaps we could all start with the same first line to act as a type of 'heading/title' since the submission doesn't have the ability to use a title.


    >>> NEW IDEA SUBMISSION <<<
    Address fundamental issues of the Static Batching system.

    It has come to the community's attention that the current Static Batching system ( as used at build time ) is not fit for purpose. Whilst a recent update ( 2023.1.0a5 - https://tinyurl.com/mw5ex8as) was made to address this, the solution is far from ideal, it was a decent (and apricated ) emergency fix but one that feels short of the improvements possible.

    Even worse a version of Unity prior to the fix ( sometime in 2022.x ) moved the batch sorting code to native code, making it a completely 'closed' system preventing previous community improvements and likely would have made finding the original issue almost impossible!

    Prior to the recent fix (2023.1.0a5) static batches were combined on the basis of the instanceID of the mesh, completely ignoring any spatial relationship! Afterwards it uses a simple axis comparison, but this is far from ideal, even as a generic solution.

    This is a critical situation as it has been demonstrated that the current state of the Static Batching system is failing developers and attempts to fix it have been locked into only the latest release (2023) ignoring LTS. Additionally community fixes no longer working as of Unity 2022.x. This makes it vital that Unity and the community worked together to get this addressed ASAP so that at least going forward from 2023 developers will have the assurance and confidence that upgrading projects would have real benefits.

    Proposal to solve the issues
    1. An open API, with hooks to/out of the native static batching system.
    2. Per project static batching sorting method option ( legacy support via instanceID or by prominent axis, modern support - space filling curves etc )
    3. Per project option to use a custom sorting method in a C# script ( Burst/Job support )
    4. Per project/per platform option to use 16 or 32 bit batch indices.
    5. Logging of the batching process for easier debugging and understanding - maybe a nice frontend

    Greater information and discussion of the issues can be found in this thread
    https://forum.unity.com/threads/unitys-static-batching-why-does-it-shoot-itself-in-the-foot.1051604/

    Importantly I'd like to request that prior to making any changes to address this issue that Unity engage directly with the community to get greater and more in-depth understanding of the problems and pain points. I feel that simply focusing on just the sorting method as outlined above might not address the full needs of the community. Ideally create anew thread and post links in the above and any other relevant threads.
    >>> END SUBMISSION <<<

    Edit:
    I might add an additional section to the submission of 'things to have'.

    One of these for myself would be the ability to use static meshes in DrawMesh(). The last time I looked at this there was no way to get the static mesh or any information at all. From memory you could use sharedMesh to get the static mesh, but no way to know which gameObjects it related to. So if you iterated through all the static meshes sharedMesh you'd render the static mesh multiple times.

    The reason for this ( again from memory ) was to render a custom renderTexture using a custom material, instead of resorting to a second camera or other methods. In the end I abandoned it as I didn't have the necessary information. Something simple like a database that related a static mesh with the gameObject meshes it contains would have been helpful. Though could be seen as part of the 'logging' option mentioned above - basically just caching the useful information from the static batching.

    Edit 2:
    I submitted the idea. If anyone else would also like to submit supporting 'ideas' then go to the link at the start of this post, scroll down to find tab options and selecting the last one 'Foundation', the scroll down again until you see the 'submit a new idea' then type in what you'd like to see in terms of improving static batching. By all means use any of the text above as a starting point, though obviously uniquely written ideas will likely have more merit than simple copy & paste.
     
    Last edited: Feb 24, 2023
  19. richardkettlewell

    richardkettlewell

    Unity Technologies

    Joined:
    Sep 9, 2015
    Posts:
    2,239
    Hey all,

    If you have some time, and feel inclined to do so, give this custom Editor build a go and let me know what you think:
    https://beta.unity3d.com/download/1939816026c6/public_download.html

    It adds a new script API: StaticBatchingUtility.SetCustomSortingDelegate.
    You give it a method, and that method should be called with System.Span<StaticBatchingUtility.BatchingData> instances as the parameter. You can then sort these however you like.

    I've not had any chance to do much testing on it yet, or get any internal feedback, but it's hopefully a starting point.
    If nothing else, it gives you a concrete API to look at, to see if it might fit your needs.

    One question in my mind is, can you get the bounds from the provided renderer instance ID? Last time I checked, I think it wasn't trivial to get a renderer from just an instance ID. But, assuming you can, you should be able to get the bounds from the renderer, if you want to sort using that instead of just the center.

    EDIT: Looks like we have https://docs.unity3d.com/ScriptReference/EditorUtility.InstanceIDToObject.html, but it's editor-only.
     
  20. Noisecrime

    Noisecrime

    Joined:
    Apr 7, 2010
    Posts:
    2,000
    Thanks @richardkettlewell for taking a stab at improving this situation. Sadly i'm on holiday for the next two weeks, but when I get back, assuming I remember and aren't overwhelmed with client updates i'll take a look. It certainly sounds like an interesting avenue to explore, hopefully others from this thread will get an opportunity to take a look.
     
    richardkettlewell likes this.
  21. Error-md1

    Error-md1

    Joined:
    Nov 9, 2022
    Posts:
    13
    Thanks! I'm glad this is finally getting addressed. That sounds like a perfectly usable API. Only being able to map the instanceID's to object in editor shouldn't be much of an issue. I'm pretty sure you can get an equivalent function at runtime through reflection though. I'll take a look at the build when I have time.

    As a side note, for those of us stuck working with 2022 I managed to create my own solution for static batching. It turns out the IProcessScene interface allows you to non-destructively modify the scene just before it gets packaged. Crucially this happens before static batching has taken place which allows me to manually sort, combine, and assign the combined meshes and then remove the static flag so normal static batching never happens. Its currently in a state of just barely functioning with the minimum amount of functionality, but I plan on continuing to update it.
    https://github.com/StressLevelZero/CustomStaticBatching
     
  22. Gondophares

    Gondophares

    Joined:
    Mar 9, 2013
    Posts:
    28
    I ran some quick tests, reusing the same test scenario from my first post. I've attached a package containing a quick test scene, a script that tries out some *very contrived* sorting logic (
    CustomStaticBatchingRoot
    ), and a batch visualizer (
    DebugBatches
    ).

    Bottom line: the new sorting, though criticisms could be leveled against its exact implementations, performs noticeably better than the mistaken instanceID sorting. The other sorting examples used here, though obviously very contrived, work as expected and I can imagine use cases where such a custom sorting logic could come in handy. At any rate, I generally prefer that approach: make a configurable basis and then, using those same tools, provide a sensible default that should cover many "typical" use cases.

    Couple small remarks:
    • I couldn't find out how to natively sort a Span within the current C# version supported by Unity. (Haven't worked much with them yet.) I'm probably overlooking something. Had to do some ugly things with temp lists as a workaround. Ignore that.
    • As you can tell from the script, always working your way up from the instance ID is a little cumbersome. Would be helpful if there was a slightly more direct access to the renderer/mesh filter, but it's not a big deal.
    Another remark I have from an architectural point of view: wouldn't it make more sense to have this custom sorting delegate be an optional second argument on
    StaticBatchingUtility.Combine()
    ? I could foresee building several different batching scripts. Currently I only have to forget to update/reset the One And True Sorting Delegate in one place before subsequent
    StaticBatchingUtility.Combine()
    s start behaving in unexpected ways.

    On a related note: it's nice that we can do this with StaticBatchingUtility, but what of build-time batching? Or is StaticBatchingUtility actually also used for that, and could we set this in some sort of build script (say: per scene?)
     

    Attached Files:

  23. AcidArrow

    AcidArrow

    Joined:
    May 20, 2010
    Posts:
    11,001
    This topic makes me sad when I remember that static batching used to work a lot more reliably in the 4.x days.
     
    Ruslank100 likes this.
  24. tatoforever

    tatoforever

    Joined:
    Apr 16, 2009
    Posts:
    4,337
    I did a test with a quick sorting algorithm and here are the results.

    Default sorting (Unity):


    Quick sorting (mine):
     
  25. Unifikation

    Unifikation

    Joined:
    Jan 4, 2023
    Posts:
    1,043
    Crikey. And strewth!
     
    tatoforever likes this.
  26. tatoforever

    tatoforever

    Joined:
    Apr 16, 2009
    Posts:
    4,337
    I improved it a bit more:
    upload_2023-7-3_13-20-33.png
     
  27. Unifikation

    Unifikation

    Joined:
    Jan 4, 2023
    Posts:
    1,043
    This is getting embarrassing... for Unity!
     
  28. Error-md1

    Error-md1

    Joined:
    Nov 9, 2022
    Posts:
    13
    Too
    Took a look at this today, seems mostly fine except for a few issues.

    Passing the delegate a span isn't great, as spans don't have any sorting methods until .NET 8, requiring the user to write their own sorting methods. I'm guessing its being passed as a span as a way to pass the pointer to the array from the native side to managed space without requiring an unsafe context or marshalling the data to/from a managed array. If that's the case, why not pass the data as a NativeArray? It can fulfill basically the same role as a span in this context, and the collections package has some nice extensions for sorting NativeArrays.

    Since there can only be one sorting delegate, a warning or error should be thrown when multiple scripts try to change the delegate. I can envision situations where multiple packages in a project include their own sorting delegate, setting it from an [InitializeOnLoadMethod] attributed method. At that point it becomes a race condition where the last method to load will win, and that probably isn't consistent between editor restarts or even script reloads.
     
  29. richardkettlewell

    richardkettlewell

    Unity Technologies

    Joined:
    Sep 9, 2015
    Posts:
    2,239
    Thanks for the feedback, I didn't realise we had no easy way to perform sorting on a Span. I think the best bet is for us to just provide a NativeArray instead.

    Though as a workaround you can probably use https://docs.unity3d.com/ScriptRefe...Utility.ConvertExistingDataToNativeArray.html to map the Span to a NativeArray, until I rework the API. I've asked our scripting team for confirmation on the best way to present this array to scripts. (but i'm almost certain NativeArray is the answer, as that has sorting methods, and can work nicely with Burst too)

    I agree. Only exposing blittable ("simple") types allows us to expose the static batching data to script with no copying. It's the most efficient way, but not the most user-friendly.

    Well, in the way I've done it so far, if the delegate is registered, it will always be used, not only when StaticBatchingUtility.Combine in manually invoked. As I think you've noticed (from a comment in your code), once you register it, the built-in sorting is turned off, and you are responsible for any sorting. So it should be invoked for build-time batching too. (though I've not really tested any of this thoroughly enough to say these things with any confidence - this is very much a side-project for me, to try and improve things for the folks in this thread)

    Would you want/need more flexible control over how it is called? is per scene control potentially useful?
     
  30. richardkettlewell

    richardkettlewell

    Unity Technologies

    Joined:
    Sep 9, 2015
    Posts:
    2,239
    Good point - thanks!
     
    Gondophares likes this.
  31. tatoforever

    tatoforever

    Joined:
    Apr 16, 2009
    Posts:
    4,337
    I also second the support for NativeArrays.
    Needed to post a comment (better than a like)! :D
     
    richardkettlewell likes this.
  32. Gondophares

    Gondophares

    Joined:
    Mar 9, 2013
    Posts:
    28
    Fair enough, I suspect NativeArray is the way to go then.

    Well, this is why I asked if StaticBatchingUtility is in fact the exact same pipeline used for build-time batching. Thing is, I'm not quite convinced of the realism of "once registered, it will always be used". This will hold true in trivial scenarios where I've consciously registered this delegate to test this exact scenario.

    But in a realistic scenario, (speaking here with the experience of semi-large projects with a long history), I or someone on my team may have added a plugin that may (unbeknown to me, or even undocumented) register its own delegate. Or we may need different delegates for different scenarios and have forgotten about an older one. All of these may register their own delegate, at certain times, in a certain order, under certain conditions, on certain platforms... What if something registers a delegate for build-time batching, accidentally also affecting the runtime batching of some script written before the custom sorting was a thing? Or vice versa. (How do assembly reloads work, by the way?) In other words, a few years from now, I foresee myself having a couple of really bad days trying to figure out why the performance of everything is suddenly on fire. ;)

    But on a more abstract level, I feel that if some data is only relevant within the context of a particular operation, it should also be local to that operation (i.e. passed). Otherwise you're just creating unnecessary state that's asking to get messed up. Or put another way: if I'm always going to (re)set the delegate before I call StaticBatchingUtility.Combine, just to be safe, they might as well be one call. I for one much prefer very explicitly passing a sorting method for some specific use case, rather than something "helpfully" changing that sorting method across my entire project.

    So I would rather propose the following:

    Code (CSharp):
    1.  
    2. // Always uses default sorting.
    3. public static void Combine(GameObject staticBatchRoot);
    4. // Uses delegateMethod or default sorting if delegateMethod is null.
    5. public static void Combine(GameObject staticBatchRoot, CustomSortingDelegate delegateMethod);
    6.  
    Which brings us to build-time batching, which I feel is the primary motivation for setting the delegate in the way currently proposed, correct? To answer your question: yes, control would definitely need to be more flexible than a per-project basis. In my case, almost all problems I sought to improve with the formerly broken batching were highly specific to a particular scene. Setting a single sorting method across the entire project would render the whole thing useless and possibly detrimental.

    I think the proper way to go about this would be to add a hook in the build process (don't know the exact API) where I am given a scene and can (optionally) return a sorting method.

    Much appreciated!
     
  33. richardkettlewell

    richardkettlewell

    Unity Technologies

    Joined:
    Sep 9, 2015
    Posts:
    2,239
    Thanks for all the feedback so far. I've made some changes (but I've not yet made a new public build)

    The important information:
    • I've replaced the Span<BatchingData> with a NativeArray<BatchingData>, so it's much easier to sort the data
    • I've changed how you "register" the custom sorting, which i will go into more detail about, below
    The script API's now take an optional extra param for the sorting, eg:
    Code (CSharp):
    1. public static void Combine(GameObject staticBatchRoot, CustomSortingDelegate delegateMethod);
    The automatic batching that happens at build time is now hooked into our C# code that dispatches the Static Batching (https://github.com/Unity-Technologies/UnityCsReference/blob/master/Editor/Mono/PostprocessScene.cs)

    I've modified that code to ask if you want to use custom sorting for a given scene, and if you return a sorting delegate, the static batching will use it.

    So usage is:
    Code (CSharp):
    1. StaticBatchingUtility.RegisterCustomSortingDelegate(MyQueryFunc);
    2.  
    3. StaticBatchingUtility.CustomSortingDelegate MyQueryFunc(SceneManagement.Scene scene)
    4. {
    5.     if (scene.path == "myspecialscene")
    6.         return MySortingFunc;
    7.     return null;
    8. }
    9.  
    10. void MySortingFunc(NativeArray<BatchingData> data)
    11. {
    12.     data.Sort(MyCompareFn);
    13. }
    Note that there is still some global state to manage here. (registering MyQueryFunc)
    I'm not really sure how to completely get around that (more ideas welcome!) :)

    It feels like, if we need the global state, then perhaps it needs to be a list of "query funcs"...

    Let me know your thoughts.
     
  34. TJHeuvel-net

    TJHeuvel-net

    Joined:
    Jul 31, 2012
    Posts:
    818
    (Out of context, but it would be great if we could use MemoryExtensions.Sort, i.e. sort spans!)