Yes, that’s the plan we have discussed. If you’re interested in the code, the project amounts to making MLoopUV into float2– in other words, finding a separate place to store MLOOPUV_VERTSEL and MLOOPUV_PINNED. That project would be very similar to the one I’ve been working on, T93602: Convert Mesh vertex data into float3.
2 Likes
So this is the data legacy UV layers keep in addition to float2? for selection and pinning. This and vertex positions as float3 sound like proper spring cleaning 
1 Like
For the pinning and selection in MLoopUV, I think it mostly comes down to a design discussion about where to store those two flags. Personally I’d suggest two separate boolean layers, but it should be discussed with others who have worked on UV editing.
The generic attribute types use the same enum, but they all start with CD_PROP_.
I’ve found that incomplete transitions are really bad for code quality. And there are so many of them! Just for fun, here’s the first few I thought of:
-
DerivedMesh→Mesh - Specific attribute types → generic attributes
-
Curve→CurveEval - Modifiers → Nodes
-
MFace→MPoly - C → C++
1 Like
Yea sure. I can start with looking if those flags are used together often, or mostly separately. But design discussion are not really my thing yet. If there is a clear idea about the way forward I don’t mind doing the gruntwork but I’m not confident enough to suggest what the way forward should be.
Hell yeah. When I was working on my texture margin code I thought DerivedMesh was the more general thing! Even though in hindsight I can see DerivedMesh is an invitation for performance hurldes 
These two I personally don’t mind. More of a ‘if it aint broken, dont fix it’ thing imho. Also there’s not really confusion as to which is the way to go
1 Like
After a quick grep through the code: They seem quite independent because they are hardly ever used together.
Also, currently it’s an int bitflag of which 2 out of the 32 bits are used. Changing it to 2 CD_PROP_BOOL arrays would be slightly cheaper memory wise I’d think. Most compilers store bools as chars I think?
To make this discussion easier to read I moved it to a new thread.
That’s great! The generic solution of boolean arrays for separate flags sounds good to me.
Also good news. And about the size, yeah, at this point we sort of count on that too, since we store boolean attributes in files.
I think the remaining design topics are:
- The names of boolean attributes that could replace these flags.
- Should they be “reserved name” attributes where the UV editor expects a certain name, or configurable in the UI of the UV editor? Probably the former, I think.
- Should we have some separate namespace for these attributes? I thought starting the names with a period might be a good way to do that.
- Whether these internal layers should show up in the “Attributes” list in mesh properties.
- I think I’d say no, since these are really just used for the UI and operators. Maybe some developer extras setting could still display all attributes in the list though.
These same questions would apply to the MVert refactor, so they’re definitely worth answering.
- I’d say a standard name. I don’t really see the point in making that configurable.
- A separate namespace for ‘internal’ stuff seems like a good idea. Though I’m not a fan of completely hiding them. I’m always amazed at what strange hacks people think of.
Thanks for moving the thread by the way. I felt rather offtopic in the other one
Whatever the decision about these bool attributes will be, I think it’s important to consider that a beginner user should not have to dive into the concept of attributes to do basic UV editor operations, which selection and pinning are a part of.
I just want to make sure there’s no such thing as having to fill out some input field with some name before one can start selecting and pinning stuff in the UV editor
Well, I’d think the gui should stay exactly the same. Just that it internally stores it somewhere else.
Only add-writers might have to do some work. Comparable to when the normals were split off recently.
1 Like
Alright, perfect I was just making sure.
I didn’t really do anything yet, except grepping through the source for the various moving parts but there’s one design-ish thing I thought of:
UV layers have a name. I guess we can eventually store the UV layers in a generic float2 attribute. We don’t need to do anything special there. Ideally you could use any float2 attribute as UV map I think. But those 2 extra bool flags probably need to be linked to the UV layer in some way.
This somehow ties in to some sort of scoping I’d guess. Maybe prepend or append stuff to the name of the UV layer?
Something like “.pinned-UVlayer” and “.vertsel-UVlayer” for an UV layer named “UVlayer” ? And then not forget to rename them when the layer itself gets renamed…
It feels a bit hackish. But maybe keep-it-simple is just the best? The renaming part somewhat bothers me.
The renaming does seem like the worst part, everything else else seems pretty straightforward.
I guess the RNA property for UV CustomDataLayers would have to have some special behavior.
I wonder if @JacquesLucke has thought about this before.
Pins are currently associated with a particular UV map. Seams on edges really should be also, but are not, since exactly this kind of synchronization is difficult. There’s also normal map tangents that are associated with each UV map, but that’s runtime data and not so much of a problem.
If there was a single set of pins and seams for one UV map, that would not really be a regression compared to the current state, since you different pins for the same seams makes little sense. But ideally that limitation could be lifted.
I’m not sure what a clean way would be to sync add/delete/transfer/rename such attributes though.
1 Like
That’s not what I was talking about. I think there’s currently a single set of pins per UV map currently?
I was just talking about keeping the association between the UV (float2) data and it’s associated pins vert-sels if you have multiple UV maps.
I should take a look at how normal map tangents are stored with an UV map. I didn’t see them yet. Or are they always generated on use… Will have to look into that
yes, this is imho the biggest design hurdle. And not something I want to burn my fingers on, green as I am in the blender world. We should probably hash this out first.
I’ll go in with an experimental conversion just to see what’s happening and what other problems I come across, but the ‘real’ job will have to wait till we think of how to do this properly. There would need to be something like ‘associated custom data layers’ I’d think.
1 Like
Yes, ideally we would keep that association. What I’m saying is that there is currently only a single set of edge seams, not one per UV map, which makes the per-UV map pins not particularly useful in practice.
However the ideal solution would be for the seams to also become associated with a UV map.
2 Likes
Ah! Well, this seems like a good occasion to straighten that as well 
. Now just to think up a fool proof way to have a General Purpose attribute also have a few dependent/associated layers…
Question:
Am I correct in assuming I need to use the CustomData interface or the attribute interface from BKE_attribute.h when I’m trying to access attributes from a (B)Mesh?
And I think in BMesh code (where I don’t have access to a Mesh) I need to use the ‘raw’ CustomData stuff? Myabe that’s best anyway for efficiency reasons.
The interface in BKE_geometry_set.hh uses a wrapper around the actual mesh data (Geometry Set/MeshComponent) and I don’t think I have that available from a BMesh * (or a Mesh *). I think that’s only available from an Object level?
I’m sorry if this post is a mess, it’s a bit of a brain dump after yet another hour of grepping the source… I’m starting to get a vague mental model of the various moving parts involved. Vague…
I don’t really expect an answer to this incoherent rambling. It already helps me to write it down. Only if I seem completely off, please let me know.
It is a bit confusing that we have to attribute APIs at the moment. Eventually they could be merged, but they each have benefits at the moment. The API in BKE_attribute.h is used for RNA and C code (not C++). That code usually works directly on an ID anyway, and is usually tied to original data, meaning working directly on DNA structs is a bit simpler. On the other hand, the attribute API in BKE_geometry_set.hh abstracts data types a bit more. For example, it exposes vertex groups and UV maps as float and float2 layers. This simplifies code and makes it faster since templates can be used for operations on attributes. The GeometryComponent API can actually be used anywhere you have a mesh, it just takes a couple lines of setup currently, like this:
MeshComponent component;
component.replace_mesh(mesh, GeometryOwnershipType::Editable);
There is no implementation of the C++ attribute API for BMesh yet. It’s probably something we should add eventually, but the rather inefficient way BMesh stores attributes makes it a bit less appetizing (as in, keeping geometry data as Mesh rather than BMesh as much as possible is probably faster).
Mesh stores all of the data for a specific attribute contiguously in big arrays, but BMesh stores the data for all attibutes together on one element in BMHeader::data, AoS style.
1 Like
That’s where I got stuck in my fist ‘attempt’.
I commented out the flag from the MLoopUV struct and checked the compiler errors to try to understand/devise ways to get at attribute data from those code locations.
For the Mesh handling I could more or less think of what to do, but for BMesh it’s more of a hassle, because BMesh loops over the BMLoops via a linked list and then gets the MLoopUV data from a Customdata layer at a fixed offset of each BMLoop (I hope I remeber this correctly). But because it’s linked list, I don’t have an offset in a MLoopArray anymore which I could use to index an attribute layer…
As far as I can see now I need to copy the bool attributes into the customdata of the BMesh (just like they are now, as part of MLoopUV) to be able to use them from within BMesh. And copy them back out to an attribute again after editing is done, probably.
Thanks for your quick reply, btw. It helps a lot.
Well, remember that any attribute is stored that way in BMesh, even boolean attributes. All you need is the offset for that particular attribute, and you can get it with some nice casting from void *. But that’s the way it’s meant to work.
Some version of CustomData_get_offset that takes an attribute name and a type would probably do the trick. We could guarantee that those layers would have a boolean type and then that shouldn’t be a problem.
So I think an algorithm dealing with one of the flags would generally retrieve the offset at the beginning, then use it while looping through the BMesh data.
Of course, I’m glad to help! Especially when you’re putting in time on a project I think is really important.