It's great that we can remove all this code, just needs to be feature complete before it can be merged.

https://builder.blender.org/admin/#/builders/18/builds/406

Patch with Cycles fixes: P2952

• Only the hair length test is still failing, however it's actually fixing a bug. where Cycles ignored the Render > Extra > Parent Particle setting.
• The first float2 attribute is assumed to be the default UV map. There doesn't seem to be anything to determine which attribute is a UV map or not, and which is the active one.

https://builder.blender.org/admin/#/builders/18/builds/407

There's still some work to be done.

• The inability to detect the active UV map will cause some files to be rendered differently.
• Eevee/workbench don't seem to have working intercept, length or UV attributes.
• Eevee curve shapes seems a bit different than before. Not sure if this is a result of Cycles and Eevee being different before and matching now, but for example render/motion_blur/curve_motion_blur.blend has more subdivision than before.

I compared the Eevee/Workbench render tests before and after. But I needed to apply D14969 to make the tests work, and also had to update the reference images before since they are outdated.

Yeah fair enough, will add that to the description.

For the Eevee/workbench changes it seems reasonable to wait for D14916. Both patches would probably conflict quite a bit otherwise. Also, this patch has to wait for D14916 anyway.
I had some trouble running the eevee/workbench tests earlier today, thanks for looking into that.

The inability to detect the active UV map will cause some files to be rendered differently.

You mean in cases where it would not detect any active uv map before and now uses a float2 attribute, or what case do you have in mind?

In D14942#402711, @Jacques Lucke (JacquesLucke) wrote:

You mean in cases where it would not detect any active uv map before and now uses a float2 attribute, or what case do you have in mind?

When there are multiple UV maps, you can set which UV map is active for viewport/render. That's the one that is used in the UV Map node when not specifying a name, and the default texture coordinate for the Image Texture node.

This is a general design issue when switching from dedicated UV map attributes to generic float2 attributes, not sure if a solution was considered for that.

I see that there is a CD_PROP_UV in D14365, that would help figure out which is a UV, but could not find a doc explaining the intended design.

This is a general design issue when switching from dedicated UV map attributes to generic float2 attributes, not sure if a solution was considered for that.

At some point we were considering adding "usage hints" to attributes. Some "active" status could be part of that.
Another possibility would be to pass the name of the "active uv map" to the shader using some property on the geometry (e.g. char *Mesh.active_uv_map or using custom properties). The UI for choosing the active uv map could still stay the same, we might just need some way to also set the name from geometry nodes.

[EDIT] Actually, these are not necessarily alternatives, both (attribute usage hints and Mesh.active_uv_map) can exist at the same time.
[EDIT2] Haven't seen CD_PROP_UV before, currently I don't agree with that design.

The CD_PROP_UV in D14365 is not 'real' though. It's just a #define to CD_PROP_FLOAT2. The only reason I used this define is to ease the search&replace if we decide to distinguish between UVs and general purpose FLOAT2 layers after all. Afaik the current plan is to not make a distinction and just use replace CD_RPOP_UV with CD_PROP_FLOAT2 before eventually merging D14365 . When starting out I didn't yet know that CD_PROP_FLOAT2 was basically unused in the codebase and I wanted an easy way to distinguish between the new (UV) layer accesses and the existing FLOAT2 stuff.

I created T98366: Active Attributes to discuss how we want to handle the problem with active uv maps.