Re: NSView patch

[Matt Rice]

On Mon, Feb 23, 2009 at 2:46 AM, Nicola Pero
<address@hidden> wrote:
> The following comment from the Apple documentation might be relevant here:
>
> "Note: For performance reasons, Cocoa does not enforce clipping among
> sibling views or guarantee correct invalidation and drawing behavior when
> sibling views overlap. If you want a view to be drawn in front of another
> view, you should make the front view a subview (or descendant) of the rear
> view."
>
> http://developer.apple.com/documentation/Cocoa/Conceptual/CocoaViewsGuide/WorkingWithAViewHierarchy/chapter_5_section_5.html#//apple_ref/doc/uid/TP40002978-CH4-SW16
>
> We may decide we're happy with that behaviour too, and document this as not
> working ;-)
>

I don't exactly consider that a "it doesn't work", more of a "is not
guaranteed to work",
as in, "don't shoot yourself in the foot" not "i'm going to shoot you
in the foot".

> It would still be good to get this working on GNUstep ... in which case, we
> probably need to review the algorithm used
> to draw the subviews to make sure the sibling subview order is respected
> when drawing.
>

it is, for subviews needing display they are traversed and drawn in
order and drawn,
mouse events through hitTest: are traversed in reverse order.

> I would say that the algorithm followed by GUI must be incorrect in the case
> of overlapping siblings - while a normal floating
> point mismatch in computing areas that need drawing could result in some
> areas being redrawn unnecessarily, I fail to
> see how it could cause a major part of a view to be drawn *above* the views
> above it, as in the screenshot ... unless there
> is an error in the drawing logic.  But possibly making it correct for
> overlapping siblings might make it too expensive
> in the normal case of nested views ? (that's what the Apple documentation
> suggests)
>

because of the floating point mismatch, whole views are not set as not
needing display after drawing

this appears to be because of  these cases in NSIntersectionRect
called from numerous places in the display machinery

      if (NSMaxX(aRect) >= NSMaxX(bRect))
        rect.size.width = NSMaxX(bRect) - rect.origin.x;
      else
        rect.size.width = NSMaxX(aRect) - rect.origin.x;

      if (NSMaxY(aRect) >= NSMaxY(bRect))
        rect.size.height = NSMaxY(bRect) - rect.origin.y;
      else
        rect.size.height = NSMaxY(aRect) - rect.origin.y;

so in this case it depends on the location origin of the view in the
superview, and if that result is representable exactly.
views which do, are marked as drawn, and views which don't are left as
needing display.


unfortunately for our unsuspecting view, subviews which were at some
odd coordinate were left as needing display,  when their super views
noticed that they were still marked as needing display and redisplayed
only those. (in case a view calls -setNeedsDisplay: on something while
in drawRect:)

everything draws and appears to work correctly until you get a
mouseEvent, then hitTest: goes in reverse order,
finds last view at those coordinates, and gives it a mouse event...
now if the user clicked on something which
was incorrectly marked as not needing display, and there is a view
which was correctly marked as displayed
above it, the mouse event goes to an NSView which appears to be below
it, which in this case frames the thing it selected and sets it as
needing display in the frames rectangle and the previous selections
frame (which there was no previous selection so it is not shown).

so thats what your seeing in the screenshot,

> Anyway Matt is probably right that the situation of overlapping subviews
> (where views are swapped up/down by using the subview
> sorting function) has never been actually used/tested. :-(

it used to work.

> I guess someone would need to spend enough time to understand the code fully
> ;-)
>
> Thanks
>
> On 23 Feb 2009, at 10:34, Richard Frith-Macdonald wrote:
>
>> While I have some sympathy with Matt's view below (that changing the
>> behavior of NSEqualRects() is a simple/elegant fix for his problem), I'm not
>> sure it's the right thing to do.
>>
>> Clearly, with an exact test for equality, rectangles (and points and
>> sizes) are not going to compare as equal once they have been through a few
>> coordinate mappings (eg when we convert from one view to another).
>>
>> That obviously seems to make a case for using slightly fuzzy comparisons,
>> but MacOS-X seems to use exact comparisons, just like our existing code, so
>> changing our implementation might break things which depend on exact
>> matching, and might get us complaints/bug reports from anyone converting
>> from MacOS-X to GNUstep.
>>
>> It seems to me that what we should really be doing is keeping our
>> calculations as exact as possible, but doing appropriate fuzzy comparisons
>> when we are trying to match rectangles and point coordinates.  However, the
>> nature of an 'appropriate' comparison could vary depending on the
>> device/context we are drawing to.  If we are working in an abstract 'points'
>> based coordinate system, but that is going to be converted to a screen or a
>> printer with a specific resolution, we must take care to use appropriate
>> rounding (not so fuzzy that the end result is out by a pixcel for instance).
>>
>> Should we be using private functions to do fuzzy comparisons rather than
>> using NSEqual... functions,
>> eg. GSAlmostEqualRects(r1, r2, precision)
>> where we adjust the precision depending on the drawing context?
>>
>> Or is Matt's solution, with a fixed precision (assumed to be fine enough
>> for all real world output devices) better?
>>
>>
>> Begin forwarded message:
>>
>>> From: Matt Rice <address@hidden>
>>> Date: 23 February 2009 08:57:32 GMT
>>> To: Richard Frith-Macdonald <address@hidden>
>>> Cc: GNUstep Developer <address@hidden>
>>> Subject: Re: NSView patch
>>>
>>> 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...
>>>
>>
>>
>>
>> _______________________________________________
>> Gnustep-dev mailing list
>> address@hidden
>> http://lists.gnu.org/mailman/listinfo/gnustep-dev
>
>
>
> _______________________________________________
> Gnustep-dev mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/gnustep-dev
>