Yeah, I think knowing how this file was created would probably be generally helpful though; as a follow up outside of this we probably want to warn the user in some way if the file is invalid.
@…
I think that's a fine tradeoff then, my concern was mostly a hypothetical about preventing possible future bugs, and we can probably achieve that in a number of ways outside of just asserts.
Fair point about it being an implementation detail.
Forgot to delete this - answered my own question after looking through this.
I noticed that in paint_vertex.cc
we have a call to update_bounds
without a corresponding tag; might want to see if there's some weird dependency there.
I don't know if I agree with shrinking the array on each invocation of this method; I think combined with the fact that update_bounds
exits early & doesn't assert on pbvh.bounds_dirty_
being empty might make a class of errors hard to find.
Nit: While I agree with the cleanup here, I think it's worth pulling these index changes out of this PR and into a separate commit.
Does the dirty_
prefix indicate that this value isn't something we expect to be valid across certain execution contexts?
Nit: Probably good to reference update_bounds
here so that it's bidirectional. I'd consider mentioning this does lazy allocation of the vector?
Ah right, missed that we're not preallocating these
Do you have a general idea of the grouping for the other flags? I see the benefit here & possibly for visibility too to have them as separate Vector<bool>
entities, but I wonder about the memory…
I must have had preferences saved from some older version - closing this