SMDoubleSlider usability on GNUstep

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

SMDoubleSlider usability on GNUstep

Yavor Doganov-3
As part of my ongoing effort to port the latest Lynkeos release to
GNUstep, I have encountered an issue with a custom class which is a
subclass of NSSlider/NSSliderCell.  The original code is available
here [1] as .dmg only, I used 7z to unpack it.

[1] http://developer.snowmintcs.com/controls/smdoubleslider/

The Lynkeos bundled version can be obtained with Subversion [2] and is
also browseable here [3].

[2] svn co svn://svn.code.sf.net/p/lynkeos/code/trunk/application/ThirdPartySources/SMDoubleSlider SMDoubleSlider
[3] https://sourceforge.net/p/lynkeos/code/HEAD/tree/trunk/application/ThirdPartySources/SMDoubleSlider/

It seems that the original code accesses NSSliderCell private ivars
directly (_value and some struct members); they have no GNUstep
equivalent.  In 2009, the Lynkeos developer made some changes to fix
compilation on GNUstep:

https://sourceforge.net/p/lynkeos/code/490/tree//trunk/application/ThirdPartySources/SMDoubleSlider/SMDoubleSliderCell.m?diff=517068cee88f3d0a275a1c9e:489

I assume that this works on Cocoa as there were several Lynkeos
releases since then.  It builds on GNUstep but crashes:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6b61216 in objc_msg_lookup (receiver=receiver@entry=0x5555562ef7f0,
    op=op@entry=0x7ffff7c6f130 <_OBJC_SELECTOR_TABLE+400>)
    at /build/gcc-8-0DvHrl/gcc-8-8-20180218/src/libobjc/sendmsg.c:439
439 /build/gcc-8-0DvHrl/gcc-8-8-20180218/src/libobjc/sendmsg.c: Няма такъв файл или директория.
(gdb) bt
#0  0x00007ffff6b61216 in objc_msg_lookup (receiver=receiver@entry=0x5555562ef7f0, op=op@entry=0x7ffff7c6f130 <_OBJC_SELECTOR_TABLE+400>)
    at /build/gcc-8-0DvHrl/gcc-8-8-20180218/src/libobjc/sendmsg.c:439
#1  0x00007ffff77d2015 in -[NSCell doubleValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at NSCell.m:269
#2  0x00007ffff778362a in -[NSActionCell doubleValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at NSActionCell.m:187
#3  0x000055555555bfdb in -[SMDoubleSliderCell doubleHiValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at SMDoubleSliderCell.m:448
#4  0x000055555555de93 in -[SMDoubleSliderCell stringHiValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at SMDoubleSliderCell.m:494
#5  0x00007ffff77d2021 in -[NSCell doubleValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at NSCell.m:269
#6  0x00007ffff778362a in -[NSActionCell doubleValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at NSActionCell.m:187
#7  0x000055555555bfdb in -[SMDoubleSliderCell doubleHiValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at SMDoubleSliderCell.m:448
#8  0x000055555555de93 in -[SMDoubleSliderCell stringHiValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at SMDoubleSliderCell.m:494
#9  0x00007ffff77d2021 in -[NSCell doubleValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at NSCell.m:269
#10 0x00007ffff778362a in -[NSActionCell doubleValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at NSActionCell.m:187
...

The devil's circle goes on and on.  Am I right to conclude that
SMDoubleSlider is tightly tied to Apple's implementation and cannot
possibly work on GNUstep?  Is there anything I can do except to
disable this functionality?


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: SMDoubleSlider usability on GNUstep

David Chisnall-7
On 20 Feb 2018, at 12:33, Yavor Doganov <[hidden email]> wrote:
>
> #1  0x00007ffff77d2015 in -[NSCell doubleValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at NSCell.m:269
> #2  0x00007ffff778362a in -[NSActionCell doubleValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at NSActionCell.m:187
> #3  0x000055555555bfdb in -[SMDoubleSliderCell doubleHiValue] (self=0x5555562ef7f0, _cmd=<optimized out>) at SMDoubleSliderCell.m:448

Skimming the code, it looks as if their -doubleValue method calls their -doubleHiValue method, which calls the superclass’s -doubleValue method.  In GNUstep, the superclass has another call to its superclass method:

https://github.com/gnustep/libs-gui/blob/master/Source/NSActionCell.m#L187

