There are situations where it would be useful to set specific fillMode per mesh rather than per material. For example in my case I render Ngon wireframes with Material.LineListDrawMode while remaining meshes can use the default fillMode. Main problem here is that some meshes can share same material but have some ngon primitives and some triangular primitives. So I have to duplicate the material for both drawMode variations.
It would automatically work for instances or any other variations of the mesh. And because Mesh already has other similar overrides such as overrideMaterialSideOrientation it would even fit with the style of other available options.
As a general rule, trying to keep any draw “pipeline” variations in the material is amazingly helpful for performances and caching. It might also be better suited for WebGPU.
Each time the mesh contains rendering information they need to be piped everywhere and it is bloating the code with extra dirty/compilation state creating potential slow path as well as being error prone.
This is some of the issues we are facing with opacity and I wish it would have been only available on the materials, I would rather not do this change even if I can totally understand how convenient it could be.
I was worried that would be the case. Although hoped that since the fillMode is already passed along as a single variable it would be doable as override option. But thanks anyway.
I am actually still considering it cause if we think about triangle fan for instance or lines they actually are more mesh notions (related to the buffers layout) than the shader themselves. Let me ask more friends for feedback @Evgeni_Popov@Deltakosh ???
Hmm, I suppose. I probably won’t be able to consider the optimization side effects you mentioned, and as a rookie Babylon.js contributor not fully sure about everything needed for a new Mesh property. I guess just the property, replicate serialization from similar properties and apply the fillMode in the render function. So sure, I’ll take a look at adding Draft PR tomorrow.
yup kinda touching everywhere fillMode is used lol + one inspector property in the mesh properties.
Maybe you can start the draft with only the core part: the prop in Mesh, the usage in render and raycast/collision so we can see how it looks before commiting to it fully ?
For the implementation I added a new internal getFillMode() helper method for SubMesh so there won’t be same increasingly complex conditional statements all over the code to get fillMode. And I replaced all existing parts in code where .fillMode was read.
Couple of parts that may require attention:
getFillMode() is currently reading renderingMesh.overrideMaterialFillMode. I’m not fully clear on differences between rendering, replacement or effective meshes, but I assumed that since the renderingMesh is only one that is actual Mesh and not AbstractMesh that likely is the one where mesh buffers come from and therefore is where fillMode override would come from as well.
Adding the overrideMaterialFillMode Mesh property triggered a “No more than 128 own properties on a mesh” unit test error at packages/dev/core/test/unit/Meshes/babylon.dictionaryMode.test.ts. That looks to be a very old unit test from 2019 and I couldn’t decipher any reason for 128 limit specifically so I just bumped the number up by 16 to 144.
It has 1 material, 2 meshes that both have bunch of instances. Other mesh overrideMaterialFillMode is changed and it works even though all meshes share same material.
Would love to see the actual implementation to make sure we are not breaking any optimizations but why not