Re: ANN: PikoPixel pixel-art editor

[H. Nikolaus Schaller]

Hi,

Am 20.09.2015 um 00:06 schrieb Josh Freeman <gnustep_lists@twilightedge.com>:

> Hi Fred,
> 
> On Sep 19, 2015, at 7:20 AM, Fred Kiefer wrote:
> 
>> Hi Josh,
>> 
>> thank you very much for porting this great code over to GNUstep.
>> 
>> 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 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.

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.

BR,
Nikolaus


> 
> 
>> I parallel I am digging through your GNUstep specific changes. I already
>> found the NSMatrix additions and the cairo non-anti-alias stuff. I am
>> going to add code into GNustep that supports this to make specific
>> additions for GNUstep unnecessary. In theory it should be possible to
>> just reuse your Cocoa code on GNUstep. Where this isn't possible, please
>> file a bug report against GNUstep, preferable with an attached patch.
> 
>    I kept an internal list with brief, 1-to-2-line descriptions of issues 
> (now at about 17 items) that needs to be expanded into full bug reports on 
> savannah when there's time - I wrote one up so far: 
> http://savannah.gnu.org/bugs/?45442 . I'd be happy to send the raw list to 
> you - or post it to the developer mailing list - if that would be useful.
> 
> 
>> Once again thank you and I will keep you updated on my progress.
> 
>   Thanks, please do!
> 
> 
>> Cheers,
>> Fred
> 
> Cheers,
> 
> Josh
> 
> 
> 
>> Am 18.09.2015 um 21:57 schrieb Josh Freeman:
>>>  PikoPixel is a free Mac OS X pixel-art editor that's currently in
>>> beta for its initial 1.0 release.
>>> 
>>>  The latest beta version, 1.0 BETA5, is the first source-code release
>>> (AGPL v3), and the first version that also runs on GNUstep.
>>> 
>>>  PikoPixel GNUstep binaries aren't available yet (haven't gotten
>>> around to figuring out GS standalone application packaging), so for the
>>> moment, PikoPixel must be built from source.
>>> 
>>>  Requirements for compiling PikoPixel are a recent version of the
>>> GNUstep development environment (June 2015 or later) and the libobjc2
>>> runtime. Also, PP's only been tested so far under Clang, and on
>>> Debian-based Linux distros (Ubuntu & Mint), so there may be issues with
>>> other configurations.
>>> 
>>>  PikoPixel's source code archive is linked at the bottom of the
>>> webpage (not the green "Download" arrow, which downloads the Mac-only
>>> binary):
>>> http://twilightedge.com/mac/pikopixel/
>>> 
>>>  Please send questions, comments, or issues to pikopixel (at)
>>> twilightedge (dot) com.
>>> 
>>> Cheers,
>>> 
>>> Josh Freeman
>>> Twilight Edge Software
>>> http://twilightedge.com
>> 
> 
> 
> _______________________________________________
> Discuss-gnustep mailing list
> Discuss-gnustep@gnu.org
> https://lists.gnu.org/mailman/listinfo/discuss-gnustep