gnustep-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: CA broken; how do I differentiate NSValues for size and point?


From: Ivan Vučica
Subject: Re: CA broken; how do I differentiate NSValues for size and point?
Date: Sun, 27 May 2018 18:44:10 +0100

I'll open a bug for -base. In the meantime I'll help Stjepan disable shadowOffset.

Minimum repro::

#import <Foundation/Foundation.h>

@interface A : NSObject
{
  NSSize _s;
}
@end
@implementation A
- (NSSize) s {
  return self->_s;
}
- (void) setS: (NSSize) s {
  self->_s = s;
}
@end
int main() {
  NSSize in = NSMakeSize(1.0, 2.0);
  NSValue * v = [NSValue valueWithBytes: &in
                               objCType: @encode(NSSize)];
  A * a = [A new];
  [a setValue: v
       forKey: @"s"];


  return 0;
}


Output:
$ clang `gnustep-config --objc-flags` `gnustep-config --objc-libs` `gnustep-config --base-libs` repro.m -o repro && ./repro
clang: warning: argument unused during compilation: '-fobjc-nonfragile-abi'
clang: warning: argument unused during compilation: '-fobjc-nonfragile-abi'
2018-05-27 18:42:13.137 repro[14432:14432] match! point is {_NSPoint=dd}, type is {_NSSize=dd}
./repro: Uncaught exception NSInvalidArgumentException, reason: [GSSizeValue-pointValue] should be overridden by subclass
Aborted





On Sun, May 27, 2018 at 6:18 PM Ivan Vučica <address@hidden> wrote:
So turns out we don't use shadowOffset in any demo code. So examining CAAnimation was pointless.

However I managed to get a backtrace using lldb, and this happens when the code sets the default shadowOffset value. So you pointed to the right place; shadowOffset is the problem. This is the place of the exception, in -[CALayer init] which calls +[CALayer defaultValueForKey:]:

  if ([key isEqualToString: @"shadowOffset"])                                                                
    {
      CGSize offset = CGSizeMake(0.0, -3.0);
      return [NSValue valueWithBytes: &offset objCType: @encode(CGSize)];
    }

What is strange is that this happens deep inside base, when I am setting valueWithBytes, with a CGSize type.

So I looked closer at the backtrace and it's happening in base/Source/Additions/GSObjCRuntime.m. Relevant chunk:

    frame #2: 0x00007ffff59e9d7d libgnustep-base.so.1.25`+[NSException raise:format:](self=0x00007ffff5f69878
, _cmd="S", name=0x00007ffff5f69438, format=0x00007ffff6003828) + 365 at NSException.m:1376
    frame #3: 0x00007ffff5b9ca2f libgnustep-base.so.1.25`-[NSObject(self=0x000000000148f568, _cmd="\xffffffa9
\x02", aSel="\x0e\x01") subclassResponsibility:] + 255 at NSObject+GNUstepBase.m:134
    frame #4: 0x00007ffff5b1d32b libgnustep-base.so.1.25`-[NSValue pointValue](self=0x000000000148f568, _cmd=
"\x0e\x01") + 43 at NSValue.m:394
    frame #5: 0x00007ffff5b58649 libgnustep-base.so.1.25`GSObjCSetVal(self=0x00000000014591c8, key="shadowOff
set", val=0x000000000148f568, sel="\xffffffda\e", type="{_NSSize=dd}", size=12, offset=0) + 3497 at GSObjCRun
time.m:1794
    frame #6: 0x00007ffff5a25857 libgnustep-base.so.1.25`SetValueForKey(self=0x00000000014591c8, anObject=0x0
00000000148f568, key="shadowOffset", size=12) + 1079 at NSKeyValueCoding.m:150
    frame #7: 0x00007ffff5a253ee libgnustep-base.so.1.25`-[NSObject(self=0x00000000014591c8, _cmd="P\x04", an
Object=0x000000000148f568, aKey=0x00007ffff62cc560) setValue:forKey:] + 382 at NSKeyValueCoding.m:370
    frame #8: 0x00007ffff5a2881d libgnustep-base.so.1.25`-[GSKVOBase setValue:forKey:](self=0x00000000014591c
8, _cmd="P\x04", anObject=0x000000000148f568, aKey=0x00007ffff62cc560) + 221 at NSKeyValueObserving.m:238
    frame #9: 0x00007ffff60a22ac libQuartzCore.so.0`-[CALayer init](self=0x00000000014591c8, _cmd="\xffffffa1