This then checks whether the object responds to -doubleValue, and if it does calls that:

https://github.com/gnustep/libs-gui/blob/master/Source/NSCell.m#L265

Unfortunately, in this case, it appears that the object value is self, so you get infinite recursion.

This looks like a bug elsewhere, as setting a cell’s object value to the cell itself is likely to cause problems.  I don’t see any calls to -setObjectValue: in the SMDoubleSlider code that you’ve linked to, so something else must be doing this.  It looks quite a bit like a use-after-free bug.

David


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: SMDoubleSlider usability on GNUstep

Yavor Doganov-3
David Chisnall wrote:
> This then checks whether the object responds to -doubleValue, and if
> it does calls that:
>
> https://github.com/gnustep/libs-gui/blob/master/Source/NSCell.m#L265
>
> Unfortunately, in this case, it appears that the object value is
> self, so you get infinite recursion.

I think this condition is always false.  _cell.has_valid_object_value
is NO and _object_value is nil.  So it jumps to NSCell.m:269 and
SMDoubleSliderCell's -stringValue is called which calls -stringHiValue
which in turn calls -doubleHiValue and from there the infinite
recursion is in place.

At least this is what I observe in the debugger.

_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: SMDoubleSlider usability on GNUstep

David Chisnall-7
On 20 Feb 2018, at 14:30, Yavor Doganov <[hidden email]> wrote:
>
> I think this condition is always false.  _cell.has_valid_object_value
> is NO and _object_value is nil.  So it jumps to NSCell.m:269 and
> SMDoubleSliderCell's -stringValue is called which calls -stringHiValue
> which in turn calls -doubleHiValue and from there the infinite
> recursion is in place.
>
> At least this is what I observe in the debugger.

Ah, that makes sense - I’d missed that in the trace.  So now the question is what happens when you call -doubleValue on an NSCell in Cocoa when it has a string value?  Here’s a little test program that finds out:

#import <Cocoa/Cocoa.h>

@interface Proxy : NSProxy
{
        @public
        id obj;
}
@end

@implementation Proxy
- (BOOL)respondsToSelector: (SEL)aSelector
{
        return [obj respondsToSelector: aSelector];
}
- (NSMethodSignature*)methodSignatureForSelector: (SEL)aSelector
{
        return [obj methodSignatureForSelector: aSelector];
}
- (void)forwardInvocation: (NSInvocation*)anInvocation
{
        NSLog(@"%@ set to proxy", anInvocation);
        [anInvocation invokeWithTarget: obj];
}
@end

@interface Test : NSActionCell @end
@implementation Test
- (float)floatValue
{
        NSLog(@"-floatValue called\n");
        return [super floatValue];
}
- (NSString*)stringValue
{
        NSLog(@"-stringValue called\n");
        return [super stringValue];
}
@end

int main(void)
{
        @autoreleasepool
        {
                Test *t = [Test new];
                Proxy *p = [Proxy alloc];
                p->obj = @"0.23";
                [t setObjectValue: t];
                NSLog(@"Querying");
                NSLog(@"%f", [t floatValue]);
        }
}

The output is:

2018-02-20 14:46:39.917 a.out[85231:11731363] Querying
2018-02-20 14:46:39.917 a.out[85231:11731363] -floatValue called
2018-02-20 14:46:39.917 a.out[85231:11731363] 0.000000


So, from this we learn that Cocoa’s NSCell implementation doesn’t call any methods on either itself or the object to find the floating point value.  This is a bit odd, but at the very least we should fix GNUstep’s NSCell to query the object and not itself to find the string value.  The correct fix is probably:

- (float) floatValue
{
  if (_cell.has_valid_object_value == YES)
  {
    if ([_object_value respondsToSelector: @selector(floatValue)]))
    {
      return [_object_value floatValue];
    }
    if ([_object_value respondsToSelector: @selector(stringValue)]))
    {
      return [[_object_value stringValue] floatValue];
    }
  }
  return 0;
}

And apply similar fixes to the other *Value methods in NSCell.

You can hack around this brokenness by including the above method in a category on NSCell in your application (though please remember to remove it once GNUstep is fixed!).

David


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: SMDoubleSlider usability on GNUstep

Fred Kiefer
David‘s analysis sounds correct to me. I will change that code when I am back home next week.

Fred

Unterwegs

