Setting face colour using the Face Properties dock doesn't appear to do anything

[distractedmosfet]

As title says, it doesn't appear to do anything. The vertex painting feature works fine, and I can set it's component type to Face and that works, but I'd like to use the Face Properties as it seems less tedious if you just wanna colour the whole face.

There's a chance I'm just not understanding this, but I felt worst case scenario that reporting it might highlight something to specify in the docs.

I have no idea of the relevant code, but I did vertex paint before trying the face properties so I'm mildly curious if this is some sort of interaction where vertex colours and face colours are different values and vertex overrides face. I can see in the Mesh Vector Data that the Face Properties colour seems to be written to the Face Data object, but the vertex colours I painted is written to Face Vertex Data.

[distractedmosfet]

Alright I decided to dive into this a little bit. There's a chance that I'm just elaborating on something blackears already knows and plans to deal with but eh, can't hurt.

So in ConvexVolume's create_mesh I can see that it pulls the color from the FaceVertexInfo, that field is set by the paint stroke command, so all that makes sense for why vertex painting works. Changing the colour in the Face Properties dock creates a CommandSetFaceColor, which sets the field on the FaceInfo, not the FaceVertexInfo. I'm not sure if the color field on FaceInfo is ever meaningfully used? Seems like it gets used in build_face_vertices but that might just be an initialization thing?

Obviously, I'm on outsider to this code base, but from my perspective, it seems like there's two options of what to do there:

1. Remove the FaceInfo color field and have the dock set the FaceVertexInfo colors. This is pretty straight-forward and least likely to surprise users I imagine. It's certainly how I assumed it'd work. But obviously I'm not party to your (blackears') motivations and long-term goals, and perhaps there is a desire for this separation of face color and vertex color. Or perhaps the separation is incidental and just something that emerged as the code-base evolved.
2. Keep both and multiply them. It's an easy change in ConvexVolume from "colors.append(fv.color)" and to "colors.append(fv.color*face.color)". This is an interesting option, allows you to colour faces but then paint a vertex later to modulate it. I can see somewhat niche use cases for this. But it's perhaps unnecessarily complicated?

I have my own local-fork and for now, I've simply added this loop into the relevant place in do_it() inside cmd_set_face_color.gd

for fv in vol.face_vertices:
    if rec.face_indices.has(fv.face_index):
        fv.color = color

So it still sets face color, it just also writes face_vertex color. This is a nice local change for anyone who just wants to be able to use this right now like myself, because it doesn't really change anything about the underlying data, and whatever blackears decides to do you should be able upgrade to newer versions without much surprise. I don't really recommend anyone hack their local version to do the multiply thing because you're fundamentally changing the meaning of the data and there's no guarantee it'd be compatible with whatever blackears chooses to do. If they ditch FaceInfo.color entirely, you'll need to either hard-fork or lose all your per-face colors.

Anyway, there's a chance I've massively misunderstood the entire thing, and none of my suggestions make sense. But worst case scenario, that'll potentially highlight some doc clarifications.

[blackears]

In the original design, colors were stored per-face but when I added the vertex color brush, colors are now stored per face-vertex and the face color is ignored. I didn't remove the face color data yet, though, in case that caused issues with people importing older projects. So this control should be changed to set face-vertex colors or removed all together. The intended way to set colors now is with the vertex color brush.

I'm in the middle of rewriting menus and keymapping right now, so it might take me a while to get to this.

[distractedmosfet]

Alright, that is broadly what I expected had happened, and certainly keeping the faceinfo colour during the transition makes absolute sense.

I will mention that as a user I am pro keeping the ability to set the vertex colors face-by-face like this personally, but I say that only to provide a single data-point that you are free to ignore without me getting weirdly entitled about it. You're presumably mostly making this for yourself and I fully respect that you'll take it in whatever direction you please. Worst case scenario, that feature seems like something that'd be easy for me to maintain for myself in a local-fork.