Skip to content

Commit e14fb47

Browse files
60k41p60k41p
committed
Implement a more thorough fix. Add tests.
1 parent fcc063e commit e14fb47

5 files changed

Lines changed: 241 additions & 17 deletions

File tree

‎distr/flecs.c‎

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80183,8 +80183,8 @@ int flecs_query_compile(
8018380183
do {
8018480184
/* Compile remaining query terms to instructions */
8018580185
for (i = start_term; i < term_count; i ++) {
80186-
ecs_term_t *term = &terms[i];
8018780186
int32_t compile = i;
80187+
ecs_term_t *term = &terms[i];
8018880188

8018980189
if (compiled & (1ull << i)) {
8019080190
continue; /* Already compiled */
@@ -85192,8 +85192,21 @@ static inline int32_t flecs_ctz64(uint64_t v) {
8519285192
#endif
8519385193
}
8519485194

85195+
static
85196+
bool flecs_query_term_is_in_or_chain(
85197+
const ecs_query_t *query,
85198+
int32_t term_index)
85199+
{
85200+
ecs_term_t *terms = query->terms;
85201+
int32_t term_count = query->term_count;
85202+
return (terms[term_index].oper == EcsOr) ||
85203+
(term_index > 0 && terms[term_index - 1].oper == EcsOr) ||
85204+
((term_index + 1) < term_count && terms[term_index + 1].oper == EcsOr);
85205+
}
85206+
8519585207
static
8519685208
flecs_query_row_mask_t flecs_query_get_row_mask(
85209+
const ecs_query_run_ctx_t *ctx,
8519785210
ecs_iter_t *it,
8519885211
ecs_table_t *table,
8519985212
int32_t block_index,
@@ -85205,6 +85218,7 @@ flecs_query_row_mask_t flecs_query_get_row_mask(
8520585218
int32_t i, field_count = it->field_count;
8520685219
ecs_flags64_t fields = and_fields | not_fields;
8520785220
bool has_bitset = false;
85221+
const ecs_query_t *query = &ctx->query->pub;
8520885222

8520985223
for (i = 0; i < field_count; i ++) {
8521085224
uint64_t field_bit = 1llu << i;
@@ -85221,8 +85235,43 @@ flecs_query_row_mask_t flecs_query_get_row_mask(
8522185235
}
8522285236

8522385237
ecs_id_t id = it->ids[i];
85238+
ecs_flags64_t block = 0;
85239+
bool field_has_bitset = false;
85240+
8522485241
ecs_bitset_t *bs = flecs_table_get_toggle(table, id);
85225-
if (!bs) {
85242+
if (bs) {
85243+
ecs_assert((64 * block_index) < bs->size, ECS_INTERNAL_ERROR, NULL);
85244+
block = bs->data[block_index];
85245+
field_has_bitset = true;
85246+
}
85247+
85248+
int32_t t;
85249+
for (t = 0; t < query->term_count; t ++) {
85250+
ecs_term_t *term = &query->terms[t];
85251+
if (!(term->flags_ & EcsTermIsToggle)) {
85252+
continue;
85253+
}
85254+
if (term->field_index != i) {
85255+
continue;
85256+
}
85257+
if (!flecs_query_term_is_in_or_chain(query, t)) {
85258+
continue;
85259+
}
85260+
if (term->id == id) {
85261+
continue;
85262+
}
85263+
85264+
ecs_bitset_t *or_bs = flecs_table_get_toggle(table, term->id);
85265+
if (!or_bs) {
85266+
continue;
85267+
}
85268+
85269+
ecs_assert((64 * block_index) < or_bs->size, ECS_INTERNAL_ERROR, NULL);
85270+
block |= or_bs->data[block_index];
85271+
field_has_bitset = true;
85272+
}
85273+
85274+
if (!field_has_bitset) {
8522685275
if (not_fields & field_bit) {
8522785276
if (op_ctx->prev_set_fields & field_bit) {
8522885277
has_bitset = false;
@@ -85232,12 +85281,10 @@ flecs_query_row_mask_t flecs_query_get_row_mask(
8523285281
continue;
8523385282
}
8523485283

85235-
ecs_assert((64 * block_index) < bs->size, ECS_INTERNAL_ERROR, NULL);
85236-
ecs_flags64_t block = bs->data[block_index];
85237-
8523885284
if (not_fields & field_bit) {
8523985285
block = ~block;
8524085286
}
85287+
8524185288
mask &= block;
8524285289
has_bitset = true;
8524385290
}
@@ -85369,7 +85416,7 @@ bool flecs_query_toggle_cmp(
8536985416
block_index = op_ctx->block_index = new_block_index;
8537085417

8537185418
flecs_query_row_mask_t row_mask = flecs_query_get_row_mask(
85372-
it, table, block_index, and_fields, not_fields, op_ctx);
85419+
ctx, it, table, block_index, and_fields, not_fields, op_ctx);
8537385420

8537485421
/* If table doesn't have bitset columns, all columns match */
8537585422
if (!(op_ctx->has_bitset = row_mask.has_bitset)) {
@@ -85463,8 +85510,8 @@ bool flecs_query_toggle(
8546385510
op_ctx->prev_set_fields = it->set_fields;
8546485511
}
8546585512

85466-
ecs_flags64_t and_fields = op->first.entity & op_ctx->prev_set_fields;
85467-
ecs_flags64_t not_fields = op->second.entity & op_ctx->prev_set_fields;
85513+
ecs_flags64_t and_fields = op->first.entity;
85514+
ecs_flags64_t not_fields = op->second.entity;
8546885515

8546985516
return flecs_query_toggle_cmp(
8547085517
op, redo, ctx, and_fields, not_fields);

‎src/query/compiler/compiler.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -993,8 +993,8 @@ int flecs_query_compile(
993993
do {
994994
/* Compile remaining query terms to instructions */
995995
for (i = start_term; i < term_count; i ++) {
996-
ecs_term_t *term = &terms[i];
997996
int32_t compile = i;
997+
ecs_term_t *term = &terms[i];
998998

999999
if (compiled & (1ull << i)) {
10001000
continue; /* Already compiled */

‎src/query/engine/eval_toggle.c‎

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,21 @@ static inline int32_t flecs_ctz64(uint64_t v) {
2828
#endif
2929
}
3030

31+
static
32+
bool flecs_query_term_is_in_or_chain(
33+
const ecs_query_t *query,
34+
int32_t term_index)
35+
{
36+
ecs_term_t *terms = query->terms;
37+
int32_t term_count = query->term_count;
38+
return (terms[term_index].oper == EcsOr) ||
39+
(term_index > 0 && terms[term_index - 1].oper == EcsOr) ||
40+
((term_index + 1) < term_count && terms[term_index + 1].oper == EcsOr);
41+
}
42+
3143
static
3244
flecs_query_row_mask_t flecs_query_get_row_mask(
45+
const ecs_query_run_ctx_t *ctx,
3346
ecs_iter_t *it,
3447
ecs_table_t *table,
3548
int32_t block_index,
@@ -41,6 +54,7 @@ flecs_query_row_mask_t flecs_query_get_row_mask(
4154
int32_t i, field_count = it->field_count;
4255
ecs_flags64_t fields = and_fields | not_fields;
4356
bool has_bitset = false;
57+
const ecs_query_t *query = &ctx->query->pub;
4458

4559
for (i = 0; i < field_count; i ++) {
4660
uint64_t field_bit = 1llu << i;
@@ -57,8 +71,43 @@ flecs_query_row_mask_t flecs_query_get_row_mask(
5771
}
5872

5973
ecs_id_t id = it->ids[i];
74+
ecs_flags64_t block = 0;
75+
bool field_has_bitset = false;
76+
6077
ecs_bitset_t *bs = flecs_table_get_toggle(table, id);
61-
if (!bs) {
78+
if (bs) {
79+
ecs_assert((64 * block_index) < bs->size, ECS_INTERNAL_ERROR, NULL);
80+
block = bs->data[block_index];
81+
field_has_bitset = true;
82+
}
83+
84+
int32_t t;
85+
for (t = 0; t < query->term_count; t ++) {
86+
ecs_term_t *term = &query->terms[t];
87+
if (!(term->flags_ & EcsTermIsToggle)) {
88+
continue;
89+
}
90+
if (term->field_index != i) {
91+
continue;
92+
}
93+
if (!flecs_query_term_is_in_or_chain(query, t)) {
94+
continue;
95+
}
96+
if (term->id == id) {
97+
continue;
98+
}
99+
100+
ecs_bitset_t *or_bs = flecs_table_get_toggle(table, term->id);
101+
if (!or_bs) {
102+
continue;
103+
}
104+
105+
ecs_assert((64 * block_index) < or_bs->size, ECS_INTERNAL_ERROR, NULL);
106+
block |= or_bs->data[block_index];
107+
field_has_bitset = true;
108+
}
109+
110+
if (!field_has_bitset) {
62111
if (not_fields & field_bit) {
63112
if (op_ctx->prev_set_fields & field_bit) {
64113
has_bitset = false;
@@ -68,12 +117,10 @@ flecs_query_row_mask_t flecs_query_get_row_mask(
68117
continue;
69118
}
70119

71-
ecs_assert((64 * block_index) < bs->size, ECS_INTERNAL_ERROR, NULL);
72-
ecs_flags64_t block = bs->data[block_index];
73-
74120
if (not_fields & field_bit) {
75121
block = ~block;
76122
}
123+
77124
mask &= block;
78125
has_bitset = true;
79126
}
@@ -205,7 +252,7 @@ bool flecs_query_toggle_cmp(
205252
block_index = op_ctx->block_index = new_block_index;
206253

207254
flecs_query_row_mask_t row_mask = flecs_query_get_row_mask(
208-
it, table, block_index, and_fields, not_fields, op_ctx);
255+
ctx, it, table, block_index, and_fields, not_fields, op_ctx);
209256

210257
/* If table doesn't have bitset columns, all columns match */
211258
if (!(op_ctx->has_bitset = row_mask.has_bitset)) {
@@ -299,8 +346,8 @@ bool flecs_query_toggle(
299346
op_ctx->prev_set_fields = it->set_fields;
300347
}
301348

302-
ecs_flags64_t and_fields = op->first.entity & op_ctx->prev_set_fields;
303-
ecs_flags64_t not_fields = op->second.entity & op_ctx->prev_set_fields;
349+
ecs_flags64_t and_fields = op->first.entity;
350+
ecs_flags64_t not_fields = op->second.entity;
304351

305352
return flecs_query_toggle_cmp(
306353
op, redo, ctx, and_fields, not_fields);

‎test/query/project.json‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2237,7 +2237,12 @@
22372237
"this_sort",
22382238
"this_table_move_2_from_3",
22392239
"toggle_0_src_only_term",
2240-
"toggle_0_src"
2240+
"toggle_0_src",
2241+
"or_toggle_first_branch_matches",
2242+
"or_toggle_second_branch_matches",
2243+
"or_toggle_third_branch_matches",
2244+
"or_toggle_all_branches_match_once",
2245+
"or_toggle_no_branches_match"
22412246
]
22422247
}, {
22432248
"id": "Sparse",

‎test/query/src/Toggle.c‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6165,3 +6165,128 @@ void Toggle_toggle_0_src(void) {
61656165

61666166
ecs_fini(world);
61676167
}
6168+
6169+
static
6170+
void Toggle_or_toggle_count_matches(ecs_iter_t *it) {
6171+
int32_t *match_count = it->ctx;
6172+
*match_count += it->count;
6173+
}
6174+
6175+
static
6176+
ecs_entity_t Toggle_or_toggle_setup(
6177+
ecs_world_t *world,
6178+
int32_t *match_count)
6179+
{
6180+
ECS_COMPONENT(world, Position);
6181+
ECS_COMPONENT(world, Velocity);
6182+
ECS_COMPONENT(world, Mass);
6183+
ECS_TAG(world, HasChangedPosition);
6184+
ECS_TAG(world, HasChangedRotation);
6185+
ECS_TAG(world, HasChangedScale);
6186+
6187+
ecs_add_id(world, HasChangedPosition, EcsCanToggle);
6188+
ecs_add_id(world, HasChangedRotation, EcsCanToggle);
6189+
ecs_add_id(world, HasChangedScale, EcsCanToggle);
6190+
6191+
ecs_system(world, {
6192+
.query.terms = {
6193+
{ .id = ecs_id(Position) },
6194+
{ .id = ecs_id(Velocity) },
6195+
{ .id = ecs_id(Mass) },
6196+
{ .id = HasChangedPosition },
6197+
{ .id = HasChangedRotation, .oper = EcsOr },
6198+
{ .id = HasChangedScale, .oper = EcsOr }
6199+
},
6200+
.query.cache_kind = cache_kind,
6201+
.callback = Toggle_or_toggle_count_matches,
6202+
.ctx = match_count
6203+
});
6204+
6205+
ecs_entity_t e = ecs_entity(world, { .name = "e" });
6206+
ecs_set(world, e, Position, {10, 20});
6207+
ecs_set(world, e, Velocity, {1, 2});
6208+
ecs_set(world, e, Mass, 3);
6209+
ecs_add(world, e, HasChangedPosition);
6210+
ecs_add(world, e, HasChangedRotation);
6211+
ecs_add(world, e, HasChangedScale);
6212+
6213+
ecs_enable_id(world, e, HasChangedPosition, false);
6214+
ecs_enable_id(world, e, HasChangedRotation, false);
6215+
ecs_enable_id(world, e, HasChangedScale, false);
6216+
6217+
return e;
6218+
}
6219+
6220+
void Toggle_or_toggle_first_branch_matches(void) {
6221+
ecs_world_t *world = ecs_mini();
6222+
int32_t match_count = 0;
6223+
ecs_entity_t e = Toggle_or_toggle_setup(world, &match_count);
6224+
6225+
ecs_progress(world, 0);
6226+
test_int(0, match_count);
6227+
6228+
match_count = 0;
6229+
ecs_enable_id(world, e, HasChangedPosition, true);
6230+
ecs_progress(world, 0);
6231+
test_int(1, match_count);
6232+
6233+
ecs_fini(world);
6234+
}
6235+
6236+
void Toggle_or_toggle_second_branch_matches(void) {
6237+
ecs_world_t *world = ecs_mini();
6238+
int32_t match_count = 0;
6239+
ecs_entity_t e = Toggle_or_toggle_setup(world, &match_count);
6240+
6241+
ecs_progress(world, 0);
6242+
test_int(0, match_count);
6243+
6244+
match_count = 0;
6245+
ecs_enable_id(world, e, HasChangedRotation, true);
6246+
ecs_progress(world, 0);
6247+
test_int(1, match_count);
6248+
6249+
ecs_fini(world);
6250+
}
6251+
6252+
void Toggle_or_toggle_third_branch_matches(void) {
6253+
ecs_world_t *world = ecs_mini();
6254+
int32_t match_count = 0;
6255+
ecs_entity_t e = Toggle_or_toggle_setup(world, &match_count);
6256+
6257+
ecs_progress(world, 0);
6258+
test_int(0, match_count);
6259+
6260+
match_count = 0;
6261+
ecs_enable_id(world, e, HasChangedScale, true);
6262+
ecs_progress(world, 0);
6263+
test_int(1, match_count);
6264+
6265+
ecs_fini(world);
6266+
}
6267+
6268+
void Toggle_or_toggle_all_branches_match_once(void) {
6269+
ecs_world_t *world = ecs_mini();
6270+
int32_t match_count = 0;
6271+
ecs_entity_t e = Toggle_or_toggle_setup(world, &match_count);
6272+
6273+
ecs_enable_id(world, e, HasChangedPosition, true);
6274+
ecs_enable_id(world, e, HasChangedRotation, true);
6275+
ecs_enable_id(world, e, HasChangedScale, true);
6276+
6277+
ecs_progress(world, 0);
6278+
test_int(1, match_count);
6279+
6280+
ecs_fini(world);
6281+
}
6282+
6283+
void Toggle_or_toggle_no_branches_match(void) {
6284+
ecs_world_t *world = ecs_mini();
6285+
int32_t match_count = 0;
6286+
Toggle_or_toggle_setup(world, &match_count);
6287+
6288+
ecs_progress(world, 0);
6289+
test_int(0, match_count);
6290+
6291+
ecs_fini(world);
6292+
}

0 commit comments

Comments
 (0)