On Tue, 11 Feb 2020 at 21:12, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
On Tue, Feb 11, 2020 at 5:33 PM Henri Verbeet <hverbeet(a)gmail.com> wrote:
On Mon, 10 Feb 2020 at 23:06, Matteo Bruni <mbruni(a)codeweavers.com> wrote:
+static void wined3d_bitmask_set_bits(DWORD *bitmask, unsigned int offset, unsigned int count) +{ + const unsigned int word_bit_count = sizeof(*bitmask) * CHAR_BIT; + const unsigned int shift = offset % word_bit_count; + + bitmask += offset / word_bit_count; + *bitmask |= ~0u >> (word_bit_count - min(count, word_bit_count)) << shift; + ++bitmask; + count -= min(count, word_bit_count - shift); + if (!count) + return; + if (count >= word_bit_count) + { + memset(bitmask, 0xffu, count / CHAR_BIT); + bitmask += count / word_bit_count; + count = count % word_bit_count; + if (!count) + return; + } + *bitmask |= (1u << count) - 1; +} Does this intentionally not handle 0 count? I also suspect this has some room for simplification.
I wrote this patch a long time ago and looked through it so many times that it's hard for me to see problems. That's where review helps I guess :) I think I intentionally don't handle 0 count, I guess I could add an assert() at least. WRT simplification, this was originally written when I was looking into a game that sets most (or all) the float constants in one go so it somewhat reflects that (e.g. I think I started from the memset() part and then tacked the rest around it afterwards). I'll try to simplify it but I might ask for more specific suggestions if I can't find anything substantial...
I was mostly thinking you could get rid of the min() bits at the start by doing something like the following: mask = ~0u << shift; mask_size = word_bit_count - shift; last_mask = (1u << ((start + count) & (word_bit_count - 1))) - 1; if (mask_size < count) { *bitmap |= mask; ++bitmap; count -= mask_size; mask = ~0u; } ... if (count) *bitmap |= (mask & last_mask); It probably ends up being a little longer, but seems more straightforward and happens to handle 0 count as well.