Mesh: Rename "polys" to "faces" #109819

Hans Goudey · 2023-07-07T14:10:55+02:00

Hans Goudey commented

2023-07-07 14:10:55 +02:00

Implements part of #101689.

The "poly" name was chosen to distinguish the MLoop + MPoly combination
from the MFace struct it replaced. Those two structures persisted
together for a long time, but nowadays MPoly is gone, and MFace
is only used in some legacy code like the particle system.

To avoid unnecessarily using a different term, increase consistency with
the UI and with BMesh, and generally make code a bit easier to read,
this commit replaces the poly term with poly. Most variables that
use the term are renamed too. Mesh.totface now has a _legacy
suffix to reduce confusion. In a next step, pdata can be renamed
to face_data as well.

Implements part of #101689. The "poly" name was chosen to distinguish the `MLoop` + `MPoly` combination from the `MFace` struct it replaced. Those two structures persisted together for a long time, but nowadays `MPoly` is gone, and `MFace` is only used in some legacy code like the particle system. To avoid unnecessarily using a different term, increase consistency with the UI and with BMesh, and generally make code a bit easier to read, this commit replaces the `poly` term with `poly`. Most variables that use the term are renamed too. `Mesh.totface` now has a `_legacy` suffix to reduce confusion. In a next step, `pdata` can be renamed to `face_data` as well.

Hans Goudey added 1 commit 2023-07-07 14:11:07 +02:00

buildbot/vexp-code-patch-coordinator Build done. Details

04adcaf195 Mesh: Rename "polys" to "faces"

Implements part of #101689.

The "poly" name was chosen to distinguish the `MLoop` + `MPoly` system
from the `MFace` system it replaced. Those two structures persisted
together for a long time, but nowadays `MPoly` is gone, and `MFace`
is only used in some legacy code like the particle system.

To avoid unnecessarily using a different term, increase consistency with
the UI and with BMesh, and generally make code a bit easier to read,
this commit replaces the `poly` term with `poly`. Most variables that
use the term are renamed too. `Mesh.fdata` and `Mesh.totface` now
have a `_legacy` suffix to reduce confusion. In a next step, `pdata`
can be renamed to `face_data` as well.

Hans Goudey commented

2023-07-07 14:12:45 +02:00

@blender-bot build

Hans Goudey added this to the Modeling project 2023-07-07 14:12:52 +02:00

Hans Goudey requested review from Campbell Barton 2023-07-07 14:12:59 +02:00

Hans Goudey added 2 commits 2023-07-10 18:12:05 +02:00

0c06fa5693 Merge branch 'main' into cleanup-mesh-face-rename

d9b3897c01 Fix facegons -> faces

Campbell Barton commented

2023-07-12 03:17:55 +02:00

My main concern with this patch is it leaves things in an inconstant state.

Anywhere p/poly is currently used as a prefix (primarily pdata / fdata). Having me->faces not refer to fdata seems highly error prone.
If it's necessary to apply these changes in steps, a short term solution could be to rename fdata -> fdata_legacy, then at least we avoid accidental f/p confusion.
It would be good to know how expansive this change will be - is this change also impacting public API's or are these exempt from the rename? Reserving breaking changes for 4.x is good but this is a fairly large breaking change without any near-term up-side beyond consistency. If that's the end goal, I'd like to run it by other core developers.

My main concern with this patch is it leaves things in an inconstant state. - Anywhere `p/poly` is currently used as a prefix (primarily `pdata` / `fdata`). Having `me->faces` not refer to `fdata` seems highly error prone. - If it's necessary to apply these changes in steps, a short term solution could be to rename `fdata` -> `fdata_legacy`, then at least we avoid accidental f/p confusion. - It would be good to know how expansive this change will be - is this change also impacting public API's or are these exempt from the rename? Reserving breaking changes for 4.x is good but this is a fairly large breaking change without any near-term up-side beyond consistency. If that's the end goal, I'd like to run it by other core developers.

Hans Goudey added 3 commits 2023-07-12 14:28:12 +02:00

2dd5587819 Merge branch 'main' into cleanup-mesh-face-rename

a489a632e5 Rename some more poly -> face

