[squeak-dev] wait2ms (The Trunk: ST80-dtl.141.mcz)

David T. Lewis lewis at mail.msen.com
Sat Feb 9 20:29:43 UTC 2013


Bob,

I tried reverting zapSelectionWithCompositionWith: to its original
implementation, and making the change that you suggest to readKeyboard.
It works exactly as you describe, including when I type rapidly in an
image with the wait2ms delay.

I will make this update to Squeak trunk. To summarize the changes
we will have at that point:

1) The wait2ms is removed
2) ParagraphEditor>>readKeyboard will always process one character at a time
3) No change to zapSelectionWithCompositionWith:

Thanks very much for your help on this!

Dave

On Sat, Feb 09, 2013 at 02:57:32PM -0500, Bob Arning wrote:
> 
> On 2/9/13 1:48 PM, David T. Lewis wrote:
> >Hi Bob, thanks for this.
> >
> >I am hopelessly out of my depth on multilingual processing and multibyte
> >characters, but I'm sure others on the list will know: Are there cases
> >(e.g. in Japan or Korea) where keyboards provide multi-byte inputs, either
> >directly or through X11? If so, the "sensor keyboardPressed whileTrue:"
> >may be necessary in order to process those key entries.
> Remember that there is another "[sensor keyboardPressed] whileTrue: " 
> higher up that encompases this, so I don't think this is something to 
> worry about.
> >Otherwise, I
> >suspect you are right that it is easier to just process one character at
> >a time. I note also that the "whileTrue:" approach dates back to at least
> >2002, well before introduction of multilingual support in Squeak. I don't
> >know if that's significant, or if that's just the way it happened to have
> >been done.
> It was that way in 1998. My guess is that if multiple input characters 
> were available, it made some sense efficiency-wise to insert them into 
> the text all at once rather than piecemeal. Back the people typed just 
> as fast as today, but the processors were much slower.
> >
> >Just by way of explanation, here is how I found and verified the change
> >to zapSelectionWithCompositionWith: in ST80-dtl.141.mcz:
> >
> >1) In the original method, I forced the method to always execute the
> >multi-character path through the code, even if the input string was one
> >character long (the normal case). So I changed from this:
> >
> >   ((aString isEmpty or: [(beforeChar := self charBefore) isNil]) or: [
> >     aString size = 1 and: [(Unicode isComposition: aString first) not]]) 
> >     ifTrue: [
> >       ^ self zapSelectionWith: (Text string: aString emphasis: 
> >       emphasisHere)].
> >
> >to this:
> >
> >   ((aString isEmpty or: [(beforeChar := self charBefore) isNil]) or: [
> >     aString size = 1 and: [(Unicode isComposition: aString first) not]]) 
> >     ifTrue: [
> >       "^ self zapSelectionWith: (Text string: aString emphasis: 
> >       emphasisHere)" ].
> >
> >Result: All characters typed into an MVC workspace resulted in the cursor
> >positioned incorrectly to the left of the current character.
> >
> >2) I changed the method to reposition the cursor correctly, leaving the
> >above change in place to continue forcing execution of the multi-byte code
> >path. This fixed the cursor positioning problem in the MVC workspace.
> >
> >3) I removed the change from step 1 so that single character input would
> >be handled through the normal path, and verified that the MVC workspace
> >still worked.
> >
> >4) I filed the updated method into another image that still has the wait2ms
> >delays, and I ran that image on a Linux interpreter VM (with "2 ms" delays
> >that might be more like 15 ms). Result: Cursor positioning now worked 
> >correctly
> >in the MVC workspace, even though keyboard entry was slow.
> I put your change in an otherwise unchanged 4.4 and mashed keys very 
> quickly. Everything got entered, but the cursor got farther and farther 
> behind the end of the text. This is on a Mac where 2ms seems to be 2ms. 
> I then changed wait2ms to wait 20ms and things got worse - it took long 
> pauses for characters to appear and when they did, the cursor did not 
> keep pace with the end of the text. What might be interesting to know 
> how often (if ever) your tests actually had more that one byte to 
> process in the zap methods. I think a single character works fine either 
> way, multiple characters work fine in the original zap.. but multiple 
> characters in the new zap are the problem.
> >
> >Conclusion: The patch in ST80-dtl.141.mcz does address a real bug in cursor
> >positioning that occured only in cases where sluggish key entry resulted
> >in multi-byte strings (presumably two characters) being handled in the
> >zapSelectionWithCompositionWith: method.
> >
> >The problem that you identified with your "z123" test does not seem to be
> >occurring in practice,
> But it did and that's what got me started here - I was trying to see 
> sluggish input in MVC, so I mashed keys as fast as I could and they all 
> made  it to the screen, just not always at the end of the text.
> >but it certainly looks as though it *could* occur.
> >I tested only by typing into a workspace from my US ascii keyboard, so I
> >certainly would not rule out the possibility of further problems.
> >
> >I do not know if it is safe to make the change you suggest to process one
> >character at a time. It looks to me like it would be good to do that, but
> >I don't know if it might cause problems for non-US keyboards or locales.
> >
> >If it was safe to make that change, then the entire bit of code for 
> >handling
> >multi-byte strings in zapSelectionWithCompositionWith: would become 
> >irrelevant,
> >and we could just get rid of it.
> I don't think so. That code has a purpose - the UnicodeCompositionStream 
> is used to combined multiple characters (like a letter followed by a 
> separate accent character) into a single character. It is basically 
> saying: if there are 1 or more characters in the typeahead and the first 
> is one of these combining characters (accent) and there is a character 
> in the text before the insertion point, then back up 1 space so we can 
> process that character and the accent together. I suspect it works fine 
> when there is just once character in the typeahead and that character is 
> an accent. I don't think it works well when there are more than one 
> character in the input. Hence my suggestion to do it a char at a time.
> 
> Cheers,
> Bob
> >
> >Hopefully someone with experience in this area can help?
> >
> >Dave
> >
> >
> >On Sat, Feb 09, 2013 at 06:52:43AM -0500, Bob Arning wrote:
> >>I fear that doesn't even work with ordinary USASCII input. I tweaked
> >>readKeyboard to simulate additional characters arriving in close 
> >>succession:
> >>
> >>         self hasSelection ifTrue: "save highlighted characters"
> >>             [UndoSelection := self selection].
> >>*((model isKindOf: Workspace) and: [char = $z]) ifTrue: [typeAhead
> >>nextPutAll: '123'].*
> >>
> >>         self zapSelectionWithCompositionWith: typeAhead contents.
> >>
> >>and what I get by typing some z's in a Workspace is:
> >>
> >>z123zzzzzz123123123123123123
> >>
> >>I think the safest way to guard against multiple characters arriving in
> >>quick succession is to simply process each one completely before
> >>proceeding to the next. This, after all, is the likeliest case by far
> >>for modern computers and ordinary humans typing:
> >>
> >>         self deselect.
> >>*sensor keyboardPressed ifTrue: "was a whileTrue:"*
> >>             [char := sensor keyboardPeek.
> >>             (self dispatchOnCharacter: char with: typeAhead) ifTrue:
> >>                 [self doneTyping.
> >>                 self setEmphasisHere.
> >>                 ^self selectAndScroll; updateMarker].
> >>             self openTypeIn].
> >>
> >>Cheers,
> >>Bob
> >>
> >>On 2/9/13 1:01 AM, commits at source.squeak.org wrote:
> >>>aText := Text string: newString emphasis: emphasisHere.
> >>>- 	self markBlock < self pointBlock
> >>>- 		ifTrue: [self setMark: self markBlock stringIndex - 1]
> >>>- 		ifFalse: [self setPoint: self  pointBlock stringIndex - 1].
> >>>-
> >>>   	wasComposition := true.
> >>>+ 	self markBlock < self pointBlock
> >>>+ 		ifTrue: [ self setMark: self markBlock stringIndex - 1.
> >>>+ 				self zapSelectionWith: aText.
> >>>+ 				self setMark: self markBlock stringIndex + 1]
> >>>+ 		ifFalse: [ self setPoint: self pointBlock stringIndex - 1.
> >>>+ 				self zapSelectionWith: aText.
> >>>+ 				self setPoint: self pointBlock stringIndex +
> >>>1]
> >>>- 	self zapSelectionWith: aText.
> >
> >
> 


More information about the Squeak-dev mailing list