Re: ANN: PikoPixel pixel-art editor

[H. Nikolaus Schaller]

Hi,

Am 20.09.2015 um 16:54 schrieb Fred Kiefer <fredkiefer@gmx.de>:

> Hi Nikolaus,
> 
> Am 20.09.2015 um 10:41 schrieb H. Nikolaus Schaller:
>> Am 20.09.2015 um 00:06 schrieb Josh Freeman 
>> <gnustep_lists@twilightedge.com>:
>>> On Sep 19, 2015, at 7:20 AM, Fred Kiefer wrote:
>>>> I am at the moment trying to get it running on gcc with the gcc 
>>>> objc runtime. I know this isn't the supported constellation, but 
>>>> I would like to at least understand what features are missing in 
>>>> this setup.
>>> 
>>> PikoPixel's implementation of method-swizzling doesn't work with 
>>> the gcc runtime because the runtime's function, 
>>> class_replaceMethod(), behaves differently than on the objc2 or 
>>> Apple runtimes. That’s the only issue I know of (though I didn't 
>>> test much further on gcc after finding it), so in theory, a 
>>> reimplementation of method-swizzling (in 
>>> NSObject_PPUtilities_MethodSwizzling.m) that removed/reworked the 
>>> class_replaceMethod() calls could allow PP to use the gcc runtime.
>> 
>> I have tried to understand a little why PikoPixel uses method 
>> swizzling at all. I know that it is a *very* powerful design
>> pattern, but makes code quite difficult to understand what happens.
>> And significant portability issues as we see them. But I never felt
>> that I need the method/class swizzling pattern (and using IMP) in
>> any Cocoa or GNUstep client code I did write - there are always
>> simple and efficient solutions without (e.g. delegation, proxy,
>> simple if-statements, performSelector).
>> 
>> At least in toggleScreencastingEnabled: I get the impression that 
>> PikoPixel wants to either call some default sendEvent: or an
>> extended one (screencastOverrideIMP_SendEvent:) depending on some
>> toggle button.
>> 
>> But in screencastOverrideIMP_SendEvent it again checks if 
>> gPPScreencastingIsEnabled which must always be true because it has 
>> not been swizzled it in otherwise.
>> 
>> At least here you could just override sendEvent:, do the test and 
>> call [super sendEvent:] at the end.
>> 
>> You even comment this as:
>> 
>> // sendEvent: only needs to be overridden during screencasting, so 
>> rather than defining // the method in the class @implementation, 
>> which makes a permanent override, instead // add/switch the method's 
>> implementation on-the-fly using class_replaceMethod()
>> 
>> What do you want to achieve by a dynamic override? Save 20 ns in 
>> event handling? You loose it by doing the swizzling each time the 
>> user applies the toggle button. Why not just do the permanent 
>> override?
> 
> I think you are right about this specific use, but then again this isn't
> actually swizzling. You seem to fail to get the general idea behind
> swizzling. You use swizzling when you don't want to have a subclass with
> specific behaviour but need the objects of the original class to behave
> slightly different.

I get this, but I question why it is necessary to avoid the subclass design
pattern - except in bug fixing or really low level stuff.

> 
>> I have not looked into other places where it is used and why, but 
>> [object performSelector:sel] can do almost the same as “[object 
>> swizzle:method with:sel]; [object method]”. performSelector: and 
>> performSelector:withObject: are much more portable.
> 
> I don't understand this remark. In what cases could performSelector:
> replace swizzling?

In the example I have studied, the toggle method makes the sendEvent
method do either this or that by swizzling. This can also be achieved
by doing perfomSelector:sel1 or performSelector:sel2. And therefore
it is possible to define a variable which stores the real selector. Then
method swizzling will be replaced by a dynamically chosen selector
to choose two different behaviours.

I.e.

- methodNotSwizzled
{
        [self performSelector:swizzleSelector];
}

- fakeSwizzle:(SEL) sel
{
        swizzleSelector=sel;
}

- behaviour1
{
        …
}

- behaviour2
{
        …
}

So you can dynamically change the behaviour of methodNotSwizzled
from the outside. Basically you do not replace the IMP pointer of 
methodNotSwizzled
but the selector used by performSelector to achieve this result.

But usually it is even be simpler to implement without any swizzling

- methodNotSwizzled
{
        if(otherBehaviour)
                [self behaviour2];
        else
                [self behaviour1];
}

This is why I question the use of swizzling at all in standard application code.
IMHO there is no need to use it since the alternatives are easier to understand
and more portable.

> 
>> By using a slightly higher level API I think the code will become 
>> much shorter, easier to understand and more portable without needing 
>> any other efforts outside of PikoPixel.
>> 
>> So my suggestion would be to modify PikoPixel to avoid swizzling at 
>> all, instead of adding another lowest level compatibility layer to 
>> GNUstep.
> 
> Swizzling should be avoided where it isn't needed, for example by fixing
> he bugs in GNUstep instead of working around them but in other cases it
> is quite a useful tool.

It can be useful for special cases, but IMHO should be avoided in any 
application code.

> And if the gcc runtime has an issue with it, we
> should try to get that fixed.

Of course, compatibility of the gcc runtime be extended. And if a bug occurs in 
areas
where swizzling is really needed, it must be fixed of course.

BR,
Nikolaus