> Am 20.02.2018 um 15:55 schrieb David Chisnall <[hidden email]>:
>
>> On 20 Feb 2018, at 14:30, Yavor Doganov <[hidden email]> wrote:
>>
>> I think this condition is always false.  _cell.has_valid_object_value
>> is NO and _object_value is nil.  So it jumps to NSCell.m:269 and
>> SMDoubleSliderCell's -stringValue is called which calls -stringHiValue
>> which in turn calls -doubleHiValue and from there the infinite
>> recursion is in place.
>>
>> At least this is what I observe in the debugger.
>
> Ah, that makes sense - I’d missed that in the trace.  So now the question is what happens when you call -doubleValue on an NSCell in Cocoa when it has a string value?  Here’s a little test program that finds out:
>
> #import <Cocoa/Cocoa.h>
>
> @interface Proxy : NSProxy
> {
>    @public
>    id obj;
> }
> @end
>
> @implementation Proxy
> - (BOOL)respondsToSelector: (SEL)aSelector
> {
>    return [obj respondsToSelector: aSelector];
> }
> - (NSMethodSignature*)methodSignatureForSelector: (SEL)aSelector
> {
>    return [obj methodSignatureForSelector: aSelector];
> }
> - (void)forwardInvocation: (NSInvocation*)anInvocation
> {
>    NSLog(@"%@ set to proxy", anInvocation);
>    [anInvocation invokeWithTarget: obj];
> }
> @end
>
> @interface Test : NSActionCell @end
> @implementation Test
> - (float)floatValue
> {
>    NSLog(@"-floatValue called\n");
>    return [super floatValue];
> }
> - (NSString*)stringValue
> {
>    NSLog(@"-stringValue called\n");
>    return [super stringValue];
> }
> @end
>
> int main(void)
> {
>    @autoreleasepool
>    {
>        Test *t = [Test new];
>        Proxy *p = [Proxy alloc];
>        p->obj = @"0.23";
>        [t setObjectValue: t];
>        NSLog(@"Querying");
>        NSLog(@"%f", [t floatValue]);
>    }
> }
>
> The output is:
>
> 2018-02-20 14:46:39.917 a.out[85231:11731363] Querying
> 2018-02-20 14:46:39.917 a.out[85231:11731363] -floatValue called
> 2018-02-20 14:46:39.917 a.out[85231:11731363] 0.000000
>
>
> So, from this we learn that Cocoa’s NSCell implementation doesn’t call any methods on either itself or the object to find the floating point value.  This is a bit odd, but at the very least we should fix GNUstep’s NSCell to query the object and not itself to find the string value.  The correct fix is probably:
>
> - (float) floatValue
> {
>  if (_cell.has_valid_object_value == YES)
>  {
>    if ([_object_value respondsToSelector: @selector(floatValue)]))
>    {
>      return [_object_value floatValue];
>    }
>    if ([_object_value respondsToSelector: @selector(stringValue)]))
>    {
>      return [[_object_value stringValue] floatValue];
>    }
>  }
>  return 0;
> }
>
> And apply similar fixes to the other *Value methods in NSCell.
>
> You can hack around this brokenness by including the above method in a category on NSCell in your application (though please remember to remove it once GNUstep is fixed!).
>
> David
>
>
> _______________________________________________
> Discuss-gnustep mailing list
> [hidden email]
> https://lists.gnu.org/mailman/listinfo/discuss-gnustep


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: SMDoubleSlider usability on GNUstep

Yavor Doganov-3
In reply to this post by David Chisnall-7
David Chisnall wrote:
> Here’s a little test program that finds out:

Thanks, that was really helpful.  And it's not surprising that on
GNUstep your test program causes infinite recursion here:

> 2018-02-20 14:46:39.917 a.out[85231:11731363] -floatValue called

> The correct fix is probably:
[...]
> And apply similar fixes to the other *Value methods in NSCell.

I followed this advice and now another infinite recursion occurs:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6096e79 in _int_malloc (av=av@entry=0x7ffff63c7c20 <main_arena>,
    bytes=bytes@entry=32) at malloc.c:3575
