Mantis Bugtracker
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0006664 [Squeak] Graphics major N/A 09-09-07 01:57 05-26-08 18:09
Reporter wiz View Status public  
Assigned To andreas
Priority normal Resolution open  
Status assigned   Product Version
Summary 0006664: TTCFont>ascent can be infinitely recursive.
Description See the spacelow log attached.

If a FixSpacedFonts basefont = self then a TTCFont will be recursive when its fallbackFont is the fixed space font.

Additional Information I found this while testing the fix for IsFontAvailable in 0005309.
When running the LocaleTest things went bonkers.

So is there some reason this should never happen?

Are TTCFonts allowed to be base fonts?
Are FixedFaceFonts allowed to be fallbackfonts?

Are the LocaleTests correctly made?

Right now it seems clear that too many confilicting assumtions are present at the same time in the font implementation.

And I think the only way out of this mess is if someone clearly states the font invariants and distributes them to all the interested parties.

Attached Files  TTFascentRecurseLowSpace.log [^] (2,369 bytes) 09-09-07 01:57
 TTCFont-ascent-efc.st [^] (455 bytes) 09-11-07 23:55

- Relationships

SYSTEM WARNING: Creating default object from empty value

related to 0006520closed  recursive fonts cause crash when running tests 
child of 0006570assigned tim A Mother for font and font test problems 

- Notes
(0011106 - 1516 - 2526 - 2526 - 2526 - 2526 - 2526)
wiz
09-11-07 23:45
edited on: 09-11-07 23:50

Why #ascent is infinitely recursive in 7143.


from current sq-7143

