[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: GWorkspace Segfault
From: |
Fred Kiefer |
Subject: |
Re: GWorkspace Segfault |
Date: |
Mon, 25 Nov 2013 09:37:49 +0100 |
On the road
Am 25.11.2013 um 02:10 schrieb Riccardo Mottola <riccardo.mottola@libero.it>:
>> On 11/24/13 12:41, Fred Kiefer wrote:
>> Oops, I wasn't aware of that. Yes of course, there is more to remove.
>> All that similar code in FSNTextCell that is never used. And in
>> FSNBrowserCell the fontAttr variable is a static although it is used by
>> different cells which might have different fonts. Obviously this isn't
>> going to work. I think it should just be a local variable in the
>> cuttitle method.
> Yes it is dubious. It is however dynamically set in setFont, at least that.
> Perhaps it is supposed that all cells will have the same font?
The method setFont: is an instance method, there is no guaranty that it will
always be the same for all cells.
>
> Whiel trying to make infoFont and fontAttr non-static, I got puzzled and made
> GWorkspace crash again, so let's move gradually.
> The first thing: is infoFont actually needed? It is used aparently only for
> the dots, but the whole cell should use its own font (also since it
> implements setFont and re-sets fontAttr in there).
>
> I thus removed infoFont from everywhere and substituted it with just [self
> font] where needed, using NSCell's. This even saves memory :)
> What do you think? It works, I just commited and you sure have seen the patch
> flowing in.
I never commented on infoFont, as I don't understand it. I think it has nothing
to do with the dots handling and it is an italic font. Maybe you better revert
that change?
>> I also don't quite see why the cuttitle method gets cached. This just
>> obscures the code and doesn't speed up much.
> Well, the method is called for each cell in a browser thus it was thought to
> cache it.
Still not very likely to be noticable.
> What I do wonder is the very strange way to cache it, I have never seen that
> and I don't understand it.
>
> Look at this:
> typedef NSString *(*cutIMP)(id, SEL, id, float);
>
> that's the only typedef for cutIMP and I can hardly get what it means, the
> IMP variable is then declared this way:
> static cutIMP cutTitle = NULL;
>
> and used this way:
> cutTitle = (cutIMP)[self instanceMethodForSelector: cutTitleSel];
>
> and invoked this way:
> cuttitle = (*cutTitle)(self, cutTitleSel, uncuttedTitle, textlenght);
>
>
> I tried to rewrite it the way I have seen done in other places, by just using
> "IMP". Please look at the attached patch.
> The main difference is
>
> static IMP cutTitleImp = NULL;
>
> (rewriting is just a first step to make it then outside initialize, but I am
> tracking down the error and ask for help).
>
> The attached patch breaks. The method gets called (!) but by an NSLog inside,
> the last float parameter textlenght is always passed as 0.
>
> Why ????
Either remove the caching completely or leave the old correct code. The typedef
is the only way the compiler knows about the passed parameter and if you remove
that it may corrupt you stack.
>
>> This method also doesn't take into account the paragraph style (writing
>> direction and alignment). You would need something like NSCells
>> _nonAutoreleasedTypingAttributes method for this, but this isn't public
>> and doesn't exist on Cocoa.
> Are you referring to having file-names written right-to-left for example?
> Well, that's perhaps a task for the future. First let's remove crashes and
> optimize stuff.
The actual code wouldn't be any more complicated, just correcter.
>>
>> Don't get me started on code reviewing these classes. You could get back
>> a lot more input than you could handle :-)
> Not all a the same time. But there are several classes in FSNode that I want
> to clean up and don't really know how :) Thus slowly... it will improve
Somehow you didn't pick up any of the suggestions I made, but tried some
different a bit dubious changes. Not sure what to make of that.
- GWorkspace Segfault, edwin ancaer, 2013/11/16
- Re: GWorkspace Segfault, Riccardo Mottola, 2013/11/17
- Message not available
- Re: GWorkspace Segfault, Riccardo Mottola, 2013/11/19
- Re: GWorkspace Segfault, Fred Kiefer, 2013/11/19
- Re: GWorkspace Segfault, edwin ancaer, 2013/11/20
- Re: GWorkspace Segfault, Riccardo Mottola, 2013/11/20
- Re: GWorkspace Segfault, edwin ancaer, 2013/11/20
- Re: GWorkspace Segfault, Riccardo Mottola, 2013/11/23
- Re: GWorkspace Segfault, Fred Kiefer, 2013/11/24
- Re: GWorkspace Segfault, Riccardo Mottola, 2013/11/24
- Re: GWorkspace Segfault,
Fred Kiefer <=
- Re: GWorkspace Segfault, Riccardo Mottola, 2013/11/25
- Re: GWorkspace Segfault, Riccardo Mottola, 2013/11/25
- Re: GWorkspace Segfault, Fred Kiefer, 2013/11/25
- Re: GWorkspace Segfault, Riccardo Mottola, 2013/11/20