3575 malloc.c: Няма такъв файл или директория.
(gdb) bt
#0  0x00007ffff6096e79 in _int_malloc (av=av@entry=0x7ffff63c7c20 <main_arena>, bytes=bytes@entry=32) at malloc.c:3575
#1  0x00007ffff6098dc3 in __GI___libc_malloc (bytes=32) at malloc.c:3051
#2  0x00007ffff70bbd4c in default_malloc (zone=<optimized out>, size=<optimized out>) at NSZone.m:122
#3  0x00007ffff70107d8 in NSAllocateObject (aClass=0x7ffff74e0a60 <_OBJC_Class_NSDoubleNumber>, extraBytes=extraBytes@entry=0, zone=
    0x7ffff7545460 <default_zone>, zone@entry=0x0) at NSObject.m:782
#4  0x00007ffff70074d0 in +[NSNumber numberWithDouble:] (self=<optimized out>, _cmd=<optimized out>, aValue=<optimized out>) at NSNumber.m:948
#5  0x00007ffff77d4d0f in -[NSCell setDoubleValue:] (self=0x5555562f15b0, _cmd=<optimized out>, aDouble=0) at NSCell.m:410
#6  0x000055555555dc8b in -[SMDoubleSliderCell setDoubleHiValue:] (self=0x5555562f15b0, _cmd=<optimized out>, aDouble=0) at SMDoubleSliderCell.m:487
#7  0x000055555555dc8b in -[SMDoubleSliderCell setDoubleHiValue:] (self=0x5555562f15b0, _cmd=<optimized out>, aDouble=0) at SMDoubleSliderCell.m:487
...

NSSliderCell's -init calls -setDoubleValue: which in turn calls
-setDoubleHiValue: and at the end it calls the superclass'
-setDoubleValue:.  NSCell's -setDoubleValue: calls -setObjectValue:
but it's the SMDoubleSliderCell's method that is used which resorts to
-setDoubleValue: again (calling [super setDoubleValue:]).

I turned NSCell's -setObjectValue: to a private method and changed all
-set*Value: methods to use it.  My test program doesn't crash now and
I can see the two knobs.  Movement is awkward (no sliding effect) and
if I click on the first knob the second one disappears.  But at least
there is some hope.  There's still something fishy going on but
unfortunately I don't understand the code well enough to figure it
out.

Is it a problem that SMDoubleSliderCell overrides both -drawKnob: and
-drawKnob?

_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: SMDoubleSlider usability on GNUstep

Yavor Doganov-3
В Tue, 20 Feb 2018 23:13:21 +0200, Yavor Doganov написа:

> I turned NSCell's -setObjectValue: to a private method and changed all
> -set*Value: methods to use it.  My test program doesn't crash now and I
> can see the two knobs.  Movement is awkward (no sliding effect) and if I
> click on the first knob the second one disappears.

I reverted this change as it is causing the awkwardness, among other
problems.  Instead, I disabled SMDoubleSliderCell's -setObjectValue:
so that NSCell's method is always used.  It seems to be working
properly except:

1. When the high knob is not set to a particular value with
-set*HiValue: it is not displayed initially.  However, if I click on
the right side of the bar both knobs become visible.

2. Possibly related with the above issue, if I click on the low knob,
the high knob disappears.  It is visible where it should be during
NSCell -trackMouse:inRect:ofView:untilMouseUp: but as soon as the
mouse is up it disappears again.  When tracking the high knob both
knobs are visible as expected.

I believe these are GNUstep bugs or at least there is a difference in
the behavior compared to Cocoa (assuming the code works as expected on
Cocoa).

> Is it a problem that SMDoubleSliderCell overrides both -drawKnob: and
> -drawKnob?

I tried merging them into -drawKnob: only, no difference in the
behavior except that the high knob is not visible when the low knob is
being moved with the mouse.

3. When the low knob is moved at the beginning of the bar, it becomes
locked and can no longer be moved with the mouse.  This seems to be
caused by this code in SMDoubleSliderCell's -startTrackingAt:inView:

    if ( [ self trackingLoKnob ] && NSEqualRects( loKnobRect,
                [ self knobRectFlipped:[ controlView isFlipped ] ] ) )
        [ self setTrackingLoKnob:( _sm_loValue > [ self minValue ] ) ];

When _sm_loValue is 0 or lower than minValue (in cases when minValue
is set explicitly to a positive value), the boolean expression is
false and only the high knob can be tracked; the low knob remains
blocked indefinitely.  I replaced > with >= and it solves the problem,
although it has the same effect as disabling this check entirely.  It
seems bogus to me, the two knobs cannot overlap.


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: SMDoubleSlider usability on GNUstep

