Fix popovers one-frame-twitching in specific circumstances #6335
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closes ppy/osu#28865. The issue is caused specifically by the "arrow" part of the popover. The feedback loop involved in this is as follows: - A popover is added to the scene graph. The automatic anchor selection logic chooses an anchor and a position for it. For the arrow part specifically, this is compounded with a counteracting adjustment that ensures that the popover arrow continues to point at its target drawable: https://github.com/ppy/osu-framework/blob/691d8671be5f65f76b4800e85141bb6fde67e139/osu.Framework/Graphics/Cursor/PopoverContainer.cs#L143-L147 - The base `Popover` class uses autosize on both axes of its `BoundingBoxContainer`, whose bounds are used to calculate the best anchor: https://github.com/ppy/osu-framework/blob/691d8671be5f65f76b4800e85141bb6fde67e139/osu.Framework/Graphics/UserInterface/Popover.cs#L21 The arrow is under the bounding box container. Adjusting a drawable's position affects autosize by way of changing `RequiredParentSizeToFit`, which means that if the drawable's target is significantly off screen, the calculated autosize of the container rapidly balloons. - If the popover size increases enough, then no anchors will be considered fitting (because to the automatic layouting logic, it looks like the popover won't fit in *any* direction). Thus, the popover's anchor is changed from whatever it was on the previous frame to `Centre`, and the arrow's position is reset to zero. - The resetting of the arrow position to zero brings us back to the situation from the first point, completing the feedback cycle. To counteract this, exclude the popover arrow from participating in autosize calculations.
peppy
approved these changes
Jul 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes ppy/osu#28865.
The issue is caused specifically by the "arrow" part of the popover. The feedback loop involved in this is as follows:
A popover is added to the scene graph. The automatic anchor selection logic chooses an anchor and a position for it. For the arrow part specifically, this is compounded with a counteracting adjustment that ensures that the popover arrow continues to point at its target drawable:
osu-framework/osu.Framework/Graphics/Cursor/PopoverContainer.cs
Lines 143 to 147 in 691d867
The base
Popoverclass uses autosize on both axes of itsBoundingBoxContainer, whose bounds are used to calculate the best anchor:osu-framework/osu.Framework/Graphics/UserInterface/Popover.cs
Line 21 in 691d867
The arrow is under the bounding box container. Adjusting a drawable's position affects autosize by way of changing
RequiredParentSizeToFit, which means that if the drawable's target is significantly off screen, the calculated autosize of the container rapidly balloons.If the popover size increases enough, then no anchors will be considered fitting (because to the automatic layouting logic, it looks like the popover won't fit in any direction). Thus, the popover's anchor is changed from whatever it was on the previous frame to
Centre, and the arrow's position is reset to zero.The resetting of the arrow position to zero brings us back to the situation from the first point, completing the feedback cycle.
To counteract this, exclude the popover arrow from participating in autosize calculations.
Also includes 4367d01 which fell out in testing.