Skip to content

Comments

Update bounds on NSClipView in setFrame only when frame value has changed#313

Merged
fredkiefer merged 1 commit intognustep:masterfrom
williameveretteggplant:experiment_NSClipView_bounds
Nov 19, 2024
Merged

Update bounds on NSClipView in setFrame only when frame value has changed#313
fredkiefer merged 1 commit intognustep:masterfrom
williameveretteggplant:experiment_NSClipView_bounds

Conversation

@williameveretteggplant
Copy link
Contributor

We are seeing a loop occur where the NSTextFieldCell calls _drawEditorWithFrame:inView: which then calls setFrame: on the NSClipView which then sets the boundsOrigin on the NSClipView based on contrainScrollPoint:. The problem is the constrainScrollPoint: causes the origin to shift by 0.5, so then needsDisplay is set on the clip view, which triggers another draw, and then constrainsScrollPoint: shifts the origin back by 0.5, so it does it again, and again...

The frame being set, however, is the same every time. So this just checks if the frame is actually a new frame, and if not it treats the call as no-op, which is what the super call to setFrame: does.

@fredkiefer
Copy link
Member

Sorry for not repeating. I am still in the process of trying to understand why this could lead to an offset of 0.5. If possible we should try to remove that and not just work around it.

@williameveretteggplant
Copy link
Contributor Author

It converts from one view coordinate system to another. Then it rounds, then it converts back again. If there is an offset of 0.5 between the two view systems you will get that adjustment of 0.5, then it gets adjusted back again. wash, rinse, repeat. Basically, you can't control all the variables in the system to guarantee that it never gets in a loop. The need to guard against it here. Basically, you make the adjustment when you first change the frame. After that, if the frame doesn't change, why go through all that logic again?

@williameveretteggplant williameveretteggplant force-pushed the experiment_NSClipView_bounds branch from bd5b014 to 5ca02c9 Compare November 4, 2024 17:46
@johnathan-becker
Copy link
Contributor

@fredkiefer I know this needs to get investigated but for now it seems to fix the issue Is there anyway to get this merged for now and maybe add a follow up issue to look into the root cause?

Copy link
Member

@fredkiefer fredkiefer left a comment

Choose a reason for hiding this comment

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

Let's merge this as a quick fix for the issue. One can argue that these checks reflect the code in NSView -setFrame: where further actions are also only taken if either the origin or the size change.

@fredkiefer fredkiefer merged commit 7855252 into gnustep:master Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants