Re: NSView patch

[Matt Rice]

On Sun, Feb 22, 2009 at 10:59 PM, Richard Frith-Macdonald
<address@hidden> wrote:
>
> On 22 Feb 2009, at 21:31, Matt Rice wrote:
>
>> On Sun, Feb 22, 2009 at 8:29 AM, Matt Rice <address@hidden> wrote:
>>>
>>> this just makes debugging a bit easier if you guys want it...
>>>
>>> bug #25658 appears to be a bug in the NSView display stuff,
>>> because some random subset of a views subviews which don't need display
>>> are getting drawRect: called multiple times through _handleAutodisplay
>>> even though they needs_display = NO;
>>> with overlapping subviews this causes views which are below other
>>> views to be drawn above views which are above them.
>>> and its kind of a pain to debug when this flag is being set all over the
>>> place.
>>
>>
>> and here is a fix for the bug i was tracking down....
>
> Please can you explain how this fixes the bug (what the actual bug is).  The
> reason I ask is that, though the idea of making NSEqualRects() consider
> slightly different rects to be equal seems fairly reasonable, it does not
> seem to be how it's implemented on MacOS-X.
> I found this by writing some tests to determine, by trial and error, the
> largest difference between two constants that was still considered equal in
> NSEqualPoints(), NSEqualSizes() and NSEqualRects() on MacOS-X, then changed
> the code on GNUstep to make it easy to set a breakpoint and examine the
> actual float values used.  When I did that I found that the point where
> values began to be considered equal was the point where the compiler made
> the two constants into identical floats.  ie. MacOS-X seems to be doing the
> same as our existing implementation and testing for exact float equality.
>
> If making NSEqualRects() fuzzy about its test for equality fixes your
> problem, perhaps the issue is in the way the function is being used
> somewhere?
>

the bug is that in NSView.m ~2400 in my patched version
the first place that _rFlags.needs_display is set to NO, in
-displayRectIgnoringOpacity:inContext:
inside of the if (NSEqualRects(...) == YES) call
if the 2 rects differ slightly such as 169.9999996... and 170 the view
will never be set as not needing display

then when it gets back to the super-view, a couple of the subviews are
still marked as needing display
so it goes through and draws those again, and subviews appear drawn
outside of the order of _sub_views.
set with the sortSubviewsUsingFunction:context:

then when the subviews handle mouse events, the view which receives
the mouse event is not the view which you appear to be clicking on,
but a view below it in the _sub_view order
leading to the behaviour in the screenshot.

https://savannah.gnu.org/file/dbmodeler_diagram_view.png?file_id=17494
attached to http://savannah.gnu.org/bugs/?25658

attached are some gdb logs (sources modified a bit for convenience)
$1 $2 and $3 are
aRect, neededRect, and NSUnionRect(neededRect, aRect)
in that order.

it also shows the drawRect: being called twice,
why the actual values slightly differ i hadn't tried to figure out as
I just figured NSEqualRects should handle it (and pretty much still
do)
but I can tell you it is non-deterministic, the same view with the
same width/height moved to 2 different locations by setting the frame
origin may cause it to start exhibiting the behaviour even though the
width/height never changed.

so it is possible that this may be a common issue but since most views
do not overlap the multiple calls to drawRect: went unnoticed.
i'll try reproducing it with Gorm, and see how that goes...


>> note that tests/Testing.h has a an EQ() macro that makes a
>> different type of comparison but I didn't exactly understand what it
>> was doing...
>
> It's testing to see if two values differ by an amount proportional to them
> rather than by an absolute amount.  This is generally what is wanted if you
> need to test floats for equality and avoid test failures due to rounding
> errors.
>
> I have some more efficient code to do the same thing ...
>
> BOOL
> almostEqual(float A, float B)
> {
> #define threshold       10      // Arbitrary fuzziness of test
>  union {int32_t i; float f;} valA, valB;
>
>  valA.f = A;
>  valB.f = B;
>
>  /*
>   * Make the values lexicographically ordered as twos-complement
>   * integers.
>   */
>  if (valA.i < 0)
>    {
>      valA.i = 0x80000000 - valA.i;
>    }
>  if (valB.i < 0)
>    {
>      valB.i = 0x80000000 - valB.i;
>    }
>  if (abs(valA.i - valB.i) < threshold)
>    {
>      return YES;
>    }
>  return NO;
> }
>
>

gdb.log.gz
Description: GNU Zip compressed data