Re: Fwd: NSView patch

[Matt Rice]

On Mon, Feb 23, 2009 at 5:50 AM, Fred Kiefer <address@hidden> wrote:
> I think we have two problems here. One being the handling of overlapping
> subviews. I really don't have an idea how this should be done. We don't
> even have a definition if the first one in the subview list or a later
> on is above (or below). If Matt needs a strict definition here, he has
> to come up with a proposal, test it on MacOSX and then try to implement
> it on GNUstep.

yeah, this isn't really a problem it all works as is,
the first item in _sub_views is at the bottom
the last item in _sub_views is at the top for drawing

for hitTest:
the last item in _sub_views atis a hit.


> The other problem that we sometimes mark subviews as still needing
> display due to rounding errors should be addressed. Here it would be
> helpful to get the detailed values of the involved rectangles. These
> surely are in Matts gdb log, but as he is using a changed version of the
> NSView code it is hard for me to tell, which is which. What would help
> is to have the values of _visibleRect, _invalidRect and aRect at the
> beginning of the method displayRectIgnoringOpacity:inContext:

the superview at the beginning of the method.

Breakpoint 2, -[NSView displayRectIgnoringOpacity:inContext:]
(self=0x276ff90, _cmd=0xdc4e70, aRect={origin = {x = 0, y = 0}, size =
{width = 476, height = 381}}, context=0x299b740) at NSView.m:2368
2368      BOOL flush = NO;
$_invalidRect ={origin = {x = 0, y = 0}, size = {width = 1, height = 1}}
$_visibleRect ={origin = {x = 0, y = 0}, size = {width = 476, height = 381}}
$aRect ={origin = {x = 0, y = 0}, size = {width = 476, height = 381}}

still in the super-view but right before we go to the subview...

Breakpoint 3, -[NSView displayRectIgnoringOpacity:inContext:]
(self=0x276ff90, _cmd=0xdc4e70, aRect={origin = {x = 0, y = 0}, size =
{width = 476, height = 381}}, context=0x299b740) at NSView.m:2450
2450                  if (NSIsEmptyRect(isect) == NO)
$_invalidRect ={origin = {x = 0, y = 0}, size = {width = 0, height = 0}}
$_visibleRect ={origin = {x = 0, y = 0}, size = {width = 476, height = 381}}
$aRect ={origin = {x = 0, y = 0}, size = {width = 476, height = 381}}
$isect ={origin = {x = 48.7490425, y = 192.641785}, size = {width =
118, height = 54}}
$subviewFrame ={origin = {x = 48.7490425, y = 192.641785}, size =
{width = 118, height = 54}}

right after we go
isect = [subview convertRect: isect fromView: self];

Breakpoint 4, -[NSView displayRectIgnoringOpacity:inContext:]
(self=0x276ff90, _cmd=0xdc4e70, aRect={origin = {x = 0, y = 0}, size =
{width = 476, height = 381}}, context=0x299b740) at NSView.m:2453
2453                      [subview displayRectIgnoringOpacity: isect
inContext: context];
$new_isect ={origin = {x = 0, y = 0}, size = {width = 117.999985, height = 54}}


now here in the view where the error occurs

Breakpoint 2, -[NSView displayRectIgnoringOpacity:inContext:]
(self=0x284f7e0, _cmd=0xdc4e70, aRect={origin = {x = 0, y = 0}, size =
{width = 117.999985, height = 54}}, context=0x299b740) at
NSView.m:2368
2368      BOOL flush = NO;
$_invalidRect ={origin = {x = 0, y = 0}, size = {width = 118, height = 54}}
$_visibleRect ={origin = {x = 0, y = 0}, size = {width = 118, height = 54}}
$aRect ={origin = {x = 0, y = 0}, size = {width = 117.999985, height = 54}}

Breakpoint 1, gsbp () at NSView.m:2362


> The following computation just involves NSIntersectionRect and
> NSUnionRect, these operations should be exact. The real problem will be
> in the convertRect:fromView: call.

yep, that appears to be the case

> Matt, does any of your views use scaling or rotation?

nope

> I had hoped that
> without these our coordinate transformation should be almost correct.
> But that almost may not be enough here. What we could do in the
> conversion methods is to use the frame and bounds matrices directly,
> when one view is the immediate superview of the other. Perhaps we could
> get a higher precision that way. Even investigating that will take a lot
> of time. Not sure, whether this small issue will be worth it...
>
> Fred
>
>
> 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...
>>>
>>
>
>