I added a debug statement to GSObjCSetVal:

          case _C_STRUCT_B:
            if (GSSelectorTypesMatch(@encode(NSPoint), type))
              {
                NSLog(@"match! point is %s, type is %s", @encode(NSPoint), type);
                NSPoint v = [val pointValue];

This is the output:
2018-05-27 17:58:46.980 hello_carenderer[10274:10274] match! point is {_NSPoint=dd}, type is {_NSPoint=dd}
2018-05-27 17:58:46.981 hello_carenderer[10274:10274] match! point is {_NSPoint=dd}, type is {_NSSize=dd}

Therefore, it's a bug in -base that makes it interpret sizes as points!

I'm still coming up with a minimum repro case.

(n.b. some of the confusion on why CGSize and CGPoint get understood as their NS equivalents is this chunk from opal:
  typedef NSPoint CGPoint;
  typedef NSSize CGSize;
  typedef NSRect CGRect;
I do not believe this to be correct, but I will not be addressing it at this time.

Separately: are typedefs of structs meant to be encoded as the original struct name? Probably yes?)


On Sun, May 27, 2018 at 5:29 PM Ivan Vučica <address@hidden> wrote:
I added a test to the -base from May 20:

  NSPoint point = {.x = 16.0, .y = 32.0};
  NSValue *pointV = [NSValue valueWithPoint: point];

  result = !strcmp(@encode(NSPoint), [pointV objCType]);
  PASS(result, "@encoding(NSPoint) == pointV objCType");

  result = strcmp(@encode(NSSize), [pointV objCType]);
  PASS(result, "@encoding(NSSize) == pointV objCType");

with this temporarly ine:
  NSLog(@"%s %s %s", @encode(NSPoint), @encode(NSSize), [pointV objCType]);

It passes and prints out:

2018-05-27 17:27:38.823 size-point-encoding[25395:25395] {_NSPoint=dd} {_NSSize=dd} {_NSPoint=dd}

I''m annoyed at how I did not notice that I did use CGSize. I will introduce support for NSSize/CGSize.

Thanks!

On Mon, May 21, 2018 at 12:53 AM Fred Kiefer <address@hidden> wrote:
I added a bit of code in base that allows to use NSValue objects for size and point with methods for the other type. This is a bit closer to the Cocoa behaviour but will require more tweaking to have it fully correct. It will at least allow you to run this test code again.

> Am 20.05.2018 um 16:02 schrieb Fred Kiefer <address@hidden>:
>
> I spend some time to understand this issue. But to be honest it took me more time to understand why this ever worked :-)
>
> The problem here is that base uses two different ways to provide concrete subclasses for NSValue. One being GSValue, which supports generic data and the other mechanism is to have a specific subclass for types like NSPoint and NSSize. For the later base only generates specific methods for that types. Resulting in different classes for NSPoint and NSSize, which are incompatible although they have the same underlying data size.
> In your demo application that animation for shadowOffset tries to set the offset size with a suitable default value of class GSSizeValue. But the code in GSObjCSetVal only compares the type information without looking at the type names. That way NSSize is regarded as the same as NSPoint and the pointValue get executed, resulting in the error you reported. Either GSObjCSetVal has to be more careful and use a check for the actual type first. Or we need to make the NSValue subclasses more general.
>
> But why did it work before? Most likely because at that time CGSize and CGPoint, where different from NSSize and NSPoint so we did not get the specific optimisation in NSValue.
>
> Hope this helps,
> Fred
>
>> Am 20.05.2018 um 14:03 schrieb Ivan Vučica <address@hidden>:
>>
>> Hi,
>>
>> Pretty much all Core Animation demos are currently broken under GNUstep with a variation on the following:
>>
>> 2018-05-20 12:54:25.464 QuartzCoreDemo[13476:13476] Problem posting notification: <NSException: 0x15b53b8> NAME:NSInvalidArgumentException REASON:[GSSizeValue-pointValue] should be overridden by subclass INFO:(null)
>>
>> CA is accessing -pointValue method if it determines that the passed NSValue matches the -objCType of NSPoint. It does not check for size values.
>>
>> Clearly, sometimes it is trying to interpolate size values, which will match the signature and it will incorrectly attempt to access -pointValue which is then not implemented by GSSizeValue. I am not sure where might it be interpolating size values, but it seems to be doing so. (Alternative bug is that NSValue is incorrectly ingesting points as sizes, then complaining when the point provided is being interpreted as a point. I can try checking this later.)
>>
>> I cannot check -respondsToSelector: because the class /does/ in fact respond to -pointValue; it just throws an exception.
>>
>> Adding a try/catch in this kind of situation would make for some very poor code hygiene, in my opinion.
>>
>> - Is NSValue supposed to be a class cluster like this? (Not on Mac at this time, can't check.)
>> - Is there a way out?
>> - Would it make sense to extend GSSizeValue and add -pointValue to it? (They're both 2d vectors.)
>>
>> Thanks
>> _______________________________________________
>> Gnustep-dev mailing list
>> address@hidden
>> https://lists.gnu.org/mailman/listinfo/gnustep-dev
>
>
> _______________________________________________
> Gnustep-dev mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/gnustep-dev


_______________________________________________
Gnustep-dev mailing list
address@hidden
https://lists.gnu.org/mailman/listinfo/gnustep-dev

reply via email to

[Prev in Thread] Current Thread [Next in Thread]