Fred Kiefer
In reply to this post by David Chisnall-7
As usual it turns out that things are slightly more complicated. There
is a tiny problem with David's test code, he did set "t" as its own
object value instead of using the proxy there. When correcting this I
noticed that the value from the proxy never gets used and extended the
test code slightly and finally testing with different subclasses of
NSCell, not just NSActionCell. They behave violently different. For
NSCell and NSActionCell floatValue and doubleValue always return 0.0
even when setting this value explicitly. NSSliderCell and
NSTextFieldCell behave different, but also not the same way. Looks like
we will need to investigate further and most likely will need to move
the actual value handling code down into the specific NSCell subclasses.
This sounds very ugly to me.

Fred


Am 20.02.2018 um 15:55 schrieb David Chisnall:

> On 20 Feb 2018, at 14:30, Yavor Doganov <[hidden email]> wrote:
>>
>> I think this condition is always false.  _cell.has_valid_object_value
>> is NO and _object_value is nil.  So it jumps to NSCell.m:269 and
>> SMDoubleSliderCell's -stringValue is called which calls -stringHiValue
>> which in turn calls -doubleHiValue and from there the infinite
>> recursion is in place.
>>
>> At least this is what I observe in the debugger.
>
> Ah, that makes sense - I’d missed that in the trace.  So now the question is what happens when you call -doubleValue on an NSCell in Cocoa when it has a string value?  Here’s a little test program that finds out:
>
> #import <Cocoa/Cocoa.h>
>
> @interface Proxy : NSProxy
> {
> @public
> id obj;
> }
> @end
>
> @implementation Proxy
> - (BOOL)respondsToSelector: (SEL)aSelector
> {
> return [obj respondsToSelector: aSelector];
> }
> - (NSMethodSignature*)methodSignatureForSelector: (SEL)aSelector
> {
> return [obj methodSignatureForSelector: aSelector];
> }
> - (void)forwardInvocation: (NSInvocation*)anInvocation
> {
> NSLog(@"%@ set to proxy", anInvocation);
> [anInvocation invokeWithTarget: obj];
> }
> @end
>
> @interface Test : NSActionCell @end
> @implementation Test
> - (float)floatValue
> {
> NSLog(@"-floatValue called\n");
> return [super floatValue];
> }
> - (NSString*)stringValue
> {
> NSLog(@"-stringValue called\n");
> return [super stringValue];
> }
> @end
>
> int main(void)
> {
> @autoreleasepool
> {
> Test *t = [Test new];
> Proxy *p = [Proxy alloc];
> p->obj = @"0.23";
> [t setObjectValue: t];
> NSLog(@"Querying");
> NSLog(@"%f", [t floatValue]);
> }
> }
>
> The output is:
>
> 2018-02-20 14:46:39.917 a.out[85231:11731363] Querying
> 2018-02-20 14:46:39.917 a.out[85231:11731363] -floatValue called
> 2018-02-20 14:46:39.917 a.out[85231:11731363] 0.000000
>
>
> So, from this we learn that Cocoa’s NSCell implementation doesn’t call any methods on either itself or the object to find the floating point value.  This is a bit odd, but at the very least we should fix GNUstep’s NSCell to query the object and not itself to find the string value.  The correct fix is probably:
>
> - (float) floatValue
> {
>   if (_cell.has_valid_object_value == YES)
>   {
>     if ([_object_value respondsToSelector: @selector(floatValue)]))
>     {
>       return [_object_value floatValue];
>     }
>     if ([_object_value respondsToSelector: @selector(stringValue)]))
>     {
>       return [[_object_value stringValue] floatValue];
>     }
>   }
>   return 0;
> }
>
> And apply similar fixes to the other *Value methods in NSCell.
>
> You can hack around this brokenness by including the above method in a category on NSCell in your application (though please remember to remove it once GNUstep is fixed!).
>
> David

_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: SMDoubleSlider usability on GNUstep

Fred Kiefer
Just to keep you updated. Things are even more complicated than I thought.
Here the extended code I have been using for my tests:

#import <Cocoa/Cocoa.h>

 @interface Proxy : NSProxy
 {
 @public
 id obj;
 }
 @end
 
 @implementation Proxy
 - (BOOL)respondsToSelector: (SEL)aSelector
 {
 return [obj respondsToSelector: aSelector];
 }

