Details

Reviewers

Bastien Montagne (mont29)
Campbell Barton (campbellbarton)

Commits

rB63ea0f7581fa: BLI_bitmap: fix _BITMAP_NUM_BLOCKS to not over-count by one block
rB8fd2b79ca190: BLI_bitmap: ability to declare by-value, and function to find lowest unset bit

Summary

In preparation for a larger change (D14162), some BLI_bitmap functionality that could be submitted separately:

Ability to declare a fixed size bitmap by-value, without extra memory allocation: BLI_BITMAP_DECLARE
Function to find the index of lowest unset bit: BLI_bitmap_find_first_unset
Test coverage of the above.
Fixed _BITMAP_NUM_BLOCKS to not over-allocate by one block. For bit counts that were exact multiple of block size, the previous macro was allocating one block too much.

Diff Detail

Repository: rB Blender

Event Timeline

Aras Pranckevicius (aras_p) requested review of this revision.Jul 14 2022, 5:10 PM

Aras Pranckevicius (aras_p) created this revision.

Looks fine besides a few picky details, and the naming of the new macro, not sure about that one.

Would not mind having @Campbell Barton (campbellbarton) check on this one too.

source/blender/blenlib/BLI_bitmap.h
49–53	Don't think we should follow the same naming as for the other 'creator' macros, since this one is more declaring it. `BLI_BITMAP_DECLARE` ?
source/blender/blenlib/intern/bitmap.c
51	Those two can be `const`.
53	can be `const` too
62–64	`const`, can also be simplified as `uint`.

This revision now requires changes to proceed.Jul 14 2022, 5:28 PM

Minor updates from review: macro name is BLI_BITMAP_DECLARE, and added some const qualifiers.

Aras Pranckevicius (aras_p) marked 4 inline comments as done.Jul 14 2022, 7:10 PM

Minor requests, otherwise LGTM.

source/blender/blenlib/BLI_bitmap.h
149	I'd prefer this is split this into two functions instead of having a `do_set` argument. Otherwise `bitmap` can't be `const` event when performing a read-only operation.
source/blender/blenlib/intern/bitmap.c
58	picky most of Blender's code uses `++` as a suffix unless the result of incrementing the value needs to be used.

This revision now requires changes to proceed.Jul 15 2022, 2:51 AM

Review feedback: change BLI_bitmap_find_first_unset to only search for the bit

Review feedback: changed prefix ++ to suffix ++.

Aras Pranckevicius (aras_p) marked 2 inline comments as done.Jul 15 2022, 8:22 AM

Accepting, although _BITMAP_NUM_BLOCKS can be committed separately (unless it's necessary to make it part of this patch).

This revision was not accepted when it landed; it landed in state Needs Review.Jul 15 2022, 9:21 AM

Closed by commit rB8fd2b79ca190: BLI_bitmap: ability to declare by-value, and function to find lowest unset bit (authored by Aras Pranckevicius (aras_p)). · Explain Why

This revision was automatically updated to reflect the committed changes.

Aras Pranckevicius (aras_p) added a commit: rB8fd2b79ca190: BLI_bitmap: ability to declare by-value, and function to find lowest unset bit.

Aras Pranckevicius (aras_p) added a commit: rB63ea0f7581fa: BLI_bitmap: fix _BITMAP_NUM_BLOCKS to not over-count by one block.

BLI_bitmap: ability to declare by-value, and function to find lowest unset bit
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 53675

source/blender/blenlib/BLI_bitmap.h

source/blender/blenlib/CMakeLists.txt

source/blender/blenlib/intern/bitmap.c

source/blender/blenlib/tests/BLI_bitmap_test.cc

BLI_bitmap: ability to declare by-value, and function to find lowest unset bitClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 53675

source/blender/blenlib/BLI_bitmap.h

source/blender/blenlib/CMakeLists.txt

source/blender/blenlib/intern/bitmap.c

source/blender/blenlib/tests/BLI_bitmap_test.cc

BLI_bitmap: ability to declare by-value, and function to find lowest unset bit
ClosedPublic
Actions

Revision Contents
Changeset List