Page MenuHome

Apply random selection factor precisely for particle select.
Needs ReviewPublic

Authored by Matias Piipari (mz2) on Jan 6 2022, 11:47 PM.

Details

Summary

The "select random" action now samples predictably the number of paths / keypoints determined by the ratio parameter.

Part of T87228

Diff Detail

Repository
rB Blender
Branch
fix-T87228-for-particles (branched from master)
Build Status
Buildable 19821
Build 19821: arc lint + arc unit

Event Timeline

Matias Piipari (mz2) requested review of this revision.Jan 6 2022, 11:47 PM
Matias Piipari (mz2) created this revision.
Aaron Carlisle (Blendify) retitled this revision from Fixes https://developer.blender.org/T87228 for particles (`PARTICLE_OT_select_random`) to Apply random selection factor precisely for particle select..Jan 7 2022, 3:01 AM
Aaron Carlisle (Blendify) edited the summary of this revision. (Show Details)
Aaron Carlisle (Blendify) added inline comments.
source/blender/editors/physics/particle_edit.c
2109–2110

Only quickly passing through, Blender code stye does not use braces in switch cases.

Matias Piipari (mz2) updated this revision to Diff 46719.EditedJan 7 2022, 9:25 AM

Style tweaks to address review feedback

I changed the implementation from switch-case to if/else given braces with switch-cases not part of Blender conventions, and the variable declarations without them got messy with a switch-case (without braces would have had to declare the variables outside, and as the cases are quite different, that would turn extra messy).

source/blender/editors/physics/particle_edit.c
2109–2110

Thanks, missed this. I've changed this to an if / else, since the business of variable declarations otherwise got messy (the cases have different needs).

2131

This first pass through the visible keys of visible points was done to determine the allocation size for the array below. I was not sure if this was necessary or if I could get this specific count from somewhere from a cache?

2135

I was not sure about the naming convention for a local struct definition like this. I just added it since it felt to me easier to follow the logic of sizing and shuffling the array below with it rather than without it.

Hans Goudey (HooglyBoogly) added inline comments.
source/blender/editors/physics/particle_edit.c
2109–2110

This isn't true, switch statements with braces are used in many places. The rule is, use braces if you declare variables inside the switch case to create a proper scope for them.

source/blender/editors/physics/particle_edit.c
2109–2110

Ah sorry about that I haven't come across that, TBH I prefer how the current code is written with if statements but I have no strong preference.

I don't like using switch statements that have long code blocks for each case but that is a personal preference.

Matias Piipari (mz2) updated this revision to Diff 46778.EditedJan 9 2022, 3:19 PM

Drops a stray whitespace change, and back to a switch-case with braces so that local variables can be declared inside the cases.

Matias Piipari (mz2) marked 3 inline comments as done.Jan 14 2022, 9:05 PM
Matias Piipari (mz2) added inline comments.
source/blender/editors/physics/particle_edit.c
2109–2110

Yep, same here. However, seems like the consensus is on switch-case, so moved to that.

2138

I was not sure about the naming convention for a local struct definition like this. I just added it since it felt to me easier to follow the logic of sizing and shuffling the array below with it rather than without it.

(Repeated the comment since I'd force pushed into the branch and the earlier comment thusly got misplaced)