- (NSMethodSignature*)methodSignatureForSelector: (SEL)aSelector
 {
 return [obj methodSignatureForSelector: aSelector];
 }

 - (void)forwardInvocation: (NSInvocation*)anInvocation
 {
     NSLog(@"%@ sent to proxy", NSStringFromSelector([anInvocation selector]));
     [anInvocation invokeWithTarget: obj];
 }

 - (BOOL)isKindOfClass: (Class)clazz
 {
     NSLog(@"isKindOfClass: called with %@", clazz);
     return [obj isKindOfClass: clazz];
 }

 - (Proxy*) copyWithZone: (NSZone*)zone
 {
     Proxy *copy = [[self class] alloc];
     copy->obj = [self->obj copyWithZone: zone];
     return copy;
 }
 @end
 
 //@interface Test : NSActionCell @end
 @interface Test : NSSliderCell @end
 @implementation Test
 - (float)floatValue
 {
 NSLog(@"-floatValue called\n");
 return [super floatValue];
 }
- (double)doubleValue
{
    NSLog(@"-doubleValue called\n");
    return [super doubleValue];
}
 - (NSString*)stringValue
 {
 NSLog(@"-stringValue called\n");
 return [super stringValue];
 }
 - (id)objectValue
 {
 NSLog(@"-objectValue called\n");
 return [super objectValue];
 }
 - (void)setObjectValue:(id)objectValue
 {
 NSLog(@"-setObjectValue called\n");
 [super setObjectValue:objectValue];
 }
 - (void)setStringValue:(NSString *)stringValue
 {
 NSLog(@"-setStringValue called\n");
 [super setStringValue:stringValue];
 }
- (void)describe
{
    NSLog(@"type %d valid %d font %@", (int)[self type], [self hasValidObjectValue], [self font]);
    NSLog(@"value %lf", [self doubleValue]);
    NSLog(@"max %lf min %lf", [self maxValue], [self minValue]);
}
 @end
 
 int main(void)
 {
     @autoreleasepool
     {
         Test *t = [[Test alloc] initTextCell: @""];
         //NSImage *image = [NSImage imageNamed: NSImageNameComputer];
         //Test *t = [[Test alloc] initImageCell: image];
         //Test *t = [[Test alloc] init];
         [t describe];
         //[t setMaxValue: 100.0];
         Proxy *p = [Proxy alloc];
         p->obj = @"0.23";
         [t setObjectValue: p];
         /*
         NSLog(@"Start setFloatValue");
         [t setFloatValue: 12.4f];
          */
         NSLog(@"Start setDoubleValue");
         [t setDoubleValue: 34.5];
         NSLog(@"Querying");
         NSLog(@"%f", [t floatValue]);
         NSLog(@"%lf", [t doubleValue]);
         NSLog(@"%@", [t stringValue]);
         NSLog(@"%@", [t objectValue]);
         //NSLog(@"%d", [[t objectValue] isKindOfClass: [NSNumber class]]);
     }
 }


To me it now looks like NSSliderCell just overrides all the value methods of its super calls and implements them by setting a numerical value directly. Not the nicest code, but to be compatible we may have to do a similar rewrite.
For NSCell and specific sub classes things are different, but only if these get initialised as text cells. Doing this on an NSSliderCell would not set the max value, which prevents that cell from working.

I plan to do a full rewrite of NSSliderCell, without touching NSCell at all. This should be enough to get  SMDoubleSlider working under GNUstep.

Cheers,
Fred


