<div dir="ltr"><div><div><div><div><div><div><div><div><div>Ah, to be complete I forgot to say farewell multiComposeSomething, since it was an unused sender of fixupLastLineIfCR that I wanted to remove too...<br></div><div>
<br>If you want to see the bug before this change, then:<br>open a Workspace, type M space space space ...<br></div>.. until the cursor wraps to... the beginning of first line.<br></div><div>type one more space and it skips on the next line.<br>
</div><div><br></div>What happens it's damn simple:<br></div>- the composition scanner crossedX<br></div>- the space that crossedX is inserted in the first line (like a CR would)<br></div>- there is no more character in the text<br>
</div>- the TextComposer has finished to composeLines and ^nil<br></div>- it tries to fixupLastLineIfCR, unnfortunately last char behaved as a CR but wasn't a CR<br><br></div>Composition finishes with a single line when there should be logically an empty second line...<br>
<br></div><div>Chris: I hope the comments carry enough intention :)<br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2013/10/2 <span dir="ltr"><<a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a>></span><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Nicolas Cellier uploaded a new version of Graphics to project The Trunk:<br>
<a href="http://source.squeak.org/trunk/Graphics-nice.243.mcz" target="_blank">http://source.squeak.org/trunk/Graphics-nice.243.mcz</a><br>
<br>
==================== Summary ====================<br>
<br>
Name: Graphics-nice.243<br>
Author: nice<br>
Time: 2 October 2013, 2:47:50.925 am<br>
UUID: 220dcb9f-1fed-4f7d-b6de-170fe2b2a3ce<br>
Ancestors: Graphics-nice.242<br>
<br>
Fix a composition glitch when the last character of a text is a space that crosses the right margin boundary.<br>
In such case, a virtual empty line must be added to the composition in order to correctly materialize text selection and cursor position, and so as to continue typing on next line.<br>
<br>
The case when last character is a carriage return is in all point similar.<br>
Indeed, a space that crossedX is visually turned into a new line.<br>
TextComposer previously tried to reverse engineer scanner's work to recognize the CR case, which is a smell.<br>
This change unifies handling for the two cases by rather asking to the scanner doesTheLineBreaksAfterLastChar?<br>
Remove fixupLastLineIfCR which is tainted with half case only.<br>
Remove the workaround in CharacterBlockScanner that did not work around anything.<br>
<br>
Fix the breaking at non space for eastern asia:<br>
1) registerBreakableIndex records that the line can wrap before the current character, and spaceIndex was pointing at this character that will potentially wrap on next line.<br>
2) It is still possible to apply Justified alignment based on space adjustment if some spaces are used in the text, so correctly set the line spaceCount and paddingWidth.<br>
<br>
=============== Diff against Graphics-nice.242 ===============<br>
<br>
Item was changed:<br>
----- Method: CharacterBlockScanner>>crossedX (in category 'stop conditions') -----<br>
crossedX<br>
"Text display has wrapping. The scanner just found a character past the x<br>
location of the cursor. We know that the cursor is pointing at a character<br>
or before one."<br>
<br>
self retrieveLastCharacterWidth.<br>
<br>
- characterIndex == nil ifFalse: [<br>
- "If the last character of the last line is a space,<br>
- and it crosses the right margin, then locating<br>
- the character block after it is impossible without this hack."<br>
- characterIndex > text size ifTrue: [<br>
- lastIndex := characterIndex.<br>
- characterPoint := (nextLeftMargin ifNil: [leftMargin]) @ (destY + line lineHeight).<br>
- ^true]].<br>
characterPoint x <= (destX + (lastCharacterWidth // 2))<br>
ifTrue: [characterPoint := destX @ destY.<br>
^true].<br>
lastIndex >= line last<br>
ifTrue: [characterPoint := destX @ destY.<br>
^true].<br>
<br>
"Pointing past middle of a character, return the next character."<br>
lastIndex := lastIndex + 1.<br>
characterPoint := destX + lastCharacterWidth + kern @ destY.<br>
^ true!<br>
<br>
Item was changed:<br>
CharacterScanner subclass: #CompositionScanner<br>
+ instanceVariableNames: 'spaceX spaceIndex lineHeight baseline lineHeightAtSpace baselineAtSpace lastBreakIsNotASpace nextIndexAfterLineBreak'<br>
- instanceVariableNames: 'spaceX spaceIndex lineHeight baseline lineHeightAtSpace baselineAtSpace lastBreakIsNotASpace'<br>
classVariableNames: ''<br>
poolDictionaries: ''<br>
category: 'Graphics-Text'!<br>
<br>
!CompositionScanner commentStamp: '<historical>' prior: 0!<br>
CompositionScanners are used to measure text and determine where line breaks and space padding should occur.!<br>
<br>
Item was changed:<br>
----- Method: CompositionScanner>>composeFrom:inRectangle:firstLine:leftSide:rightSide: (in category 'scanning') -----<br>
composeFrom: startIndex inRectangle: lineRectangle<br>
firstLine: firstLine leftSide: leftSide rightSide: rightSide<br>
"Answer an instance of TextLineInterval that represents the next line in the paragraph."<br>
| runLength stopCondition |<br>
"Set up margins"<br>
leftMargin := lineRectangle left.<br>
leftSide ifTrue: [leftMargin := leftMargin +<br>
(firstLine ifTrue: [textStyle firstIndent]<br>
ifFalse: [textStyle restIndent])].<br>
destX := spaceX := leftMargin.<br>
rightMargin := lineRectangle right.<br>
rightSide ifTrue: [rightMargin := rightMargin - textStyle rightIndent].<br>
lastIndex := startIndex. "scanning sets last index"<br>
destY := lineRectangle top.<br>
lineHeight := baseline := 0. "Will be increased by setFont"<br>
line := (TextLine start: lastIndex stop: 0 internalSpaces: 0 paddingWidth: 0)<br>
rectangle: lineRectangle.<br>
self setStopConditions. "also sets font"<br>
runLength := text runLengthFor: startIndex.<br>
runStopIndex := (lastIndex := startIndex) + (runLength - 1).<br>
+ nextIndexAfterLineBreak := spaceCount := 0.<br>
- spaceCount := 0.<br>
lastBreakIsNotASpace := false.<br>
self handleIndentation.<br>
leftMargin := destX.<br>
line leftMargin: leftMargin.<br>
<br>
[stopCondition := self scanCharactersFrom: lastIndex to: runStopIndex<br>
in: text string rightX: rightMargin stopConditions: stopConditions<br>
kern: kern.<br>
"See setStopConditions for stopping conditions for composing."<br>
self perform: stopCondition] whileFalse.<br>
<br>
^ line<br>
lineHeight: lineHeight + textStyle leading<br>
baseline: baseline + textStyle leading!<br>
<br>
Item was changed:<br>
----- Method: CompositionScanner>>composeLine:fromCharacterIndex:inParagraph: (in category 'scanning') -----<br>
composeLine: lineIndex fromCharacterIndex: startIndex inParagraph: aParagraph<br>
"Answer an instance of TextLineInterval that represents the next line in the paragraph."<br>
| runLength stopCondition |<br>
destX := spaceX := leftMargin := aParagraph leftMarginForCompositionForLine: lineIndex.<br>
destY := 0.<br>
rightMargin := aParagraph rightMarginForComposition.<br>
leftMargin >= rightMargin ifTrue: [self error: 'No room between margins to compose'].<br>
lastIndex := startIndex. "scanning sets last index"<br>
lineHeight := textStyle lineGrid. "may be increased by setFont:..."<br>
baseline := textStyle baseline.<br>
self setStopConditions. "also sets font"<br>
self handleIndentation.<br>
runLength := text runLengthFor: startIndex.<br>
runStopIndex := (lastIndex := startIndex) + (runLength - 1).<br>
line := TextLineInterval<br>
start: lastIndex<br>
stop: 0<br>
internalSpaces: 0<br>
paddingWidth: 0.<br>
+ nextIndexAfterLineBreak := spaceCount := 0.<br>
- spaceCount := 0.<br>
lastBreakIsNotASpace := false.<br>
<br>
[stopCondition := self scanCharactersFrom: lastIndex to: runStopIndex<br>
in: text string rightX: rightMargin stopConditions: stopConditions<br>
kern: kern.<br>
"See setStopConditions for stopping conditions for composing."<br>
self perform: stopCondition] whileFalse.<br>
<br>
^line<br>
lineHeight: lineHeight + textStyle leading<br>
baseline: baseline + textStyle leading!<br>
<br>
Item was changed:<br>
----- Method: CompositionScanner>>cr (in category 'stop conditions') -----<br>
cr<br>
"Answer true. Set up values for the text line interval currently being<br>
composed."<br>
<br>
pendingKernX := 0.<br>
(lastIndex < text size and: [(text at: lastIndex) = CR and: [(text at: lastIndex+1) = Character lf]]) ifTrue: [lastIndex := lastIndex + 1].<br>
line stop: lastIndex.<br>
+ nextIndexAfterLineBreak := lastIndex + 1.<br>
spaceX := destX.<br>
lastBreakIsNotASpace := false.<br>
line paddingWidth: rightMargin - spaceX.<br>
^true!<br>
<br>
Item was changed:<br>
----- Method: CompositionScanner>>crossedX (in category 'stop conditions') -----<br>
crossedX<br>
"There is a word that has fallen across the right edge of the composition<br>
rectangle. This signals the need for wrapping which is done to the last<br>
space that was encountered, as recorded by the space stop condition,<br>
or any other breakable character if the language permits so."<br>
<br>
pendingKernX := 0.<br>
<br>
lastBreakIsNotASpace ifTrue:<br>
+ ["In some languages break is possible before a non space."<br>
+ nextIndexAfterLineBreak := spaceIndex.<br>
+ line stop: spaceIndex - 1.<br>
- ["In some languages break is possible on non space."<br>
- line stop: spaceIndex.<br>
lineHeight := lineHeightAtSpace.<br>
baseline := baselineAtSpace.<br>
+ line paddingWidth: rightMargin - spaceX.<br>
- spaceCount := spaceCount - 1.<br>
- spaceIndex := spaceIndex - 1.<br>
- line paddingWidth: rightMargin.<br>
line internalSpaces: spaceCount.<br>
^true].<br>
<br>
spaceCount >= 1 ifTrue:<br>
["The common case. First back off to the space at which we wrap."<br>
line stop: spaceIndex.<br>
+ nextIndexAfterLineBreak := spaceIndex + 1.<br>
lineHeight := lineHeightAtSpace.<br>
baseline := baselineAtSpace.<br>
spaceCount := spaceCount - 1.<br>
spaceIndex := spaceIndex - 1.<br>
<br>
"Check to see if any spaces preceding the one at which we wrap.<br>
Double space after punctuation, most likely."<br>
[(spaceCount > 1 and: [(text at: spaceIndex) = Space])]<br>
whileTrue:<br>
[spaceCount := spaceCount - 1.<br>
"Account for backing over a run which might<br>
change width of space."<br>
font := text fontAt: spaceIndex withStyle: textStyle.<br>
spaceIndex := spaceIndex - 1.<br>
spaceX := spaceX - (font widthOf: Space)].<br>
line paddingWidth: rightMargin - spaceX.<br>
line internalSpaces: spaceCount]<br>
ifFalse:<br>
["Neither internal nor trailing spaces -- almost never happens."<br>
lastIndex := lastIndex - 1.<br>
[destX <= rightMargin or: [ lastIndex = 0 ]]<br>
whileFalse:<br>
[destX := destX - (font widthOf: (text at: lastIndex)).<br>
lastIndex := lastIndex - 1].<br>
+ nextIndexAfterLineBreak := lastIndex + 1.<br>
spaceX := destX.<br>
line paddingWidth: rightMargin - destX.<br>
line stop: (lastIndex max: line first)].<br>
^true!<br>
<br>
Item was added:<br>
+ ----- Method: CompositionScanner>>doesTheLineBreaksAfterLastChar (in category 'accessing') -----<br>
+ doesTheLineBreaksAfterLastChar<br>
+ ^nextIndexAfterLineBreak > text size!<br>
<br>
Item was changed:<br>
----- Method: TextComposer>>composeLinesFrom:to:delta:into:priorLines:atY:textStyle:text:container:wantsColumnBreaks: (in category 'as yet unclassified') -----<br>
composeLinesFrom: argStart to: argStop delta: argDelta into: argLinesCollection priorLines: argPriorLines atY: argStartY textStyle: argTextStyle text: argText container: argContainer wantsColumnBreaks: argWantsColumnBreaks<br>
<br>
wantsColumnBreaks := argWantsColumnBreaks.<br>
lines := argLinesCollection.<br>
theTextStyle := argTextStyle.<br>
theText := argText.<br>
theContainer := argContainer.<br>
deltaCharIndex := argDelta.<br>
currCharIndex := startCharIndex := argStart.<br>
stopCharIndex := argStop.<br>
prevLines := argPriorLines.<br>
currentY := argStartY.<br>
maxRightX := theContainer left.<br>
possibleSlide := stopCharIndex < theText size and: [theContainer isMemberOf: Rectangle].<br>
nowSliding := false.<br>
prevIndex := 1.<br>
"choose an appropriate scanner - should go away soon, when they can be unified"<br>
scanner := CompositionScanner new.<br>
scanner text: theText textStyle: theTextStyle.<br>
scanner wantsColumnBreaks: wantsColumnBreaks.<br>
defaultLineHeight := scanner computeDefaultLineHeight.<br>
isFirstLine := true.<br>
self composeAllLines.<br>
isFirstLine ifTrue: ["No space in container or empty text"<br>
self<br>
addNullLineWithIndex: startCharIndex<br>
andRectangle: (theContainer topLeft extent: 0@defaultLineHeight)<br>
] ifFalse: [<br>
+ (lines last last = theText size and: [scanner doesTheLineBreaksAfterLastChar])<br>
+ ifTrue: [self addNullLineForIndex: theText size + 1]<br>
- self fixupLastLineIfCR<br>
].<br>
^{lines asArray. maxRightX}<br>
-<br>
!<br>
<br>
Item was removed:<br>
- ----- Method: TextComposer>>fixupLastLineIfCR (in category 'as yet unclassified') -----<br>
- fixupLastLineIfCR<br>
- "This awful bit is to ensure that if we have scanned all the text and the last character is a CR that there is a null line at the end of lines. Sometimes this was not happening which caused anomalous selections when selecting all the text. This is implemented as a post-composition fixup because I couldn't figure out where to put it in the main logic."<br>
-<br>
- (theText size > 0 and: [CharacterSet crlf includes: theText last]) ifFalse: [^self].<br>
- self addNullLineForIndex: theText size + 1.<br>
- !<br>
<br>
Item was removed:<br>
- ----- Method: TextComposer>>multiComposeLinesFrom:to:delta:into:priorLines:atY:textStyle:text:container:wantsColumnBreaks: (in category 'as yet unclassified') -----<br>
- multiComposeLinesFrom: argStart to: argStop delta: argDelta into: argLinesCollection priorLines: argPriorLines atY: argStartY textStyle: argTextStyle text: argText container: argContainer wantsColumnBreaks: argWantsColumnBreaks<br>
-<br>
- "temporarily add this here to support move to drop MultiTextComposer"<br>
- "now redundant and ready to remove later"<br>
- wantsColumnBreaks := argWantsColumnBreaks.<br>
- lines := argLinesCollection.<br>
- theTextStyle := argTextStyle.<br>
- theText := argText.<br>
- theContainer := argContainer.<br>
- deltaCharIndex := argDelta.<br>
- currCharIndex := startCharIndex := argStart.<br>
- stopCharIndex := argStop.<br>
- prevLines := argPriorLines.<br>
- currentY := argStartY.<br>
- maxRightX := theContainer left.<br>
- possibleSlide := stopCharIndex < theText size and: [theContainer isMemberOf: Rectangle].<br>
- nowSliding := false.<br>
- prevIndex := 1.<br>
- scanner := CompositionScanner new text: theText textStyle: theTextStyle.<br>
- scanner wantsColumnBreaks: wantsColumnBreaks.<br>
- defaultLineHeight := scanner computeDefaultLineHeight.<br>
- isFirstLine := true.<br>
- self composeAllLines.<br>
- isFirstLine ifTrue: ["No space in container or empty text"<br>
- self<br>
- addNullLineWithIndex: startCharIndex<br>
- andRectangle: (theContainer topLeft extent: 0@defaultLineHeight)<br>
- ] ifFalse: [<br>
- self fixupLastLineIfCR<br>
- ].<br>
- ^{lines asArray. maxRightX}<br>
-<br>
- !<br>
<br>
<br>
</blockquote></div><br></div>