ascent
"ar 11/14/2006 15:35 TTCFont ascent {accessing}"
    ascent ifNil:[ascent := ttcDescription ascender * self pixelSize // (ttcDescription ascender - ttcDescription descender) * Scale y].
    ^ (fallbackFont notNil
            and: [fallbackFont ascent > ascent])
        ifTrue: [fallbackFont ascent]
        ifFalse: [ascent]

endlessly recursive when TTCFont fallBackFont is a Fixed Face Font with the TTCFont as its basefont.

from sq-7067
ascent
“efc 9/30/2005 21:28 TTCFont ascent {accessing}”
    "Use a cached copy if available"
    ascent ifNil:[
        ascent := ttcDescription ascender * self pixelSize // (ttcDescription ascender - ttcDescription descender) * Scale y.
    (fallbackFont notNil and: [fallbackFont ascent > ascent])
        ifTrue: [ascent := fallbackFont ascent].
    ].
    ^ascent

from both

ascent
"ar 1/5/2003 16:58 FixedFaceFont ascent {accessing}"
    ^baseFont ascent

from earlier

ascent
"tak 12/22/2004 02:28 TTCFont ascent {accessing}"
    | ascent |
    ascent := ttcDescription ascender * self pixelSize // (ttcDescription ascender - ttcDescription descender) * Scale y.
    ^ (fallbackFont notNil
            and: [fallbackFont ascent > ascent])
        ifTrue: [fallbackFont ascent]
        ifFalse: [ascent]

tak’s code would also be endlessly recursive.

efc code barely avoids recusion because ascent is set and so fallback ascent never gets called.

For both tak and ar's code recursion is infinite (for basefont == this ttcfont )

 
(0011107 - 423 - 525 - 525 - 525 - 525 - 525)
wiz
09-12-07 00:00
edited on: 09-12-07 00:09

 TTCFont-ascent-efc.st uploaded.

This is exactly the 7067 implementation.

[OT] This is why I need the histories. 7143 has only one version "ar's"
7067 has only one version "efc'c"
I had to go back to 7054 to discover "tak's" version.

Anyway the present suggested fix is a reversion to efc's code. Though it would be good if ar can report why his is different.

Yours in curiosity and service, --Jerome Peace

 
(0011108 - 209 - 245 - 245 - 245 - 245 - 245)
wiz
09-12-07 00:03

Reminder sent to: andreas

Hi andreas,

It would be good if you could give looking at this a priority.
It is a serious bug and it seems to be of your making.
It may affect your projects as well.

Cheers - Jerome Peace, Bug tracker
 
(0011204 - 1502 - 1574 - 1574 - 1574 - 1574 - 1574)
wiz
09-25-07 07:55

two thoughts to add.

My current notion is that lazy evaluation of ascent in TTCFont is the crux of the problem. Messages are a community affair. The traditional protocol in other objects is that ascent is just an unrecusive accessor not a lazy accessor that does evaluations. So a fix that brings TTCFont in line with that tradition would be good.

The other thought is an uneasy one. The infinite recursion found here is probably not the only case. I have not looked for recursion elsewhere in TTCFont accessors. So the fact that I have found on obvious case suggests that there are others to be found if searched for.

I have started to look for ways of determining if a message is potentially recursive. So far I have discovered there are 40K+ compiled methods representing 20K+ unique messages. Of which only 10-15% are easily proven not recusive. So a lot of squeak is likely recursive. And there are probably quite a few instances of potentially infinite recursion.

I have already found several methods that prove to be infinitely recursive under circumstances that are quite likely to happen.

The chilling part of this thought is that the way squeak is maintained it is probably quite easy to introduce infinite recursion bugs. These bugs will not be immediately obvious. They will cause problems that are severe to the hapless soul who runs into them. They will also be quite hard to track down.

And most chilling. No one (excluding present company) is even looking for them.
 
(0011224 - 69 - 69 - 69 - 69 - 69 - 69)
RalphJohnson
09-29-07 17:55

I think I fixed this problem. I posted the solution over in 0006520;
 
(0011245 - 1056 - 1174 - 1174 - 1174 - 1174 - 1174)
wiz
10-07-07 04:29

The fix in 6520 relies on the a restriction on the fonts.

The TTCFont-ascent-efc.st code reversion stops ascent from being infinitely recursive even it the fonts are the same.

My belief is that the recusion bug should be handled in the recursive methods. Relying just on the well ordering of fonts is riskier.

Also it seems to me that there is no reason for TTCFonts to open themselves up to recursive problems.

As I looked at the code for fixed faced font it seemed be designed to provide a font with exactly one glyph. (error glyph or password glyph).

So it would seem to me that if a TTCFonts defined those two glyphs then there would be no reason for them not to be a fixed face font basefont.

The fact that they we are running into problems due to recursion here points to a design that missed the "simpliest thing that just might work" principle.

My recommendation is that TTCfont>>ascent should be reverted to
TTCFont-ascent-efc.st in addition to any other fixing changes.

Yours in curiosity and service, --Jerome Peace
 
(0011281 - 1079 - 1139 - 1139 - 1139 - 1139 - 1139)
wiz
10-10-07 03:46

I have made a pass at redoing the fix in ascent. And I ran into an issue with my ignorance of what ascent is needed to do.

efc fix assigns ascent to the greater of an ascent calculated from the ttc font description or the ascent of the fallback font. It avoids infinite recusion by always returning early if ascent is set and
always setting ascent before the potentially recursive call.

The question I have is why wasn't that left the way it was? Looking at ar's and tak's fixes the ascent ivar is not given the fallback fonts ascent value. That is, when the fall back fonts value is greater than ascent the fall back value is returned. But it is not used to set the ivar ascent whereas in efc's it does.

I can see where setting ascent to the larger value rather than the calculated value could lead to font glitches. But taming recusion requires the circuit breaker of choosing an value for the ivar and then sticking to it.
So I need to hear why tak and ar choose to do it the way they did rather than efc's way.

Yours in curiosity and service, --Jerome Peace
 
(0012168 - 1426 - 1552 - 1552 - 1552 - 1552 - 1552)
wiz
05-26-08 18:09
edited on: 05-26-08 18:17

Rethink:

I am looking at this again. Noticing the OLPC squeak also does something infinitely recursive.

My current thought is that TTCfont is wrong to lazily initialize ascent in the first place.

It should always be set. ascent should just return the value of the preset ascent just as StrikeFont does.

Since it depends on fallbackfont, setting fallbackfont is the right place to set ascent (and probably widthOf:)

the computation should be factored into its own method e.g.computeAscent which returns the straight forward computatation.

The other thing I think has happen with 3.10 (and therefore 3.10.1) is that the code has now been messily patched around. Fallbackfonts are forbiden to be ttc fonts. This is NOT GOOD. The only purpose for a fallback font is to supply a password glyph or error glyph regardless of character input. This means that ideally a font would be the ideal basefont for its own fallback. The recursion is the real problem and calls for a fix.

As I look at TTCFonts I get more and more curious.
Why do the store a 32 depth form for each glyph???
If only one color is involved wouldn't a 1 bit depth suffice?
If grey scaling is used then a colorform of 256 bits would surely do the deed with bitblt doing the translation to screen depth.

I am still of the impression that TTCFonts were crafted by coders who were not aware fully of what they were doing at the time.

 

- Issue History
Date Modified Username Field Change
09-09-07 01:57 wiz New Issue
09-09-07 01:57 wiz Status new => assigned
09-09-07 01:57 wiz Assigned To  => andreas
09-09-07 01:57 wiz File Added: TTFascentRecurseLowSpace.log
09-09-07 01:58 wiz Relationship added child of 0006570
09-11-07 23:45 wiz Note Added: 0011106
09-11-07 23:50 wiz Note Edited: 0011106
09-11-07 23:55 wiz File Added: TTCFont-ascent-efc.st
09-12-07 00:00 wiz Note Added: 0011107
09-12-07 00:03 wiz Note Added: 0011108
09-12-07 00:09 wiz Note Edited: 0011107
09-12-07 00:15 wiz Relationship added related to 0006520
09-25-07 07:55 wiz Note Added: 0011204
09-29-07 17:55 RalphJohnson Note Added: 0011224
10-07-07 04:29 wiz Note Added: 0011245
10-10-07 03:46 wiz Note Added: 0011281
05-26-08 18:09 wiz Note Added: 0012168
05-26-08 18:17 wiz Note Edited: 0012168


Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
87 total queries executed.
50 unique queries executed.
Powered by Mantis Bugtracker