> Am 26.02.2018 um 23:42 schrieb Fred Kiefer <[hidden email]>:
>
> As usual it turns out that things are slightly more complicated. There
> is a tiny problem with David's test code, he did set "t" as its own
> object value instead of using the proxy there. When correcting this I
> noticed that the value from the proxy never gets used and extended the
> test code slightly and finally testing with different subclasses of
> NSCell, not just NSActionCell. They behave violently different. For
> NSCell and NSActionCell floatValue and doubleValue always return 0.0
> even when setting this value explicitly. NSSliderCell and
> NSTextFieldCell behave different, but also not the same way. Looks like
> we will need to investigate further and most likely will need to move
> the actual value handling code down into the specific NSCell subclasses.
> This sounds very ugly to me.
>
> Fred
>
>
> Am 20.02.2018 um 15:55 schrieb David Chisnall:
>> On 20 Feb 2018, at 14:30, Yavor Doganov <[hidden email]> wrote:
>>>
>>> I think this condition is always false.  _cell.has_valid_object_value
>>> is NO and _object_value is nil.  So it jumps to NSCell.m:269 and
>>> SMDoubleSliderCell's -stringValue is called which calls -stringHiValue
>>> which in turn calls -doubleHiValue and from there the infinite
>>> recursion is in place.
>>>
>>> At least this is what I observe in the debugger.
>>
>> Ah, that makes sense - I’d missed that in the trace.  So now the question is what happens when you call -doubleValue on an NSCell in Cocoa when it has a string value?  Here’s a little test program that finds out:
>>
>> #import <Cocoa/Cocoa.h>
>>
>> @interface Proxy : NSProxy
>> {
>> @public
>> id obj;
>> }
>> @end
>>
>> @implementation Proxy
>> - (BOOL)respondsToSelector: (SEL)aSelector
>> {
>> return [obj respondsToSelector: aSelector];
>> }
>> - (NSMethodSignature*)methodSignatureForSelector: (SEL)aSelector
>> {
>> return [obj methodSignatureForSelector: aSelector];
>> }
>> - (void)forwardInvocation: (NSInvocation*)anInvocation
>> {
>> NSLog(@"%@ set to proxy", anInvocation);
>> [anInvocation invokeWithTarget: obj];
>> }
>> @end
>>
>> @interface Test : NSActionCell @end
>> @implementation Test
>> - (float)floatValue
>> {
>> NSLog(@"-floatValue called\n");
>> return [super floatValue];
>> }
>> - (NSString*)stringValue
>> {
>> NSLog(@"-stringValue called\n");
>> return [super stringValue];
>> }
>> @end
>>
>> int main(void)
>> {
>> @autoreleasepool
>> {
>> Test *t = [Test new];
>> Proxy *p = [Proxy alloc];
>> p->obj = @"0.23";
>> [t setObjectValue: t];
>> NSLog(@"Querying");
>> NSLog(@"%f", [t floatValue]);
>> }
>> }
>>
>> The output is:
>>
>> 2018-02-20 14:46:39.917 a.out[85231:11731363] Querying
>> 2018-02-20 14:46:39.917 a.out[85231:11731363] -floatValue called
>> 2018-02-20 14:46:39.917 a.out[85231:11731363] 0.000000
>>
>>
>> So, from this we learn that Cocoa’s NSCell implementation doesn’t call any methods on either itself or the object to find the floating point value.  This is a bit odd, but at the very least we should fix GNUstep’s NSCell to query the object and not itself to find the string value.  The correct fix is probably:
>>
>> - (float) floatValue
>> {
>>  if (_cell.has_valid_object_value == YES)
>>  {
>>    if ([_object_value respondsToSelector: @selector(floatValue)]))
>>    {
>>      return [_object_value floatValue];
>>    }
>>    if ([_object_value respondsToSelector: @selector(stringValue)]))
>>    {
>>      return [[_object_value stringValue] floatValue];
>>    }
>>  }
>>  return 0;
>> }
>>
>> And apply similar fixes to the other *Value methods in NSCell.
>>
>> You can hack around this brokenness by including the above method in a category on NSCell in your application (though please remember to remove it once GNUstep is fixed!).
>>
>> David
>
> _______________________________________________
> Discuss-gnustep mailing list
> [hidden email]
> https://lists.gnu.org/mailman/listinfo/discuss-gnustep


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: SMDoubleSlider usability on GNUstep

Yavor Doganov-3
В Wed, 07 Mar 2018 23:35:53 +0100, Fred Kiefer написа:

> To me it now looks like NSSliderCell just overrides all the value
> methods of its super calls and implements them by setting a numerical
> value directly.

It appears so.

> For NSCell and specific sub classes things are different, but only
> if these get initialised as text cells. Doing this on an
> NSSliderCell would not set the max value, which prevents that cell
> from working.

This explains why Lynkeos code like

NSSliderCell *slider = [[[NSSliderCell alloc] initTextCell:@""]
autorelease];

doesn't work.  I had to change this to plain -init (which calls
-initImageCell: with a nil argument in the GNUstep implementation).

> I plan to do a full rewrite of NSSliderCell, without touching NSCell
> at all.

