search

gnustep / libs-gui

The GNUstep gui library is a library of graphical user interface classes written completely in the Objective-C language; the classes are based upon Apple's Cocoa framework (which came from the OpenStep specification). *** Larger patches require copyright assignment to FSF. please file bugs here. ***
http://www.gnustep.org
GNU General Public License v3.0
279 stars 103 forks source link

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

Closed williameveretteggplant closed 1 week ago

williameveretteggplant commented 1 month ago

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 commented 1 month ago

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 commented 4 weeks ago

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?

johnathan-becker commented 2 weeks ago

@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?