9d245a1aac Rename `fdata` to `fdata_legacy`

Hans Goudey commented

2023-07-12 14:30:30 +02:00

I renamed fdata to fdata_legacy in this PR. But I would rather leave renaming pdata to face_data to a separate commit, so I can rename edata->edge_data and vdata->vert_data in the same commit.
Personally I don't have a strong opinion about whether to change RNA or not. I would rather do that in a separate PR though, to potentially group it with a "loop" to "corner" rename there too.

- I renamed `fdata` to `fdata_legacy` in this PR. But I would rather leave renaming `pdata` to `face_data` to a separate commit, so I can rename `edata->edge_data` and `vdata->vert_data` in the same commit. - Personally I don't have a strong opinion about whether to change RNA or not. I would rather do that in a separate PR though, to potentially group it with a "loop" to "corner" rename there too.

Hans Goudey added 1 commit 2023-07-14 14:03:30 +02:00

fe026e5854 Merge branch 'main' into cleanup-mesh-face-rename

Campbell Barton commented

2023-07-21 10:11:41 +02:00

Since this doesn't touch public API's (RNA), it seems OK as is.

Although soon after this I it would be good to rename custom-data:

Mesh::vert_data
Mesh::edge_data
Mesh::loop_data
Mesh::face_data
Mesh::face_data_legacy (stays as-is).

Currently having pdata references faces fairly counter intuitive but if it's only for a short time I don't think it's an issue.

Since this doesn't touch public API's (RNA), it seems OK as is. Although soon after this I it would be good to rename custom-data: - `Mesh::vert_data` - `Mesh::edge_data` - `Mesh::loop_data` - `Mesh::face_data` - `Mesh::face_data_legacy` (stays as-is). Currently having `pdata` references faces fairly counter intuitive but if it's only for a short time I don't think it's an issue.

Campbell Barton approved these changes 2023-07-21 10:12:49 +02:00

Campbell Barton reviewed 2023-07-21 10:16:27 +02:00

source/blender/blenlib/BLI_edgehash.h Outdated

						
				@ -181,3 +181,3 @@

				#define BLI_EDGEHASH_SIZE_GUESS_FROM_LOOPS(totloop) ((totloop) / 2)

				#define BLI_EDGEHASH_SIZE_GUESS_FROM_POLYS(totpoly) ((totpoly)*2)

				#define BLI_EDGEHASH_SIZE_GUESS_FROM_POLYS(faces_num) ((faces_num)*2)

Campbell Barton commented

2023-07-21 10:16:27 +02:00

BLI_EDGEHASH_SIZE_GUESS_FROM_POLYS -> BLI_EDGEHASH_SIZE_GUESS_FROM_FACES totloop can be renamed too.

`BLI_EDGEHASH_SIZE_GUESS_FROM_POLYS` -> `BLI_EDGEHASH_SIZE_GUESS_FROM_FACES` totloop can be renamed too.

Campbell Barton commented

2023-07-21 11:29:50 +02:00

Noticed some uppercase uses of POLY that could be renamed to FACE as part of this patch: DT_DATATYPE_IS_POLY DT_DATATYPE_IS_POLY MREMAP_MODE_VERT_POLY_NEAREST ME_POLY_TRI_TOT.

Noticed some uppercase uses of POLY that could be renamed to FACE as part of this patch: `DT_DATATYPE_IS_POLY DT_DATATYPE_IS_POLY MREMAP_MODE_VERT_POLY_NEAREST ME_POLY_TRI_TOT`.

Hans Goudey added 3 commits 2023-07-24 22:05:04 +02:00

e9ab5a5f57 Merge branch 'main' into cleanup-mesh-face-rename

b79972fb3b Rename more defines/enum values

c52c2b9f61 Fixes after merge

Hans Goudey referenced this issue from a commit

2023-07-24 22:07:03 +02:00

Mesh: Rename "polys" to "faces"

Hans Goudey merged commit 5e9ea9243b into main

2023-07-24 22:07:03 +02:00

Hans Goudey deleted branch cleanup-mesh-face-rename

2023-07-24 22:07:05 +02:00

Almaz-Shinbay referenced this issue from a commit