I may be wrong but I think that David's suggestion to modify
NSCell -*Value methods is correct nevertheless (this has nothing to do
with your analysis of course).

Many thanks in advance for working on this.  I'd wish I had the
appropriate knowledge and skills to help.


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: SMDoubleSlider usability on GNUstep

Fred Kiefer
In reply to this post by Yavor Doganov-3


> Am 23.02.2018 um 13:32 schrieb Yavor Doganov <[hidden email]>:
>
> В Tue, 20 Feb 2018 23:13:21 +0200, Yavor Doganov написа:
>
>> I turned NSCell's -setObjectValue: to a private method and changed all
>> -set*Value: methods to use it.  My test program doesn't crash now and I
>> can see the two knobs.  Movement is awkward (no sliding effect) and if I
>> click on the first knob the second one disappears.
>
> I reverted this change as it is causing the awkwardness, among other
> problems.  Instead, I disabled SMDoubleSliderCell's -setObjectValue:
> so that NSCell's method is always used.  It seems to be working
> properly except:
>
> 1. When the high knob is not set to a particular value with
> -set*HiValue: it is not displayed initially.  However, if I click on
> the right side of the bar both knobs become visible.
>
> 2. Possibly related with the above issue, if I click on the low knob,
> the high knob disappears.  It is visible where it should be during
> NSCell -trackMouse:inRect:ofView:untilMouseUp: but as soon as the
> mouse is up it disappears again.  When tracking the high knob both
> knobs are visible as expected.
>
> I believe these are GNUstep bugs or at least there is a difference in
> the behavior compared to Cocoa (assuming the code works as expected on
> Cocoa).
>
>> Is it a problem that SMDoubleSliderCell overrides both -drawKnob: and
>> -drawKnob?
>
> I tried merging them into -drawKnob: only, no difference in the
> behavior except that the high knob is not visible when the low knob is
> being moved with the mouse.
>
> 3. When the low knob is moved at the beginning of the bar, it becomes
> locked and can no longer be moved with the mouse.  This seems to be
> caused by this code in SMDoubleSliderCell's -startTrackingAt:inView:
>
>    if ( [ self trackingLoKnob ] && NSEqualRects( loKnobRect,
>                [ self knobRectFlipped:[ controlView isFlipped ] ] ) )
>        [ self setTrackingLoKnob:( _sm_loValue > [ self minValue ] ) ];
>
> When _sm_loValue is 0 or lower than minValue (in cases when minValue
> is set explicitly to a positive value), the boolean expression is
> false and only the high knob can be tracked; the low knob remains
> blocked indefinitely.  I replaced > with >= and it solves the problem,
> although it has the same effect as disabling this check entirely.  It
> seems bogus to me, the two knobs cannot overlap.

I committed a change to NSSliderCell that should fix all of the above issues as well as the original problem. This is done by directly using the value in many of the slider cell methods instead of accessing it via the method.  This brings our implementation of that class closer to the Apple version. But in my opinion this also reduces the software quality for that class. Please have a look at the change yourself to confirm this. We now copy the same code over and over again.

In another mail you asked me to implement some additional changes that David suggested in his original mails. I won’t do this, not just because the analysis of these mails was wrong, but also because this would even further reduce our code quality. Having similar behaviour to the Apple implementation is very important to me but we need a better argument than that to make our code worse.

Cheers,
Fred
_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: SMDoubleSlider usability on GNUstep

Yavor Doganov-3
В Mon, 12 Mar 2018 00:29:51 +0100, Fred Kiefer написа:

> I committed a change to NSSliderCell that should fix all of the above
> issues as well as the original problem.

Thanks, I confirm that.  No issues at all.  The only bad thing (for
me) is that I can't apply this patch to 0.26.2 as it breaks binary
compatibility.  No problem, the important thing is that it works.

> But in my opinion this also reduces the software quality for that
> class. Please have a look at the change yourself to confirm this. We
> now copy the same code over and over again.

Right :/.  The quality is still better than Apple's because there's an
accessor method for the _value ivar but you are right that this is
code duplication for no good reason.

> In another mail you asked me to implement some additional changes that
> David suggested in his original mails. I won’t do this, not just because
> the analysis of these mails was wrong, but also because this would even
> further reduce our code quality.

OK, I certainly won't argue about this.


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep