Page MenuHome

Reduce ambiguity with GPU vertex buffer operations
Needs ReviewPublic

Authored by Campbell Barton (campbellbarton) on Aug 21 2020, 10:05 AM.

Details

Summary

Made based on discussion in rBdd3f5186260eddc2c115b560bb832baff0f108ae.

replace_all = (
    ("GPU_vertbuf_data_len_set",      "GPU_vertbuf_set_number_of_verts"),
    ("GPU_vertbuf_data_alloc",        "GPU_vertbuf_alloc_number_of_verts"),
    ("GPU_vertbuf_data_resize",       "GPU_vertbuf_resize_number_of_verts"),
)

Diff Detail

Repository
rB Blender
Branch
TEMP-GPU-REFACTOR (branched from master)
Build Status
Buildable 9706
Build 9706: arc lint + arc unit

Event Timeline

Campbell Barton (campbellbarton) created this revision.
Campbell Barton (campbellbarton) retitled this revision from Rename GPU functions to Reduce functions.Aug 21 2020, 10:08 AM
Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)
Campbell Barton (campbellbarton) retitled this revision from Reduce functions to Reduce ambiguity with GPU vertex buffer operations.
Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)

@Campbell Barton (campbellbarton), as I've mentioned in the linked commit, naming is fine with me. But I expect such conventions become a part of guideline.
What is the urge of committing the cleanup vs. agreeing in conventions with the team and making it a part of guidelines first?

We can hold off with this until details are agreed on.
(posting so you can drop concern with that commit!)

Semi-related: Do we have convention about where the get and set of a getter/setter function should be? As in GPU_vertbuf_set_number_of_verts vs GPU_vertbuf_number_of_verts_set.

The GPUVertBuf only deals with number of elements (vertices) as the number of attributes has already been specified inside the vertex format. So I don't really see any need to be more explicit other than the parameter names (which could be changed to vert_len instead of v_len).

To me, removing the _data_ is confusing as it hides that the function affect the data the buffer hold (which is not true until data_alloc is called). We might soon have GPU_vertbuf_alloc() to allocate the GPUVertBuf object itself without initialization and the name would conflict with data handling.

The only issue I can see is GPU_vertbuf_data_len_set needing a better name for what it does. Any suggestion on this is welcomed.

If I have to have a take on this, I would rename to GPU_vertbuf_verts_* or GPU_vertbuf_verts_data_*.

And finally, if I have no choice but to use GPU_vertbuf_set_number_of_verts I would prefer the shorter version GPU_vertbuf_set_num_verts`.

Personally I like the idea of having get/set/alloc etc at the end. It groups the ordering inside the IDE especially when a lot of changes happen in that area. But on the other hand having a strict definition of get/set/alloc would also help as most likely you know what you want to do. I think these kind of topics work better when posted on devtalk.