Skip to content

fix: JFXRippler 可能无法覆盖整个控件#5609

Open
CiiLu wants to merge 5 commits intoHMCL-dev:mainfrom
CiiLu:rippppplerrrrrrrrrr
Open

fix: JFXRippler 可能无法覆盖整个控件#5609
CiiLu wants to merge 5 commits intoHMCL-dev:mainfrom
CiiLu:rippppplerrrrrrrrrr

Conversation

@CiiLu
Copy link
Contributor

@CiiLu CiiLu commented Feb 21, 2026

例:版本下载页面
image

@Glavo Glavo requested a review from Copilot February 21, 2026 15:13
@CiiLu CiiLu marked this pull request as draft February 21, 2026 15:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

该 PR 旨在修复 JFXRippler 在某些较大控件(如版本下载页面)上水波纹效果半径不足、无法覆盖整个控件的问题。

Changes:

  • 调整 computeRippleRadius() 的计算方式:由“对角线长度 + 上限裁剪 + 额外裕量”改为“仅对角线长度”。

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

double width2 = control.getLayoutBounds().getWidth() * control.getLayoutBounds().getWidth();
double height2 = control.getLayoutBounds().getHeight() * control.getLayoutBounds().getHeight();
return Math.min(Math.sqrt(width2 + height2), RIPPLE_MAX_RADIUS) * 1.1 + 5;
return Math.sqrt(Math.pow(control.getLayoutBounds().getWidth(), 2) + Math.pow(control.getLayoutBounds().getHeight(), 2));
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

computeRippleRadius() now returns the exact diagonal, but the ripple grow animation only scales to 0.9 while the mouse is pressed (see Ripple.inAnimation target scale). This means the visible radius while pressed will be 0.9 * diagonal and can still fail to reach the far corner until mouse release, which undermines the goal of covering the whole control. Consider either (a) restoring a safety margin (e.g., divide by 0.9 and add a small padding) or (b) changing the in-animation target scale to 1 so the computed radius is actually reached.

Suggested change
return Math.sqrt(Math.pow(control.getLayoutBounds().getWidth(), 2) + Math.pow(control.getLayoutBounds().getHeight(), 2));
// compute the control diagonal
double width = control.getLayoutBounds().getWidth();
double height = control.getLayoutBounds().getHeight();
double diagonal = Math.sqrt(width * width + height * height);
// compensate for the in-animation max scale (0.9) and add a small padding
double safetyScale = 1.0 / 0.9;
double padding = 2.0;
return diagonal * safetyScale + padding;

Copilot uses AI. Check for mistakes.
double width2 = control.getLayoutBounds().getWidth() * control.getLayoutBounds().getWidth();
double height2 = control.getLayoutBounds().getHeight() * control.getLayoutBounds().getHeight();
return Math.min(Math.sqrt(width2 + height2), RIPPLE_MAX_RADIUS) * 1.1 + 5;
return Math.sqrt(Math.pow(control.getLayoutBounds().getWidth(), 2) + Math.pow(control.getLayoutBounds().getHeight(), 2));
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Math.pow(x, 2) here is unnecessary overhead compared to x * x, and this change also makes RIPPLE_MAX_RADIUS unused in this class (it was previously referenced in this method). Consider switching back to multiplication and either removing the unused constant or repurposing it (e.g., as an optional upper bound via the styleable ripplerRadius property) to avoid dead code.

Suggested change
return Math.sqrt(Math.pow(control.getLayoutBounds().getWidth(), 2) + Math.pow(control.getLayoutBounds().getHeight(), 2));
double width = control.getLayoutBounds().getWidth();
double height = control.getLayoutBounds().getHeight();
return Math.sqrt(width * width + height * height);

Copilot uses AI. Check for mistakes.
@CiiLu CiiLu marked this pull request as ready for review February 21, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants