-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix possible bug in MotionBlur #2168
Conversation
Reviewer's Guide by SourceryThe PR fixes a bug in the MotionBlur implementation where negative direction_range values weren't handled correctly, and updates the default direction_range to match the documentation. The fix modifies the motion kernel creation to properly handle negative directions by applying the absolute value for scaling and then negating if needed. Sequence diagram for create_motion_kernel with negative directionsequenceDiagram
participant MotionBlur
participant create_motion_kernel
MotionBlur->>create_motion_kernel: Call with direction < 0
create_motion_kernel->>create_motion_kernel: Calculate t = t * (1 + abs(direction))
create_motion_kernel->>create_motion_kernel: If direction < 0, t = t * -1
create_motion_kernel-->>MotionBlur: Return adjusted kernel
Updated class diagram for MotionBlurclassDiagram
class MotionBlur {
- ScaleIntType blur_limit
- bool allow_shifted
- tuple< float, float > angle_range
- tuple< float, float > direction_range
- bool | None always_apply
- float p
+ __init__(blur_limit, allow_shifted, angle_range, direction_range, always_apply, p)
}
note for MotionBlur "Updated default direction_range to (-0.5, 0.5)"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @huuquan1994 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
The current MotionBlur won't work if
direction_range
is set to-1.0
. This is because the create_motion_kernel does not handle that case, resulting in a "near zero" kernel (i.e., kernel with very few non-zero values).Also, I updated the default value of
direction_range
to(-0.5, 0.5)
to match the description hereI hope that my PR correctly fixes the bug. Thanks!
Summary by Sourcery
Fix the MotionBlur function to handle negative direction values and update the default direction_range for consistency.
Bug Fixes:
Enhancements: