Re: [Fwd: Re: set of text scrolling patches (pageUp:/pageDown:) (+ Bonus ProjectCenter Patch :) )]

[David Ayers]

David Ayers wrote:

> Nicola Pero wrote:
>
> > Thanks David, I applied most of it - definitely the pageUp/pageDown, and
> > the fix for the font panel key equivalent.  Thanks!  Very good stuff. 
> > :-)
> Thanks a lot!
>
> > +   * Take verticalPageScroll into accout, but try to make sure
> > +   * that amount is never negativ (ie do not scroll backwards.)
> > +   * FIXME: Maybe we sould send messages instead of accessing
> > +   * ivars for verticakPageScroll as subclasses may override it,
> > +   * but for now be consistent with the rest of the code.
> >
> > If you want to change the verticalPageScroll, you call
> > setVerticalPageScroll:.  There is no need to subclass it. :-)
> >
> > If a subclass wants to change the vertical scroll, it can call
> > setVerticalPageScroll: as well (in -initWithFrame: I suppose).
> >
> > So I don't see any advantage in sending a message to itself to access 
> > its
> > own ivar in this case ... in practice it would just be a useless 
> > slow-down
> > - the API already gives you all the flexibility you need.
> >
> > There might definitely be cases in which sending a message to itself to
> > access an ivar might be a very good idea for more powerful 
> > subclassing etc
> > - but in this case in practical terms it's just a waste of resources
> > (which, I think, is why we access all those variables directly).
> Fine, not religious about it, since this *is* free software, if anyone 
> *really* needs it they can see it and state thier case or change it as 
> they need it.
>
> > +  iPoint.y = NSMinY(oldGlyphBounds) - 1;
> >
> > This is precisely where I was not really convinced.  It's sort of a
> > philosophical question :-) ... why 1 ?  why not 2 or 0.3 or 1000 ?
> >
> > I know it's because in practical terms, 1 works well with the normal 
> > fonts
> > under the normal scaling (you probably thought you'd never get a font
> > smaller than 1 in points/pixels).
> Yes you're right, I momentarily wondered what value I wanted. First I 
> thought of takeing half the size of the current line fragment, but 
> that upper line fragment couln have a very different size. Then I 
> thought of going down/up the text storage (or glyph) chain) until the 
> line fragment changes, but then I would need to to know in which 
> direction the line sweeps. And this is when I reallised that 
> implementing the hardly documented NSLineSweepDirection and 
> NSLineMovesDirction handling would be much more fundamental, than 
> worrying about fonts with less than 1.0 points. :-) As this 
> NSLine*Direction is niether handled by my patch or the current 
> imlpementation, I didn't give the "search delta" much though anymore.
>
> For practical reasons I would just leave the 1.0 and add a "FIXME: 
> when we have NSLine*Direction handling and then we can search up/down 
> the charchter/glyph chain to get the size/position of the prev/next 
> line fragment/gyph. Because in certain cases (ie: NSLineSweepUp/Down) 
> we would have to select the prev/next glyph instead of the prev/next 
> line fragement.
>
> Cheers,
> Dave

After another quick chat with Alexander, I'm not so sure anymore that 
1.0 is such a good value. You probably need some allgorithm, which loop 
through all line fragments and determinst the best fit. And also take 
into account vertical line fragments, that would result in selection the 
next/prev glyph in the same fragement. But all this needs a new 
NSLayoutManager and some of it's subclasses and goes far beyond what I 
can currently suggest in patch.
As a compromise maybe a
step = MIN (0.01, [oldGlyphBounds height]/10);
would solve it until then.
This would still cause problems in a text of alternating extreme sizes, 
but I think we shouldn't worry about that yet. I believe Alex is looking 
into a new and improved NSLayoutManger and type setting implementations.

Cheers,
Dave